Re: [PATCH 1/1] hwmon: (adt7475) Added attenuator bypass support

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

On Wed, Dec 18, 2019 at 04:30:39AM +0000, Logan Shaw wrote:
[...]
> > 
> > The datasheet for ADC7475 does not say anything about the ability to
> > control
> > attenuators other than for vcc in configuration register 4. Bit 4, 6,
> > and 7
> > are listed as unused/reserved, suggesting that those bits - if at all
> > - are
> > only defined for other chips. Nothing in this patch suggests what
> > those chips
> > are. Attenuation bits need to be validated against the chip type.
> 
> You are right, I missed including important details. The ADT7476 and
> ADT7490 datasheets specify "Bits [7:4] of Configuration Register 4
> (0x7D) can be used to bypass individual voltage channel attenuators".
> 
> My thought process was it would be up to the person configuring the
> devicetree to only add the attributes where appropiate (for example,
> not for a ADT7475 chip). I can see this is dangerious. Instead would it
> be acceptable to add a check to the load_individual_bypass_attenuators
> and load_all_bypass_attenuator functions that verifies the device
> supports setting the appropiate bits and if not return 0 immediately?
> 

Devicetree properties are acceptable, but not writing bits which
are not supported (reserved) for a given chip. How to implement this
is up to you.

Based on your feedback, and my personal opinion, I won't accept
new (non-standard) sysfs attributes. Note that I also won't accept
C++ style comments in this driver.

Guenter