[PATCH 0/2] correct macro issues

[PATCH 0/2] correct macro issues

[Michael Harris]

This series corrects the following issues found in the macro definitions:
- Magic numbers which correspond to bits or masks, but aren't using
  BIT() or GENMASK()
- Macros that aren't in use anywhere in the driver

Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
---
Michael Harris (2):
      staging: iio: adt7316: remove unused macros
      staging: iio: adt7316: convert magic numbers to BIT() and GENMASK()

 drivers/staging/iio/addac/adt7316.c | 91 ++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 51 deletions(-)
---
base-commit: 5d2905f2c6acdd1c553f9b022dee730ab2cced71
change-id: 20260130-adt7316-correct-macros-b6801b4ae1b1

Best regards,
-- 
Michael Harris <michaelharriscode@gmail.com>

[PATCH 1/2] staging: iio: adt7316: remove unused macros

[Michael Harris]

Delete macros that aren't in use

Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
---
 drivers/staging/iio/addac/adt7316.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
index 8a9a8262c2bec34f3c3e79d8174f492b9a23fb70..4173c8822fff495e8c69d9cf6c11be9e9227a8c1 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -111,7 +111,6 @@
 #define ADT7316_AD_SINGLE_CH_MODE	0x10
 #define ADT7316_DISABLE_AVERAGING	0x20
 #define ADT7316_EN_SMBUS_TIMEOUT	0x40
-#define ADT7316_RESET			0x80
 
 /*
  * ADT7316 config3
@@ -154,7 +153,6 @@
 /*
  * ADT7316 value masks
  */
-#define ADT7316_VALUE_MASK		0xfff
 #define ADT7316_T_VALUE_SIGN		0x400
 #define ADT7316_T_VALUE_FLOAT_OFFSET	2
 #define ADT7316_T_VALUE_FLOAT_MASK	0x2
@@ -194,15 +192,6 @@ struct adt7316_chip_info {
  * Logic interrupt mask for user application to enable
  * interrupts.
  */
-#define ADT7316_IN_TEMP_HIGH_INT_MASK	0x1
-#define ADT7316_IN_TEMP_LOW_INT_MASK	0x2
-#define ADT7316_EX_TEMP_HIGH_INT_MASK	0x4
-#define ADT7316_EX_TEMP_LOW_INT_MASK	0x8
-#define ADT7316_EX_TEMP_FAULT_INT_MASK	0x10
-#define ADT7516_AIN1_INT_MASK		0x4
-#define ADT7516_AIN2_INT_MASK		0x20
-#define ADT7516_AIN3_INT_MASK		0x40
-#define ADT7516_AIN4_INT_MASK		0x80
 #define ADT7316_VDD_INT_MASK		0x100
 #define ADT7316_TEMP_INT_MASK		0x1F
 #define ADT7516_AIN_INT_MASK		0xE0

-- 
2.52.0

Re: [PATCH 1/2] staging: iio: adt7316: remove unused macros

[Jonathan Cameron]

On Fri, 30 Jan 2026 23:38:27 -0800
Michael Harris <michaelharriscode@gmail.com> wrote:

> Delete macros that aren't in use
Hi Michael,

Why?  

I'm not against driver authors only introducing register defines they
are using, but I'm also not against them deciding on comprehensive defines
or at least making sure they cover all the fields in a given register.

Having only parts of a register defined sometimes makes people miss
that they can't just write the whole thing because there may be other
bits they do not indent to change. As such it can be useful to have them there.

Thanks,

Jonathan


> 
> Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
> ---
>  drivers/staging/iio/addac/adt7316.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> index 8a9a8262c2bec34f3c3e79d8174f492b9a23fb70..4173c8822fff495e8c69d9cf6c11be9e9227a8c1 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -111,7 +111,6 @@
>  #define ADT7316_AD_SINGLE_CH_MODE	0x10
>  #define ADT7316_DISABLE_AVERAGING	0x20
>  #define ADT7316_EN_SMBUS_TIMEOUT	0x40
> -#define ADT7316_RESET			0x80
>  
>  /*
>   * ADT7316 config3
> @@ -154,7 +153,6 @@
>  /*
>   * ADT7316 value masks
>   */
> -#define ADT7316_VALUE_MASK		0xfff
>  #define ADT7316_T_VALUE_SIGN		0x400
>  #define ADT7316_T_VALUE_FLOAT_OFFSET	2
>  #define ADT7316_T_VALUE_FLOAT_MASK	0x2
> @@ -194,15 +192,6 @@ struct adt7316_chip_info {
>   * Logic interrupt mask for user application to enable
>   * interrupts.
>   */
> -#define ADT7316_IN_TEMP_HIGH_INT_MASK	0x1
> -#define ADT7316_IN_TEMP_LOW_INT_MASK	0x2
> -#define ADT7316_EX_TEMP_HIGH_INT_MASK	0x4
> -#define ADT7316_EX_TEMP_LOW_INT_MASK	0x8
> -#define ADT7316_EX_TEMP_FAULT_INT_MASK	0x10
> -#define ADT7516_AIN1_INT_MASK		0x4
> -#define ADT7516_AIN2_INT_MASK		0x20
> -#define ADT7516_AIN3_INT_MASK		0x40
> -#define ADT7516_AIN4_INT_MASK		0x80
>  #define ADT7316_VDD_INT_MASK		0x100
>  #define ADT7316_TEMP_INT_MASK		0x1F
>  #define ADT7516_AIN_INT_MASK		0xE0
> 

Re: [PATCH 1/2] staging: iio: adt7316: remove unused macros

[Michael Harris]

(Resent to include the mailing list, apologies for the duplicate)

On 1/31/26 7:54 AM, Jonathan Cameron wrote:
>> Delete macros that aren't in use
> Why?
>
> I'm not against driver authors only introducing register defines they
> are using, but I'm also not against them deciding on comprehensive defines
> or at least making sure they cover all the fields in a given register.
>
> Having only parts of a register defined sometimes makes people miss
> that they can't just write the whole thing because there may be other
> bits they do not indent to change. As such it can be useful to have them there.

Hi Jonathan,

I was under the impression that it was good practice to remove unused
code (including macros) for the purpose of reducing clutter. Hearing
your thoughts, I see why it would be better to leave them, despite
being unused.

Please disregard this patch.

Thanks for your feedback,
Michael

On Sat, Jan 31, 2026 at 7:54 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 30 Jan 2026 23:38:27 -0800
> Michael Harris <michaelharriscode@gmail.com> wrote:
>
> > Delete macros that aren't in use
> Hi Michael,
>
> Why?
>
> I'm not against driver authors only introducing register defines they
> are using, but I'm also not against them deciding on comprehensive defines
> or at least making sure they cover all the fields in a given register.
>
> Having only parts of a register defined sometimes makes people miss
> that they can't just write the whole thing because there may be other
> bits they do not indent to change. As such it can be useful to have them there.
>
> Thanks,
>
> Jonathan
>
>
> >
> > Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
> > ---
> >  drivers/staging/iio/addac/adt7316.c | 11 -----------
> >  1 file changed, 11 deletions(-)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> > index 8a9a8262c2bec34f3c3e79d8174f492b9a23fb70..4173c8822fff495e8c69d9cf6c11be9e9227a8c1 100644
> > --- a/drivers/staging/iio/addac/adt7316.c
> > +++ b/drivers/staging/iio/addac/adt7316.c
> > @@ -111,7 +111,6 @@
> >  #define ADT7316_AD_SINGLE_CH_MODE    0x10
> >  #define ADT7316_DISABLE_AVERAGING    0x20
> >  #define ADT7316_EN_SMBUS_TIMEOUT     0x40
> > -#define ADT7316_RESET                        0x80
> >
> >  /*
> >   * ADT7316 config3
> > @@ -154,7 +153,6 @@
> >  /*
> >   * ADT7316 value masks
> >   */
> > -#define ADT7316_VALUE_MASK           0xfff
> >  #define ADT7316_T_VALUE_SIGN         0x400
> >  #define ADT7316_T_VALUE_FLOAT_OFFSET 2
> >  #define ADT7316_T_VALUE_FLOAT_MASK   0x2
> > @@ -194,15 +192,6 @@ struct adt7316_chip_info {
> >   * Logic interrupt mask for user application to enable
> >   * interrupts.
> >   */
> > -#define ADT7316_IN_TEMP_HIGH_INT_MASK        0x1
> > -#define ADT7316_IN_TEMP_LOW_INT_MASK 0x2
> > -#define ADT7316_EX_TEMP_HIGH_INT_MASK        0x4
> > -#define ADT7316_EX_TEMP_LOW_INT_MASK 0x8
> > -#define ADT7316_EX_TEMP_FAULT_INT_MASK       0x10
> > -#define ADT7516_AIN1_INT_MASK                0x4
> > -#define ADT7516_AIN2_INT_MASK                0x20
> > -#define ADT7516_AIN3_INT_MASK                0x40
> > -#define ADT7516_AIN4_INT_MASK                0x80
> >  #define ADT7316_VDD_INT_MASK         0x100
> >  #define ADT7316_TEMP_INT_MASK                0x1F
> >  #define ADT7516_AIN_INT_MASK         0xE0
> >
>

[PATCH 2/2] staging: iio: adt7316: convert magic numbers to BIT() and GENMASK()

[Michael Harris]

Improve readability by converting raw hex macros to use BIT() or GENMASK()
instead.

Update a few sysfs_emit() string formats to expect the unsigned long type
given by BIT() and GENMASK() and prevent compiler errors.

Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
---
 drivers/staging/iio/addac/adt7316.c | 80 ++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
index 4173c8822fff495e8c69d9cf6c11be9e9227a8c1..0af7e18ff9684993622a268110f0a17c0bff3616 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -29,11 +29,11 @@
 #define ADT7316_INT_STAT1		0x0
 #define ADT7316_INT_STAT2		0x1
 #define ADT7316_LSB_IN_TEMP_VDD		0x3
-#define ADT7316_LSB_IN_TEMP_MASK	0x3
-#define ADT7316_LSB_VDD_MASK		0xC
+#define ADT7316_LSB_IN_TEMP_MASK	GENMASK(1, 0)
+#define ADT7316_LSB_VDD_MASK		GENMASK(3, 2)
 #define ADT7316_LSB_VDD_OFFSET		2
 #define ADT7316_LSB_EX_TEMP_AIN		0x4
-#define ADT7316_LSB_EX_TEMP_MASK	0x3
+#define ADT7316_LSB_EX_TEMP_MASK	GENMASK(1, 0)
 #define ADT7516_LSB_AIN_SHIFT		2
 #define ADT7316_AD_MSB_DATA_BASE        0x6
 #define ADT7316_AD_MSB_DATA_REGS        3
@@ -88,19 +88,19 @@
 /*
  * ADT7316 config1
  */
-#define ADT7316_EN			0x1
-#define ADT7516_SEL_EX_TEMP		0x4
-#define ADT7516_SEL_AIN1_2_EX_TEMP_MASK	0x6
-#define ADT7516_SEL_AIN3		0x8
-#define ADT7316_INT_EN			0x20
-#define ADT7316_INT_POLARITY		0x40
-#define ADT7316_PD			0x80
+#define ADT7316_EN			BIT(0)
+#define ADT7516_SEL_EX_TEMP		BIT(2)
+#define ADT7516_SEL_AIN1_2_EX_TEMP_MASK	GENMASK(2, 1)
+#define ADT7516_SEL_AIN3		BIT(3)
+#define ADT7316_INT_EN			BIT(5)
+#define ADT7316_INT_POLARITY		BIT(6)
+#define ADT7316_PD			BIT(7)
 
 /*
  * ADT7316 config2
  */
-#define ADT7316_AD_SINGLE_CH_MASK	0x3
-#define ADT7516_AD_SINGLE_CH_MASK	0x7
+#define ADT7316_AD_SINGLE_CH_MASK	GENMASK(1, 0)
+#define ADT7516_AD_SINGLE_CH_MASK	GENMASK(2, 0)
 #define ADT7316_AD_SINGLE_CH_VDD	0
 #define ADT7316_AD_SINGLE_CH_IN		1
 #define ADT7316_AD_SINGLE_CH_EX		2
@@ -108,54 +108,54 @@
 #define ADT7516_AD_SINGLE_CH_AIN2	3
 #define ADT7516_AD_SINGLE_CH_AIN3	4
 #define ADT7516_AD_SINGLE_CH_AIN4	5
-#define ADT7316_AD_SINGLE_CH_MODE	0x10
-#define ADT7316_DISABLE_AVERAGING	0x20
-#define ADT7316_EN_SMBUS_TIMEOUT	0x40
+#define ADT7316_AD_SINGLE_CH_MODE	BIT(4)
+#define ADT7316_DISABLE_AVERAGING	BIT(5)
+#define ADT7316_EN_SMBUS_TIMEOUT	BIT(6)
 
 /*
  * ADT7316 config3
  */
-#define ADT7316_ADCLK_22_5		0x1
-#define ADT7316_DA_HIGH_RESOLUTION	0x2
-#define ADT7316_DA_EN_VIA_DAC_LDAC	0x8
-#define ADT7516_AIN_IN_VREF		0x10
-#define ADT7316_EN_IN_TEMP_PROP_DACA	0x20
-#define ADT7316_EN_EX_TEMP_PROP_DACB	0x40
+#define ADT7316_ADCLK_22_5		BIT(0)
+#define ADT7316_DA_HIGH_RESOLUTION	BIT(1)
+#define ADT7316_DA_EN_VIA_DAC_LDAC	BIT(3)
+#define ADT7516_AIN_IN_VREF		BIT(4)
+#define ADT7316_EN_IN_TEMP_PROP_DACA	BIT(5)
+#define ADT7316_EN_EX_TEMP_PROP_DACB	BIT(6)
 
 /*
  * ADT7316 DAC config
  */
-#define ADT7316_DA_2VREF_CH_MASK	0xF
-#define ADT7316_DA_EN_MODE_MASK		0x30
+#define ADT7316_DA_2VREF_CH_MASK	GENMASK(3, 0)
+#define ADT7316_DA_EN_MODE_MASK		GENMASK(5, 4)
 #define ADT7316_DA_EN_MODE_SHIFT	4
 #define ADT7316_DA_EN_MODE_SINGLE	0x00
 #define ADT7316_DA_EN_MODE_AB_CD	0x10
 #define ADT7316_DA_EN_MODE_ABCD		0x20
 #define ADT7316_DA_EN_MODE_LDAC		0x30
-#define ADT7316_VREF_BYPASS_DAC_AB	0x40
-#define ADT7316_VREF_BYPASS_DAC_CD	0x80
+#define ADT7316_VREF_BYPASS_DAC_AB	BIT(6)
+#define ADT7316_VREF_BYPASS_DAC_CD	BIT(7)
 
 /*
  * ADT7316 LDAC config
  */
-#define ADT7316_LDAC_EN_DA_MASK		0xF
-#define ADT7316_DAC_IN_VREF		0x10
-#define ADT7516_DAC_AB_IN_VREF		0x10
-#define ADT7516_DAC_CD_IN_VREF		0x20
+#define ADT7316_LDAC_EN_DA_MASK		GENMASK(3, 0)
+#define ADT7316_DAC_IN_VREF		BIT(4)
+#define ADT7516_DAC_AB_IN_VREF		BIT(4)
+#define ADT7516_DAC_CD_IN_VREF		BIT(5)
 #define ADT7516_DAC_IN_VREF_OFFSET	4
-#define ADT7516_DAC_IN_VREF_MASK	0x30
+#define ADT7516_DAC_IN_VREF_MASK	GENMASK(5, 4)
 
 /*
  * ADT7316 INT_MASK2
  */
-#define ADT7316_INT_MASK2_VDD		0x10
+#define ADT7316_INT_MASK2_VDD		BIT(4)
 
 /*
  * ADT7316 value masks
  */
-#define ADT7316_T_VALUE_SIGN		0x400
+#define ADT7316_T_VALUE_SIGN		BIT(10)
 #define ADT7316_T_VALUE_FLOAT_OFFSET	2
-#define ADT7316_T_VALUE_FLOAT_MASK	0x2
+#define ADT7316_T_VALUE_FLOAT_MASK	BIT(1)
 
 /*
  * Chip ID
@@ -167,7 +167,7 @@
 #define ID_ADT7517		0x12
 #define ID_ADT7519		0x14
 
-#define ID_FAMILY_MASK		0xF0
+#define ID_FAMILY_MASK		GENMASK(7, 4)
 #define ID_ADT73XX		0x0
 #define ID_ADT75XX		0x10
 
@@ -192,9 +192,9 @@ struct adt7316_chip_info {
  * Logic interrupt mask for user application to enable
  * interrupts.
  */
-#define ADT7316_VDD_INT_MASK		0x100
-#define ADT7316_TEMP_INT_MASK		0x1F
-#define ADT7516_AIN_INT_MASK		0xE0
+#define ADT7316_VDD_INT_MASK		BIT(8)
+#define ADT7316_TEMP_INT_MASK		GENMASK(4, 0)
+#define ADT7516_AIN_INT_MASK		GENMASK(7, 5)
 #define ADT7316_TEMP_AIN_INT_MASK	\
 	(ADT7316_TEMP_INT_MASK)
 
@@ -783,7 +783,7 @@ static ssize_t adt7316_show_DAC_2Vref_ch_mask(struct device *dev,
 	struct iio_dev *dev_info = dev_to_iio_dev(dev);
 	struct adt7316_chip_info *chip = iio_priv(dev_info);
 
-	return sysfs_emit(buf, "0x%x\n",
+	return sysfs_emit(buf, "0x%lx\n",
 		chip->dac_config & ADT7316_DA_2VREF_CH_MASK);
 }
 
@@ -1023,7 +1023,7 @@ static ssize_t adt7316_show_DAC_internal_Vref(struct device *dev,
 	struct adt7316_chip_info *chip = iio_priv(dev_info);
 
 	if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)
-		return sysfs_emit(buf, "0x%x\n",
+		return sysfs_emit(buf, "0x%lx\n",
 			(chip->ldac_config & ADT7516_DAC_IN_VREF_MASK) >>
 			ADT7516_DAC_IN_VREF_OFFSET);
 	return sysfs_emit(buf, "%d\n",
@@ -1146,7 +1146,7 @@ static ssize_t adt7316_show_ad(struct adt7316_chip_info *chip,
 		sign = '-';
 	}
 
-	return sysfs_emit(buf, "%c%d.%.2d\n", sign,
+	return sysfs_emit(buf, "%c%d.%.2ld\n", sign,
 		(data >> ADT7316_T_VALUE_FLOAT_OFFSET),
 		(data & ADT7316_T_VALUE_FLOAT_MASK) * 25);
 }

-- 
2.52.0

Re: [PATCH 2/2] staging: iio: adt7316: convert magic numbers to BIT() and GENMASK()

[Jonathan Cameron]

On Fri, 30 Jan 2026 23:38:28 -0800
Michael Harris <michaelharriscode@gmail.com> wrote:

> Improve readability by converting raw hex macros to use BIT() or GENMASK()
> instead.
> 
> Update a few sysfs_emit() string formats to expect the unsigned long type
> given by BIT() and GENMASK() and prevent compiler errors.
> 
> Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
> ---
>  drivers/staging/iio/addac/adt7316.c | 80 ++++++++++++++++++-------------------
>  1 file changed, 40 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> index 4173c8822fff495e8c69d9cf6c11be9e9227a8c1..0af7e18ff9684993622a268110f0a17c0bff3616 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -29,11 +29,11 @@


> @@ -88,19 +88,19 @@
>  /*
>   * ADT7316 config1
>   */
> -#define ADT7316_EN			0x1
> -#define ADT7516_SEL_EX_TEMP		0x4
> -#define ADT7516_SEL_AIN1_2_EX_TEMP_MASK	0x6
> -#define ADT7516_SEL_AIN3		0x8
> -#define ADT7316_INT_EN			0x20
> -#define ADT7316_INT_POLARITY		0x40
> -#define ADT7316_PD			0x80
> +#define ADT7316_EN			BIT(0)
Arguably its a separate change but I'd much prefer if these defines
included what register they were found in. That is things like

#define ADT7316_CONFIG1_EN
etc. It makes it a lot easier to spot mismatches in the code.

Also this define sounds like it would be general enable / disable of capture
but it seems to only be about monitoring modes.

> +#define ADT7516_SEL_EX_TEMP		BIT(2)
That's not a mask.  It is a value and it should clearly placed
in the relevant field, so you can do things like
FIELD_PREP(ADT7516_SEL_AIN1_2_EX_TEMP_MASK, ADT7516...TEMP)
So it should take the value 2 == b10 not BIT(2).

> +#define ADT7516_SEL_AIN1_2_EX_TEMP_MASK	GENMASK(2, 1)
I'd put the masks before the values. 

> +#define ADT7516_SEL_AIN3		BIT(3)
> +#define ADT7316_INT_EN			BIT(5)
> +#define ADT7316_INT_POLARITY		BIT(6)
> +#define ADT7316_PD			BIT(7)
>  
>  /*
>   * ADT7316 config2
>   */
> -#define ADT7316_AD_SINGLE_CH_MASK	0x3
> -#define ADT7516_AD_SINGLE_CH_MASK	0x7
> +#define ADT7316_AD_SINGLE_CH_MASK	GENMASK(1, 0)
> +#define ADT7516_AD_SINGLE_CH_MASK	GENMASK(2, 0)
>  #define ADT7316_AD_SINGLE_CH_VDD	0
>  #define ADT7316_AD_SINGLE_CH_IN		1
>  #define ADT7316_AD_SINGLE_CH_EX		2
> @@ -108,54 +108,54 @@
>  #define ADT7516_AD_SINGLE_CH_AIN2	3
>  #define ADT7516_AD_SINGLE_CH_AIN3	4
>  #define ADT7516_AD_SINGLE_CH_AIN4	5
> -#define ADT7316_AD_SINGLE_CH_MODE	0x10
> -#define ADT7316_DISABLE_AVERAGING	0x20
> -#define ADT7316_EN_SMBUS_TIMEOUT	0x40
> +#define ADT7316_AD_SINGLE_CH_MODE	BIT(4)
> +#define ADT7316_DISABLE_AVERAGING	BIT(5)
> +#define ADT7316_EN_SMBUS_TIMEOUT	BIT(6)
As commented on previous patch I'd prefer a full register definition
even if a few bits aren't used. Makes it easier to spot when they
are being accidentally cleared.

>  
>  /*
>   * ADT7316 config3
>   */
> -#define ADT7316_ADCLK_22_5		0x1
> -#define ADT7316_DA_HIGH_RESOLUTION	0x2
> -#define ADT7316_DA_EN_VIA_DAC_LDAC	0x8
> -#define ADT7516_AIN_IN_VREF		0x10
> -#define ADT7316_EN_IN_TEMP_PROP_DACA	0x20
> -#define ADT7316_EN_EX_TEMP_PROP_DACB	0x40
> +#define ADT7316_ADCLK_22_5		BIT(0)
> +#define ADT7316_DA_HIGH_RESOLUTION	BIT(1)
> +#define ADT7316_DA_EN_VIA_DAC_LDAC	BIT(3)
> +#define ADT7516_AIN_IN_VREF		BIT(4)
> +#define ADT7316_EN_IN_TEMP_PROP_DACA	BIT(5)
> +#define ADT7316_EN_EX_TEMP_PROP_DACB	BIT(6)
>  
>  /*
>   * ADT7316 DAC config
>   */
> -#define ADT7316_DA_2VREF_CH_MASK	0xF
> -#define ADT7316_DA_EN_MODE_MASK		0x30
> +#define ADT7316_DA_2VREF_CH_MASK	GENMASK(3, 0)
> +#define ADT7316_DA_EN_MODE_MASK		GENMASK(5, 4)
>  #define ADT7316_DA_EN_MODE_SHIFT	4
>  #define ADT7316_DA_EN_MODE_SINGLE	0x00
>  #define ADT7316_DA_EN_MODE_AB_CD	0x10
>  #define ADT7316_DA_EN_MODE_ABCD		0x20
>  #define ADT7316_DA_EN_MODE_LDAC		0x30
> -#define ADT7316_VREF_BYPASS_DAC_AB	0x40
> -#define ADT7316_VREF_BYPASS_DAC_CD	0x80
> +#define ADT7316_VREF_BYPASS_DAC_AB	BIT(6)
> +#define ADT7316_VREF_BYPASS_DAC_CD	BIT(7)
>  
>  /*
>   * ADT7316 LDAC config
>   */
> -#define ADT7316_LDAC_EN_DA_MASK		0xF
> -#define ADT7316_DAC_IN_VREF		0x10
> -#define ADT7516_DAC_AB_IN_VREF		0x10
> -#define ADT7516_DAC_CD_IN_VREF		0x20
> +#define ADT7316_LDAC_EN_DA_MASK		GENMASK(3, 0)
> +#define ADT7316_DAC_IN_VREF		BIT(4)
> +#define ADT7516_DAC_AB_IN_VREF		BIT(4)
> +#define ADT7516_DAC_CD_IN_VREF		BIT(5)
>  #define ADT7516_DAC_IN_VREF_OFFSET	4
Try and get rid of separate shifts / offsets.  (I think this
is just a shift?)  Replace them with use of FIELD_GET()/ FIELD_PREP() and
the mask.

A precursor patch doing that might make this patch easier to
read as all the shifts would likely be gone.

> -#define ADT7516_DAC_IN_VREF_MASK	0x30
> +#define ADT7516_DAC_IN_VREF_MASK	GENMASK(5, 4)
>  
>  /*
>   * ADT7316 INT_MASK2
>   */
> -#define ADT7316_INT_MASK2_VDD		0x10
> +#define ADT7316_INT_MASK2_VDD		BIT(4)
>  
>  /*
>   * ADT7316 value masks
>   */
> -#define ADT7316_T_VALUE_SIGN		0x400
> +#define ADT7316_T_VALUE_SIGN		BIT(10)

From a quick glance, these are all 2's complement.
As such, using sign_extend32() or similar would be much nicer
than the custom sign manipulation in here and the need for
this macro would probably go away.

>  #define ADT7316_T_VALUE_FLOAT_OFFSET	2
> -#define ADT7316_T_VALUE_FLOAT_MASK	0x2
> +#define ADT7316_T_VALUE_FLOAT_MASK	BIT(1)
The usage of these is also rather strange and more about
a fixed point maths conversion than anything related to registers.
I'd expect that to get cleaned up by reporting temperature as _raw
and providing an appropriate scale.

>  
>  /*
>   * Chip ID
> @@ -167,7 +167,7 @@
>  #define ID_ADT7517		0x12
>  #define ID_ADT7519		0x14
>  
> -#define ID_FAMILY_MASK		0xF0
> +#define ID_FAMILY_MASK		GENMASK(7, 4)
>  #define ID_ADT73XX		0x0
>  #define ID_ADT75XX		0x10
>  
> @@ -192,9 +192,9 @@ struct adt7316_chip_info {
>   * Logic interrupt mask for user application to enable
>   * interrupts.
>   */
> -#define ADT7316_VDD_INT_MASK		0x100
> -#define ADT7316_TEMP_INT_MASK		0x1F
> -#define ADT7516_AIN_INT_MASK		0xE0
> +#define ADT7316_VDD_INT_MASK		BIT(8)
> +#define ADT7316_TEMP_INT_MASK		GENMASK(4, 0)
> +#define ADT7516_AIN_INT_MASK		GENMASK(7, 5)
>  #define ADT7316_TEMP_AIN_INT_MASK	\
>  	(ADT7316_TEMP_INT_MASK)
>  
> @@ -783,7 +783,7 @@ static ssize_t adt7316_show_DAC_2Vref_ch_mask(struct device *dev,
>  	struct iio_dev *dev_info = dev_to_iio_dev(dev);
>  	struct adt7316_chip_info *chip = iio_priv(dev_info);
>  
> -	return sysfs_emit(buf, "0x%x\n",
> +	return sysfs_emit(buf, "0x%lx\n",
>  		chip->dac_config & ADT7316_DA_2VREF_CH_MASK);
Is the compiler complaining about these? It really should be able to tell that the masks
are small enough that the original can always print the right thing.

>  }
>  
> @@ -1023,7 +1023,7 @@ static ssize_t adt7316_show_DAC_internal_Vref(struct device *dev,
>  	struct adt7316_chip_info *chip = iio_priv(dev_info);
>  
>  	if ((chip->id & ID_FAMILY_MASK) == ID_ADT75XX)
> -		return sysfs_emit(buf, "0x%x\n",
> +		return sysfs_emit(buf, "0x%lx\n",
>  			(chip->ldac_config & ADT7516_DAC_IN_VREF_MASK) >>
>  			ADT7516_DAC_IN_VREF_OFFSET);
>  	return sysfs_emit(buf, "%d\n",
> @@ -1146,7 +1146,7 @@ static ssize_t adt7316_show_ad(struct adt7316_chip_info *chip,
>  		sign = '-';
>  	}
>  
> -	return sysfs_emit(buf, "%c%d.%.2d\n", sign,
> +	return sysfs_emit(buf, "%c%d.%.2ld\n", sign,
>  		(data >> ADT7316_T_VALUE_FLOAT_OFFSET),
>  		(data & ADT7316_T_VALUE_FLOAT_MASK) * 25);
I mention this above.  This calculation is odd.  I'd just expect the maths to be done
with a multiplier, probably 100 such that the 1/4 per bit after the decimal point
gets buried in that and you can do a simple % 100 here to get the decimal places.

I've asked for a couple of different types of change in here. It is likely
that some of them should be in separate patches.

Thanks,

Jonathan

>  }
> 

Re: [PATCH 2/2] staging: iio: adt7316: convert magic numbers to BIT() and GENMASK()

[Michael Harris]

On 1/31/26 8:21 AM, Jonathan Cameron wrote:
> I've asked for a couple of different types of change in here. It is likely
> that some of them should be in separate patches.
> 
> Thanks,
> 
> Jonathan
Thank you for the detailed review. I am working on a v2 series to
address your feedback and will send it when ready.

Thanks,
Michael

Re: [PATCH 2/2] staging: iio: adt7316: convert magic numbers to BIT() and GENMASK()

[David Laight]

On Sat, 31 Jan 2026 16:21:40 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 30 Jan 2026 23:38:28 -0800
> Michael Harris <michaelharriscode@gmail.com> wrote:
.. 
> > -	return sysfs_emit(buf, "0x%x\n",
> > +	return sysfs_emit(buf, "0x%lx\n",
> >  		chip->dac_config & ADT7316_DA_2VREF_CH_MASK);  
> Is the compiler complaining about these? It really should be able to tell that the masks
> are small enough that the original can always print the right thing.

That will be because BIT() and GENMASK() generate 'unsigned long' values.
Sort of silly when they are used for hardware registers that are fixed width.
You can use BIT_U32() and GENMASK_U32() instead - they will be
'unsigned int'.

	David