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. Guenter
