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 Cheers, Biju
