Re: [PATCH 1/2] hwmon: (lm90) split set&show temp as common codes

[Guenter Roeck]

On Tue, Feb 25, 2014 at 05:21:40PM +0800, Wei Ni wrote:
> On 02/25/2014 04:57 PM, Guenter Roeck wrote:
> > On 02/25/2014 12:21 AM, Wei Ni wrote:
> >> On 02/25/2014 02:32 PM, Guenter Roeck wrote:
> >>> On 02/24/2014 10:21 PM, Wei Ni wrote:
> >>>> Split set&show temp codes as common functions, so we can use it
> >>>> directly when implement linux thermal framework.
> >>>> And handle error return value for the lm90_select_remote_channel
> >>>> and write_tempx, then set_temp8 and set_temp11 could return it
> >>>> to user-space.
> >>>>
> >>>> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
> >>>> Signed-off-by: Jean Delvare <khali@xxxxxxxxxxxx>
> >>>> ---
> >>>>    drivers/hwmon/lm90.c |  170 ++++++++++++++++++++++++++++++++++----------------
> >>>>    1 file changed, 115 insertions(+), 55 deletions(-)
> >>>>
> >>>> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> >>>> index c9ff08d..fb9e224 100644
> >>>> --- a/drivers/hwmon/lm90.c
> >>>> +++ b/drivers/hwmon/lm90.c
> >>>> @@ -473,20 +473,29 @@ static int lm90_read16(struct i2c_client *client, u8 regh, u8 regl, u16 *value)
> >>>>     * various registers have different meanings as a result of selecting a
> >>>>     * non-default remote channel.
> >>>>     */
> >>>> -static inline void lm90_select_remote_channel(struct i2c_client *client,
> >>>> -					      struct lm90_data *data,
> >>>> -					      int channel)
> >>>> +static inline int lm90_select_remote_channel(struct i2c_client *client,
> >>>> +					     struct lm90_data *data,
> >>>> +					     int channel)
> >>>>    {
> >>>>    	u8 config;
> >>>> +	int err;
> >>>>
> >>>>    	if (data->kind == max6696) {
> >>>>    		lm90_read_reg(client, LM90_REG_R_CONFIG1, &config);
> >>>>    		config &= ~0x08;
> >>>>    		if (channel)
> >>>>    			config |= 0x08;
> >>>> -		i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> >>>> -					  config);
> >>>> +		err = i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
> >>>> +						config);
> >>>> +		if (err < 0) {
> >>>> +			dev_err(&client->dev,
> >>>> +				"Failed to select remote channel %d, err %d\n",
> >>>> +				channel, err);
> >>>> +			return err;
> >>>
> >>> Not my call to make, but I really dislike all that new noisiness.
> >>> Sure, it is ok to pass the error back, but in my opinion that is
> >>> good enough. If every driver in the kernel would be that noisy,
> >>> the log would be all but useless.
> >>
> >> This was discussed in https://lkml.org/lkml/2013/12/227
> >> Jean wish to catch and return write errors,then the set_temp8() could
> >> return error to user-space.
> >>
> > 
> > The link doesn't work for me. Anyway, catching write errors and being
> > noisy about it are different issues, and I am a bit surprised that
> > Jean wants the driver to be noisy about it, but if so I won't object.
> 
> Sorry, the link should be https://lkml.org/lkml/2013/7/12/227
> 
FWIW, Jean was asking for errors to be detected and be reported to the user,
not to create log messages to the console for each error.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/