On 28/10/16 09:26, Eva Rachel Retuya wrote: > Introduce defines for shifting and mask under the config register for > better readability. Also, introduce helper variables for index > calculation. > > Signed-off-by: Eva Rachel Retuya <eraretuya@xxxxxxxxx> Looks good to me. Lars could you sanity check this one as well? Thanks, Jonathan > --- > This patch might cause a conflict with this patch: > staging: iio: cdc/ad7746: fix missing return value > https://marc.info/?l=linux-driver-devel&m=147741283722806&w=2 > I am waiting to rebase to the branch where this commit is present but > I was told it was not yet pushed to kernel.org > > Changes in v3: > * Make commit message more detailed. > * Fix incorrect use of GENMASK. > * Drop the AD7746_CONF_*FS(x) macros and use the mask and direct shifting where > needed. > * With the proper masks set, invert the AND'ng and shifting on the index > calculations. > > Changes in v2: > * Use GENMASK to generate both VTFS and CAPFS masks including offset, > respectively. > * Introduce helper variables for index calculation. > > drivers/staging/iio/cdc/ad7746.c | 38 ++++++++++++++++++++++---------------- > 1 file changed, 22 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c > index f41251c..e97658a 100644 > --- a/drivers/staging/iio/cdc/ad7746.c > +++ b/drivers/staging/iio/cdc/ad7746.c > @@ -70,8 +70,10 @@ > #define AD7746_EXCSETUP_EXCLVL(x) (((x) & 0x3) << 0) > > /* Config Register Bit Designations (AD7746_REG_CFG) */ > -#define AD7746_CONF_VTFS(x) ((x) << 6) > -#define AD7746_CONF_CAPFS(x) ((x) << 3) > +#define AD7746_CONF_VTFS_SHIFT 6 > +#define AD7746_CONF_CAPFS_SHIFT 3 > +#define AD7746_CONF_VTFS_MASK GENMASK(7, 6) > +#define AD7746_CONF_CAPFS_MASK GENMASK(5, 3) > #define AD7746_CONF_MODE_IDLE (0 << 0) > #define AD7746_CONF_MODE_CONT_CONV (1 << 0) > #define AD7746_CONF_MODE_SINGLE_CONV (2 << 0) > @@ -217,15 +219,16 @@ static int ad7746_select_channel(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan) > { > struct ad7746_chip_info *chip = iio_priv(indio_dev); > - int ret, delay; > + int ret, delay, idx; > u8 vt_setup, cap_setup; > > switch (chan->type) { > case IIO_CAPACITANCE: > cap_setup = (chan->address & 0xFF) | AD7746_CAPSETUP_CAPEN; > vt_setup = chip->vt_setup & ~AD7746_VTSETUP_VTEN; > - delay = ad7746_cap_filter_rate_table[(chip->config >> 3) & > - 0x7][1]; > + idx = (chip->config & AD7746_CONF_CAPFS_MASK) >> > + AD7746_CONF_CAPFS_SHIFT; > + delay = ad7746_cap_filter_rate_table[idx][1]; > > if (chip->capdac_set != chan->channel) { > ret = i2c_smbus_write_byte_data(chip->client, > @@ -246,8 +249,9 @@ static int ad7746_select_channel(struct iio_dev *indio_dev, > case IIO_TEMP: > vt_setup = (chan->address & 0xFF) | AD7746_VTSETUP_VTEN; > cap_setup = chip->cap_setup & ~AD7746_CAPSETUP_CAPEN; > - delay = ad7746_cap_filter_rate_table[(chip->config >> 6) & > - 0x3][1]; > + idx = (chip->config & AD7746_CONF_VTFS_MASK) >> > + AD7746_CONF_VTFS_SHIFT; > + delay = ad7746_cap_filter_rate_table[idx][1]; > break; > default: > return -EINVAL; > @@ -369,8 +373,8 @@ static int ad7746_store_cap_filter_rate_setup(struct ad7746_chip_info *chip, > if (i >= ARRAY_SIZE(ad7746_cap_filter_rate_table)) > i = ARRAY_SIZE(ad7746_cap_filter_rate_table) - 1; > > - chip->config &= ~AD7746_CONF_CAPFS(0x7); > - chip->config |= AD7746_CONF_CAPFS(i); > + chip->config &= ~AD7746_CONF_CAPFS_MASK; > + chip->config |= i << AD7746_CONF_CAPFS_SHIFT; > > return 0; > } > @@ -387,8 +391,8 @@ static int ad7746_store_vt_filter_rate_setup(struct ad7746_chip_info *chip, > if (i >= ARRAY_SIZE(ad7746_vt_filter_rate_table)) > i = ARRAY_SIZE(ad7746_vt_filter_rate_table) - 1; > > - chip->config &= ~AD7746_CONF_VTFS(0x3); > - chip->config |= AD7746_CONF_VTFS(i); > + chip->config &= ~AD7746_CONF_VTFS_MASK; > + chip->config |= i << AD7746_CONF_VTFS_SHIFT; > > return 0; > } > @@ -527,7 +531,7 @@ static int ad7746_read_raw(struct iio_dev *indio_dev, > long mask) > { > struct ad7746_chip_info *chip = iio_priv(indio_dev); > - int ret, delay; > + int ret, delay, idx; > u8 regval, reg; > > mutex_lock(&indio_dev->mlock); > @@ -635,13 +639,15 @@ static int ad7746_read_raw(struct iio_dev *indio_dev, > case IIO_CHAN_INFO_SAMP_FREQ: > switch (chan->type) { > case IIO_CAPACITANCE: > - *val = ad7746_cap_filter_rate_table[ > - (chip->config >> 3) & 0x7][0]; > + idx = (chip->config & AD7746_CONF_CAPFS_MASK) >> > + AD7746_CONF_CAPFS_SHIFT; > + *val = ad7746_cap_filter_rate_table[idx][0]; > ret = IIO_VAL_INT; > break; > case IIO_VOLTAGE: > - *val = ad7746_vt_filter_rate_table[ > - (chip->config >> 6) & 0x3][0]; > + idx = (chip->config & AD7746_CONF_VTFS_MASK) >> > + AD7746_CONF_VTFS_SHIFT; > + *val = ad7746_vt_filter_rate_table[idx][0]; > break; > default: > ret = -EINVAL; > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
