linux-kernel - Re: [PATCH 1/2] hwmon: (jc42) optionally try to disable the SMBUS timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 26 Oct 2017 06:45:26 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Peter Rosin <peda@...ntia.se>, Rob Herring <robh@...nel.org>
Cc:     linux-kernel@...r.kernel.org, Mark Rutland <mark.rutland@....com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
        Russell King <linux@...linux.org.uk>,
        Jean Delvare <jdelvare@...e.com>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 1/2] hwmon: (jc42) optionally try to disable the SMBUS
 timeout

On 10/25/2017 11:44 PM, Peter Rosin wrote:
> On 2017-10-18 04:38, Guenter Roeck wrote:
>> On 10/17/2017 03:16 PM, Rob Herring wrote:
>>> On Fri, Oct 13, 2017 at 01:35:27PM -0700, Guenter Roeck wrote:
>>>> On Fri, Oct 13, 2017 at 04:26:57PM +0200, Peter Rosin wrote:
>>>>> On 2017-10-13 15:50, Guenter Roeck wrote:
>>>>>> On 10/13/2017 02:27 AM, Peter Rosin wrote:
>>>>>>> With a nxp,se97 chip on an atmel sama5d31 board, the I2C adapter driver
>>>>>>> is not always capable of avoiding the 25-35 ms timeout as specified by
>>>>>>> the SMBUS protocol. This may cause silent corruption of the last bit of
>>>>>>> any transfer, e.g. a one is read instead of a zero if the sensor chip
>>>>>>> times out. This also affects the eeprom half of the nxp-se97 chip, where
>>>>>>> this silent corruption was originally noticed. Other I2C adapters probably
>>>>>>> suffer similar issues, e.g. bit-banging comes to mind as risky...
>>>>>>>
>>>>>>> The SMBUS register in the nxp chip is not a standard Jedec register, but
>>>>>>> it is not special to the nxp chips either, at least the atmel chips
>>>>>>> have the same mechanism. Therefore, do not special case this on the
>>>>>>> manufacturer, it is opt-in via the device property anyway.
>>>>>>>
>>>>>>> Signed-off-by: Peter Rosin <peda@...ntia.se>
>>>>>>> ---
>>>>>>>     Documentation/devicetree/bindings/hwmon/jc42.txt |  4 ++++
>>>>>>>     drivers/hwmon/jc42.c                             | 20 ++++++++++++++++++++
>>>>>>>     2 files changed, 24 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/hwmon/jc42.txt b/Documentation/devicetree/bindings/hwmon/jc42.txt
>>>>>>> index 07a250498fbb..f569db58f64a 100644
>>>>>>> --- a/Documentation/devicetree/bindings/hwmon/jc42.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/jc42.txt
>>>>>>> @@ -34,6 +34,10 @@ Required properties:
>>>>>>>     
>>>>>>>     - reg: I2C address
>>>>>>>     
>>>>>>> +Optional properties:
>>>>>>> +- smbus-timeout-disable: When set, the smbus timeout function will be disabled.
>>>>>>> +			 This is not supported on all chips.
>>>
>>> Is this only for jc24 devices or could be any smbus device?
>>>
>>
>> SMBus timeout is a standard SMBus functionality, so I would say any. It is by
>> default enabled on an SMBus device (actually it is not just enabled, it is
>> mandatory). The ability to disable it comes handy if a SMBus chip is connected
>> to an I2C controller which does not (or not necessarily) follow SMBus rules.
>>
>> I had seen that problem myself with MAX6697, and STTS751 (and its driver) also
>> supports it.
> 
> So, is the approach with an optional smbus-timeout-disable property documented
> in .../bindings/hwmon/jc42.txt good-to-go or should it be documented in some
> common SMBus client-device file? I don't fine any such beast, so I'm unsure
> how to proceed in that case.
> 

I would suggest .../bindings/hwmon/jc42.txt. Even though the functionality is
supported by various SMBus chips, it is not supported by _every_ SMBus chip.

I also found this:

https://github.com/opennetworklinux/linux/blob/master/3.2.65-1%2Bdeb7u2/patches/driver-adt7470-knob-to-disable-smbus-timeout.patch

which suggests that we should find a common solution (even though that patch
never found its way upstream).

Rob, are you ok with "smbus-timeout-disable" as suggested above ?

Thanks,
Guenter