On Wed, May 27, 2020 at 03:42:52PM -0700, Guenter Roeck wrote: > On Tue, May 26, 2020 at 11:22:59AM +0200, Jean Delvare wrote: > > Hi all, > > > > In the context of bug #207771, I got to look into the adt7470 driver. > > I'm slowing understanding the logic of the background temperature > > registers update thread, still there are 2 things I do not understand: > > > > 1* Function adt7470_read_temperatures() sets data->num_temp_sensors, > > however this value seems to be only used to limit the wait time of > > future calls to the same function. In the general update function we > > still read ALL temperature sensors regardless of its value: > > > > for (i = 0; i < ADT7470_TEMP_COUNT; i++) > > data->temp[i] = i2c_smbus_read_byte_data(client, > > ADT7470_TEMP_REG(i)); > > > > Shouldn't this loop be bounded with data->num_temp_sensors instead of > > ADT7470_TEMP_COUNT? > > > Guess so. > > > 2* Function adt7470_read_temperatures() also sets > > data->temperatures_probed to 1, and this boolean is then used to skip > > further calls to that function. But do we really need a separate > > variable for this, given that num_temp_sensors >= 0 matches the same > > condition as far as I can see? > > > Agreed. On the other side, those are optimizations. I am not really sure > if that driver is worth spending time on, given the age of the chip. I /think/ the answer to both questions is yes, but I don't have the hardware anymore so I have no way to QA that... :/ --D > Guenter
