On Sun, Aug 20, 2023 at 05:36:02PM +0000, Biju Das wrote: > Hi Guenter Roeck, > > > Subject: RE: [PATCH 0/2] Convert enum->pointer for data in the tmp51x match > > tables > > > > Hi Guenter Roeck, > > > > Thanks for the feedback. > > > > > Subject: Re: [PATCH 0/2] Convert enum->pointer for data in the tmp51x > > > match tables > > > > > > Either case, I would want to keep temp_config and the number of > > > channels in struct tmp51x_data to avoid repeated double dereferences > > > and because temp_config could change (resistance correction > > > enabled/disabled should be a devicetree property, conversion rate as > > > well as channel enable should be sysfs attributes, and channel > > > enable/disable should also be devicetree properties). The value could > > > also be used to support suspend/resume in the driver if someone wants to > > add that at some point. > > > > > > In this context, > > > if (data->id == tmp512 && channel == 4) > > > return 0; > > > is wrong because there are 3 or 4 channels, meaning the channel > > > numbers are > > > 0..3 and there is no channel 4. This should be "channel == 3" > > > to disable the 4th channel on tmp512. Interesting that no one > > > (including me ;-) noticed this. > > > > If I am correct, as per above, only max number channels supported by the > > chip can go to device data. That is the only HW difference between these 2 > > chips and other chip specific data can be derived from this. > > I guess, the below initial values for temp_config > is chip specific apart from the number of supported channels > as we won't be able to derive from number of channels?? > > #define TMP512_TEMP_CONFIG_DEFAULT 0xBF80 > #define TMP513_TEMP_CONFIG_DEFAULT 0xFF80 > Bit 3-6 enable temperature sensors. The difference in bit 6 is that the 3rd external temperature sensor (which only exists for tmp513) is enabled if the bit is set. So the base value for TMP513_TEMP_CONFIG_DEFAULT would be 0xBF80 with bit 6 set if the chip has four temperature sensors. Guenter
