Re: [PATCH 3/6] hwmon: (max6697) Use bit operations where possible

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Jul 23, 2024 at 05:37:25AM +0000, Tzung-Bi Shih wrote:
> On Mon, Jul 22, 2024 at 05:52:01PM -0700, Guenter Roeck wrote:
> > @@ -32,20 +34,31 @@ static const u8 MAX6697_REG_CRIT[] = {
> >  			0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27 };
> >  
> >  /*
> > - * Map device tree / platform data register bit map to chip bit map.
> > + * Map device tree / internal register bit map to chip bit map.
> >   * Applies to alert register and over-temperature register.
> >   */
> > +
> > +#define MAX6697_EXTERNAL_MASK_DT	GENMASK(7, 1)
> > +#define MAX6697_LOCAL_MASK_DT		BIT(0)
> > +#define MAX6697_EXTERNAL_MASK_CHIP	GENMASK(6, 0)
> > +#define MAX6697_LOCAL_MASK_CHIP		BIT(7)
> > +
> > +/* alert - local channel is in bit 6 */
> >  #define MAX6697_ALERT_MAP_BITS(reg)	((((reg) & 0x7e) >> 1) | \
> >  				 (((reg) & 0x01) << 6) | ((reg) & 0x80))
> > -#define MAX6697_OVERT_MAP_BITS(reg) (((reg) >> 1) | (((reg) & 0x01) << 7))
> > +
> > +/* over-temperature - local channel is in bit 7 */
> > +#define MAX6697_OVERT_MAP_BITS(reg)	\
> > +	(FIELD_PREP(MAX6697_EXTERNAL_MASK_CHIP, FIELD_GET(MAX6697_EXTERNAL_MASK_DT, reg)) | \
> > +	 FIELD_PREP(MAX6697_LOCAL_MASK_CHIP, FIELD_GET(MAX6697_LOCAL_MASK_DT, reg)))
> 
> How about:
> #define MAX6697_OVERT_MAP_BITS(reg)	\
>     (FIELD_GET(MAX6697_EXTERNAL_MASK_DT, reg) | \
>      FIELD_PREP(MAX6697_LOCAL_MASK_CHIP, reg))
> 

I don't think that works because FIELD_PREP validates that reg does not
have bits set outside the mask. Either case, I prefer to keep the more
complex version, though, because it is more complete. The generated code
should hopefully be the same.

Thanks,
Guenter