| Message ID | 20220320181428.168109-9-marex@denx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,01/10] dt-bindings: iio: adc: ti,ads1015: Add missing ADS1115 compatible string | expand |
On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote: > > Instead of storing only data_rate in private data, store pointer to the > whole chip data and use the data_rate from chip data throughout the driver. > No functional change. This is done in preparation for switch to read_avail(). switching ... > if (period <= ads1015_comp_queue[i] * > - USEC_PER_SEC / data->data_rate[dr]) > + USEC_PER_SEC / data_rate[dr]) I would put these two to one line.
On 3/21/22 10:26, Andy Shevchenko wrote: > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote: >> >> Instead of storing only data_rate in private data, store pointer to the >> whole chip data and use the data_rate from chip data throughout the driver. >> No functional change. This is done in preparation for switch to read_avail(). > > switching Fixed > ... > >> if (period <= ads1015_comp_queue[i] * >> - USEC_PER_SEC / data->data_rate[dr]) >> + USEC_PER_SEC / data_rate[dr]) > > I would put these two to one line. That'd make the line over 80 chars long, is that OK in iio now ?
On Mon, Mar 21, 2022 at 03:41:20PM +0100, Marek Vasut wrote: > On 3/21/22 10:26, Andy Shevchenko wrote: > > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote: ... > > > if (period <= ads1015_comp_queue[i] * > > > - USEC_PER_SEC / data->data_rate[dr]) > > > + USEC_PER_SEC / data_rate[dr]) if (period <= ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr]) > > I would put these two to one line. > > That'd make the line over 80 chars long, is that OK in iio now ? If not (Jonathan, what's your opinion?), you can do a better splitting if (period <= ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr]) although I think it's worse than putting on one line.
On Mon, 21 Mar 2022 18:17:28 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Mar 21, 2022 at 03:41:20PM +0100, Marek Vasut wrote: > > On 3/21/22 10:26, Andy Shevchenko wrote: > > > On Sun, Mar 20, 2022 at 8:14 PM Marek Vasut <marex@denx.de> wrote: > > ... > > > > > if (period <= ads1015_comp_queue[i] * > > > > - USEC_PER_SEC / data->data_rate[dr]) > > > > + USEC_PER_SEC / data_rate[dr]) > > if (period <= ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr]) > > > > I would put these two to one line. > > > > That'd make the line over 80 chars long, is that OK in iio now ? Depends on readability. Where it doesn't hurt such as line breaks in between functional parameters I prefer we stick to sub 80 chars. > > If not (Jonathan, what's your opinion?), you can do a better splitting > > if (period <= > ads1015_comp_queue[i] * USEC_PER_SEC / data_rate[dr]) > > although I think it's worse than putting on one line. > Agreed. This particular case is more readable on one line. Thanks, Jonathan
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c index 19e75ebdddd49..66431d1445d9b 100644 --- a/drivers/iio/adc/ti-ads1015.c +++ b/drivers/iio/adc/ti-ads1015.c @@ -227,7 +227,7 @@ struct ads1015_data { unsigned int comp_mode; struct ads1015_thresh_data thresh_data[ADS1015_CHANNELS]; - const unsigned int *data_rate; + const struct ads1015_chip_data *chip; /* * Set to true when the ADC is switched to the continuous-conversion * mode and exits from a power-down state. This flag is used to avoid @@ -368,6 +368,7 @@ static int ads1015_set_power_state(struct ads1015_data *data, bool on) static int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val) { + const unsigned int *data_rate = data->chip->data_rate; int ret, pga, dr, dr_old, conv_time; unsigned int old, mask, cfg; @@ -402,8 +403,8 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val) } if (data->conv_invalid) { dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT; - conv_time = DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr_old]); - conv_time += DIV_ROUND_UP(USEC_PER_SEC, data->data_rate[dr]); + conv_time = DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr_old]); + conv_time += DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr]); conv_time += conv_time / 10; /* 10% internal clock inaccuracy */ usleep_range(conv_time, conv_time + 1); data->conv_invalid = false; @@ -470,7 +471,7 @@ static int ads1015_set_data_rate(struct ads1015_data *data, int chan, int rate) int i; for (i = 0; i < ARRAY_SIZE(ads1015_data_rate); i++) { - if (data->data_rate[i] == rate) { + if (data->chip->data_rate[i] == rate) { data->channel_data[chan].data_rate = i; return 0; } @@ -528,7 +529,7 @@ static int ads1015_read_raw(struct iio_dev *indio_dev, break; case IIO_CHAN_INFO_SAMP_FREQ: idx = data->channel_data[chan->address].data_rate; - *val = data->data_rate[idx]; + *val = data->chip->data_rate[idx]; ret = IIO_VAL_INT; break; default: @@ -588,7 +589,7 @@ static int ads1015_read_event(struct iio_dev *indio_dev, dr = data->channel_data[chan->address].data_rate; comp_queue = data->thresh_data[chan->address].comp_queue; period = ads1015_comp_queue[comp_queue] * - USEC_PER_SEC / data->data_rate[dr]; + USEC_PER_SEC / data->chip->data_rate[dr]; *val = period / USEC_PER_SEC; *val2 = period % USEC_PER_SEC; @@ -610,6 +611,7 @@ static int ads1015_write_event(struct iio_dev *indio_dev, int val2) { struct ads1015_data *data = iio_priv(indio_dev); + const unsigned int *data_rate = data->chip->data_rate; int realbits = chan->scan_type.realbits; int ret = 0; long long period; @@ -635,7 +637,7 @@ static int ads1015_write_event(struct iio_dev *indio_dev, for (i = 0; i < ARRAY_SIZE(ads1015_comp_queue) - 1; i++) { if (period <= ads1015_comp_queue[i] * - USEC_PER_SEC / data->data_rate[dr]) + USEC_PER_SEC / data_rate[dr]) break; } data->thresh_data[chan->address].comp_queue = i; @@ -992,7 +994,7 @@ static int ads1015_probe(struct i2c_client *client, indio_dev->channels = chip->channels; indio_dev->num_channels = chip->num_channels; indio_dev->info = chip->info; - data->data_rate = chip->data_rate; + data->chip = chip; data->event_channel = ADS1015_CHANNELS; /*
Instead of storing only data_rate in private data, store pointer to the whole chip data and use the data_rate from chip data throughout the driver. No functional change. This is done in preparation for switch to read_avail(). Signed-off-by: Marek Vasut <marex@denx.de> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Daniel Baluta <daniel.baluta@nxp.com> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- V3: New patch --- drivers/iio/adc/ti-ads1015.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)