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
