Re: Questions about adt7470 driver

["Darrick J. Wong" <darrick.wong@oracle.com>]

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