Hi Guenter-san, Thank you so much for your comments. Oh I see and actually we have some I2C error units in our products. The patch is for the error case mainly. But I think that sometimes the I2C error is still caused on the normal units also actually. I am not sure about that the normal units error behavior is caused by the board (I2C controller) or the chip ADT7476A but probably the former but not sure. So should these patches not be applied to the adt7475 driver as you mentioned? At first I thought that the I2C SMBus APIs return error codes but adt7475 driver does not check so it should be checked the error codes. Is this not correct? To make sure let me confirm above. Regards, Ikegami > -----Original Message----- > From: linux-hwmon-owner@xxxxxxxxxxxxxxx > [mailto:linux-hwmon-owner@xxxxxxxxxxxxxxx] On Behalf Of Guenter Roeck > Sent: Thursday, July 12, 2018 12:42 PM > To: IKEGAMI Tokunori; Jean Delvare > Cc: PACKHAM Chris; linux-hwmon@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 3/5] hwmon: adt7475: Change to use adt7475_read macro > > On 07/11/2018 07:52 PM, IKEGAMI Tokunori wrote: > > Hi Guenter-san, > > > > Thank you so much for your comments. > > Okay now I am thinking to change the adt7475_read macro to a function > to repeat for the error case. > > If you have any comment about this please let me know. > > Yes - we should only do this if it is known to be a chip problem. > Patching the chip driver for a board (or i2c controller) problem > is not appropriate. > > Guenter > > > Anyway I will do send v2 version patches later. > > > > Regards, > > Ikegami > > > >> -----Original Message----- > >> From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter > >> Roeck > >> Sent: Thursday, July 12, 2018 10:50 AM > >> To: IKEGAMI Tokunori; Jean Delvare > >> Cc: PACKHAM Chris; linux-hwmon@xxxxxxxxxxxxxxx > >> Subject: Re: [PATCH 3/5] hwmon: adt7475: Change to use adt7475_read > macro > >> > >> On 07/11/2018 06:04 PM, Tokunori Ikegami wrote: > >>> It shoudl be same as with other functions to use adt7475_read. > >>> So change to use it instead of i2c_smbus_read_byte_data. > >>> > >> > >> I don't see a point in this change. Replacing a function name doesn't > >> make > >> the code easier to read. If anything, you could consider dropping > >> adt7475_read > >> and calling i2c_smbus_read_byte_data() directly. > >> > >> Guenter > >> > >>> Signed-off-by: Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx> > >>> Cc: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> > >>> Cc: linux-hwmon@xxxxxxxxxxxxxxx > >>> --- > >>> drivers/hwmon/adt7475.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c > >>> index a40eb62ee6b1..bad250729e99 100644 > >>> --- a/drivers/hwmon/adt7475.c > >>> +++ b/drivers/hwmon/adt7475.c > >>> @@ -1012,7 +1012,7 @@ static ssize_t > >> pwm_use_point2_pwm_at_crit_store(struct device *dev, > >>> return -EINVAL; > >>> > >>> mutex_lock(&data->lock); > >>> - data->config4 = i2c_smbus_read_byte_data(client, > >> REG_CONFIG4); > >>> + data->config4 = adt7475_read(REG_CONFIG4); > >>> if (val) > >>> data->config4 |= CONFIG4_MAXDUTY; > >>> else > >>> > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-hwmon" > in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{�� &��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
