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. 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 > > ��.n��������+%������w��{.n�����{��&��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�