linux-kernel - Re: [PATCH] dt-bindings: i2c: Add armada-38x i2c binding

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 04 Oct 2017 14:52:11 +0200
From:   Gregory CLEMENT <gregory.clement@...e-electrons.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Chris Packham <Chris.Packham@...iedtelesis.co.nz>,
        Kalyan Kinthada <Kalyan.Kinthada@...iedtelesis.co.nz>,
        "devicetree\@vger.kernel.org" <devicetree@...r.kernel.org>,
        "wsa\@the-dreams.de" <wsa@...-dreams.de>,
        "pawel.moll\@arm.com" <pawel.moll@....com>,
        "mark.rutland\@arm.com" <mark.rutland@....com>,
        "ijc+devicetree\@hellion.org.uk" <ijc+devicetree@...lion.org.uk>,
        "galak\@codeaurora.org" <galak@...eaurora.org>,
        "linux-i2c\@vger.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] dt-bindings: i2c: Add armada-38x i2c binding

Hi Rob,
 
 On mer., oct. 04 2017, Rob Herring <robh@...nel.org> wrote:

> On Wed, Oct 4, 2017 at 3:33 AM, Gregory CLEMENT
> <gregory.clement@...e-electrons.com> wrote:
>> Hi Rob,
>>
>>  On mar., oct. 03 2017, Rob Herring <robh@...nel.org> wrote:
>>
>>> On Wed, Sep 27, 2017 at 09:33:00AM +0200, Gregory CLEMENT wrote:
>>>> Hi Chris,
>>>>
>>>>  On mer., sept. 27 2017, Chris Packham <Chris.Packham@...iedtelesis.co.nz> wrote:
>>>>
>>>> > Hi Gregory,
>>>> >
>>>> > On 27/09/17 00:56, Gregory CLEMENT wrote:
>>>> >> Hi Kalyan,
>>>> >>
>>>> >> Please try avoid top-posting.
>>>> >>
>>>> >>   On dim., sept. 24 2017, Kalyan Kinthada <Kalyan.Kinthada@...iedtelesis.co.nz> wrote:
>>>> >>
>>>> >>> Hi Gregory,
>>>> >>>
>>>> >>> I got this information from Armada-38x functional errata document.
>>>> >>
>>>> >> OK but in any case just adding a new compatible was not enough you have
>>>> >> to update the driver in the same time, however for this case we won't
>>>> >> need it, see below.
>>>> >>
>>>> >>>
>>>> >>> I can add the "marvell,mv78230-i2c" compatible string to the appropriate  device tree files
>>>> >>> but the i2c-mv64xxx driver enables an additional  feature (offload i2c transactions)
>>>> >>> based on the compatible string "marvell,mv78230-i2c".
>>>> >>>
>>>> >>> I am not sure if this feature (offload i2c transactions) should be enabled for Armada-38x devices.
>>>> >>> That is the reason I felt for the need of a new compatible string
>>>> >>> specifically for Armada-38x SoCs.
>>>> >>
>>>> >> Indeed the Armada-38x SoCs does not support hardware offloading (at
>>>> >> least according the datasheet). But it happens that in the earlier
>>>> >> version of the Armada XP the hardware offloading was buggy, so we
>>>> >> introduced a compatible for this case: marvell,mv78230-a0-i2c. This
>>>> >> compatible enable the errata fix but not the offloading feature. That
>>>> >> means that it is exactly the compatible you need for Armada 38x (and
>>>> >> Armada 39x and 375 I think).
>>>> >
>>>> > The "mv78230-a0-i2c" dt-binding has the following note
>>>> >
>>>> >    Note: Only use "marvell,mv78230-a0-i2c" for a
>>>> >    very rare, initial version of the SoC which
>>>> >    had broken offload support.  Linux
>>>> >    auto-detects this and sets it appropriately.
>>>> >
>>>> > If we are going to re-use this binding for armada-38x we should probably
>>>> > remove this note. Personally my preference would be an armada-38x
>>>>
>>>> Updating the documentation is the right thing to do yes.
>>>>
>>>>
>>>> > compatible string (or 370 if that's the common base of these SoCs). But
>>>> > of course we'll go with whatever your preference is as maintainer.
>>>>
>>>> If the IP is compatible then there is no reason to add a new one, else
>>>> we will end with a compatible for each SoC and the compatible property
>>>> will just loose its meaning.
>>>
>>> If you all had added compatibles for each SoC in the first place, then
>>> we wouldn't be having this dicussion.
>>
>> Really??
>>
>> For Armada 38x we have already 6 flavor 381, 382, 383, 385, 388.  Then
>> we have 9 SoC families on mvebu: orion5x, kirkwood, dove, Armada 370,
>> Armada XP, Armada 375, Armada 39x, Armada 7K, Armada 8K. Of course in
>> each family we have several flavors.
>>
>> But then Allwiner also use this driver in 8 SoC families: sun4i, sun5i,
>> sun6i, sun7i, sun8i, sun9i, sunxi, sun50i. Here again each family have
>> their own flavor.
>>
>> So you are just suggesting to blot the i2c driver with more than 50
>> compatible string!
>
> No.
>
>> That also mean updating the i2c driver each time a new SoC flavor
>> appear, so more or less for each release.
>
> No. You can have one or several fallbacks. Use the fallback as long as
> you think they are the same and then add the more specific one to the
> driver when you find differences.

So you mean adding all the possible variant of the SoC in the binding
documentation, but only implement them in the driver when needed?

Gregory

>
> Rob

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com