Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1495698

adm9240 error handling (was Re: [PATCHv1] hwmon: adm9240: handle temperature readings below 0)

From Chris Packham <Chris.Packham@alliedtelesis.co.nz>
Newsgroups linux.kernel
Subject adm9240 error handling (was Re: [PATCHv1] hwmon: adm9240: handle temperature readings below 0)
Date 2016-10-05 07:10 +0200
Message-ID <soMCR-4Fn-5@gated-at.bofh.it> (permalink)
References <sopdf-5Kn-9@gated-at.bofh.it> <soEYF-7J5-7@gated-at.bofh.it> <soF8m-81O-27@gated-at.bofh.it> <soG4q-bv-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Guenter,

On 10/05/2016 11:10 AM, Guenter Roeck wrote:
> On Tue, Oct 04, 2016 at 09:09:10PM +0000, Chris Packham wrote:
>>
>>>
>>> Of course, all that doesn't solve the real problem in this driver, which is
>>> that it ignores error codes from the smbus functions, but that is a different
>>> problem.
>>
>> Yeah I'd noted that too. This patch is actually a port of changes we've
>> made to a custom LM81 driver based on the original adm9240 driver which
>> included some error handling. I'll look at bringing more of that code in
>> for a future patch.
>>
> Sounds good...
>

So on this. The as-yet unpublished changes we have basically consist of 
changing adm9240_update_device() to do something like this


                         || !data->valid) {

                 for (i = 0; i < 6; i++) { /* read voltages */
-                       data->in[i] = i2c_smbus_read_byte_data(client,
+                       ret = i2c_smbus_read_byte_data(client,
                                         ADM9240_REG_IN(i));
+                       if (ret >= 0)
+                               data->in[i] = ret;

The user is still none the wiser that there was an error but at least we 
haven't just attempted to write a error code to the data. Is this the 
direction you want to head? Are there other hwmon drivers that do 
something saner?

Another attempt we've made is to try to read something innocuous like 
the CONFIG register to check that the i2c bus is in a good state before 
reading the rest of the values.

As you can guess these "fixes" have piled up over time and because 
they're hard to test we haven't been brave enough to remove them.

Any thoughts?

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCHv1] hwmon: adm9240: handle temperature readings below 0 Chris Packham <chris.packham@alliedtelesis.co.nz> - 2016-10-04 06:10 +0200
  Re: [PATCHv1] hwmon: adm9240: handle temperature readings below 0 Guenter Roeck <linux@roeck-us.net> - 2016-10-04 23:00 +0200
    Re: [PATCHv1] hwmon: adm9240: handle temperature readings below 0 Chris Packham <Chris.Packham@alliedtelesis.co.nz> - 2016-10-04 23:10 +0200
      Re: [PATCHv1] hwmon: adm9240: handle temperature readings below 0 Guenter Roeck <linux@roeck-us.net> - 2016-10-05 00:10 +0200
        adm9240 error handling (was Re: [PATCHv1] hwmon: adm9240: handle  temperature readings below 0) Chris Packham <Chris.Packham@alliedtelesis.co.nz> - 2016-10-05 07:10 +0200
          Re: adm9240 error handling (was Re: [PATCHv1] hwmon: adm9240: handle  temperature readings below 0) Guenter Roeck <linux@roeck-us.net> - 2016-10-05 15:30 +0200

csiph-web