On Thu, Aug 08, 2024 at 07:46:36AM +0530, Abhishek Tamboli wrote: > On Wed, Aug 07, 2024 at 11:38:34AM -0700, Guenter Roeck wrote: > Hi Guenter, > Thank you for your feedback. > > On 8/7/24 11:17, Abhishek Tamboli wrote: > > > Fix the issue of lm93_read_byte() and lm93_read_word() return 0 on > > > read failure after retries, which could be confused with valid data. > > > > > > Address the TODO: what to return in case of error? > > > > > > Signed-off-by: Abhishek Tamboli <abhishektamboli9@xxxxxxxxx> > > > --- > > > drivers/hwmon/lm93.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/hwmon/lm93.c b/drivers/hwmon/lm93.c > > > index be4853fad80f..b76f3c1c6297 100644 > > > --- a/drivers/hwmon/lm93.c > > > +++ b/drivers/hwmon/lm93.c > > > @@ -798,6 +798,7 @@ static unsigned LM93_ALARMS_FROM_REG(struct block1_t b1) > > > static u8 lm93_read_byte(struct i2c_client *client, u8 reg) > > > > This is still returning an u8. > My interpretation of the TODO was to address the error condition while keeping the > existing logic of the driver intact. I understand that this driver is > old and that changes should be approached with caution. Those TODOs are, at this point, pretty much pointless. If you want to help with improving kernel code, it might be better to pick something from the drivers/staging/ directory and help improve it. The only thing that would really help for the lm93 driver would be a complete overhaul, and that would only make sense if someone has real hardware to test the resulting code; the driver is too complex to just rely on unit tests. For example, the excessive retries might be because the chip is really bad with its communication, or it may have been observed on a system with a bad i2c controller, making it completely unnecesssary today. Either case, if those retries are really necessary due to chip issues, they should be hiddden behind regmap (which should also be used to replace in-driver caching). And so on. Really, if you want to get into kenrel development, it would be much better to help improving code which is actually being used, as mentioned above. Thanks, Guenter
