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 <[email protected]>
---
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;
--
2.7.4
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 <[email protected]>
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;
>
On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
> 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 <[email protected]>
> Looks good to me.
>
> Lars could you sanity check this one as well?
Acked-by: Lars-Peter Clausen <[email protected]>
On 30/10/16 17:46, Lars-Peter Clausen wrote:
> On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
>> 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 <[email protected]>
>> Looks good to me.
>>
>> Lars could you sanity check this one as well?
>
> Acked-by: Lars-Peter Clausen <[email protected]>
There was a bit of fun applying this. Eva, can you check I didn't mess it
up?
Applied to the togreg branch of iio.git. Initially pushed out as testing
for the autobuilders to play with it. (and this time I remembered to actually
do the push!)
thanks,
Jonathan
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Sun, Oct 30, 2016 at 06:49:00PM +0000, Jonathan Cameron wrote:
> On 30/10/16 17:46, Lars-Peter Clausen wrote:
> > On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
> >> 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 <[email protected]>
> >> Looks good to me.
> >>
> >> Lars could you sanity check this one as well?
> >
> > Acked-by: Lars-Peter Clausen <[email protected]>
> There was a bit of fun applying this. Eva, can you check I didn't mess it
> up?
It looks fine to me. Thanks!
>
> Applied to the togreg branch of iio.git. Initially pushed out as testing
> for the autobuilders to play with it. (and this time I remembered to actually
> do the push!)
>
> thanks,
>
> Jonathan
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
On Mon, Oct 31, 2016 at 03:49:01PM +0800, Eva Rachel Retuya wrote:
> On Sun, Oct 30, 2016 at 06:49:00PM +0000, Jonathan Cameron wrote:
> > On 30/10/16 17:46, Lars-Peter Clausen wrote:
> > > On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
> > >> 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 <[email protected]>
> > >> Looks good to me.
> > >>
> > >> Lars could you sanity check this one as well?
> > >
> > > Acked-by: Lars-Peter Clausen <[email protected]>
> > There was a bit of fun applying this. Eva, can you check I didn't mess it
> > up?
>
> It looks fine to me. Thanks!
>
Have to retract that statement, I rebased and went through the merge
conflict. Extra whitespace can be seen after the statement in line 644:
*val = ad7746_cap_filter_rate_table[idx][0];
What to do?
Eva
> >
> > Applied to the togreg branch of iio.git. Initially pushed out as testing
> > for the autobuilders to play with it. (and this time I remembered to actually
> > do the push!)
> >
> > thanks,
> >
> > Jonathan
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> >
On 31/10/16 08:05, Eva Rachel Retuya wrote:
> On Mon, Oct 31, 2016 at 03:49:01PM +0800, Eva Rachel Retuya wrote:
>> On Sun, Oct 30, 2016 at 06:49:00PM +0000, Jonathan Cameron wrote:
>>> On 30/10/16 17:46, Lars-Peter Clausen wrote:
>>>> On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
>>>>> 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 <[email protected]>
>>>>> Looks good to me.
>>>>>
>>>>> Lars could you sanity check this one as well?
>>>>
>>>> Acked-by: Lars-Peter Clausen <[email protected]>
>>> There was a bit of fun applying this. Eva, can you check I didn't mess it
>>> up?
>>
>> It looks fine to me. Thanks!
>>
>
> Have to retract that statement, I rebased and went through the merge
> conflict. Extra whitespace can be seen after the statement in line 644:
> *val = ad7746_cap_filter_rate_table[idx][0];
>
Err I messed up. Will fix... Will push out in a mo.
Jonathan
> What to do?
> Eva
>
>>>
>>> Applied to the togreg branch of iio.git. Initially pushed out as testing
>>> for the autobuilders to play with it. (and this time I remembered to actually
>>> do the push!)
>>>
>>> thanks,
>>>
>>> Jonathan
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>