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  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, 30 Aug 2017 22:59:14 +0200
From:   Javier Martinez Canillas <javier@...hile0.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Wolfram Sang <wsa@...-dreams.de>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        Rob Herring <robh@...nel.org>, Florian Larysch <fl@...1.de>,
        David Lechner <david@...hnology.com>,
        Rob Herring <robh+dt@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Sören Brinkmann <soren.brinkmann@...inx.com>,
        Simon Horman <horms@...ge.net.au>,
        Michal Simek <michal.simek@...inx.com>,
        Dinh Nguyen <dinguyen@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Will Deacon <will.deacon@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Sekhar Nori <nsekhar@...com>, Scott Wood <oss@...error.net>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Joachim Eastwood <manabian@...il.com>,
        Mark Rutland <mark.rutland@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Paul Mackerras <paulus@...ba.org>,
        Magnus Damm <magnus.damm@...il.com>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Uwe Kleine-König <kernel@...gutronix.de>,
        Linux I2C <linux-i2c@...r.kernel.org>
Subject: Re: [RESEND PATCH v5 00/16] eeprom: at24: Add OF device ID table

Hello Geert,

On Wed, Aug 30, 2017 at 10:15 PM, Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
> Hi Javier,
>
> On Wed, Aug 30, 2017 at 9:57 PM, Javier Martinez Canillas
> <javier@...hile0.org> wrote:
>>> I think we should talk about the same case: Let me repeat what I did:
>>>
>>> 1) I added your patch "eeprom: at24: Add OF device ID table"
>>> 2) I added an EEPROM node to an I2C
>>>
>>> +       eeprom@50 {
>>> +               compatible = "renesas,24c01";
>>> +               reg = <0x50>;
>>> +       };
>>>
>>> -> no at24 binding to the device
>>>
>>> 3) I revert your patch
>>>
>>> -> at24 binding to the device
>>>
>>
>> I've tested this and you are right, it fails...
>>
>> The problem is that the patch also changes how the driver obtains the
>> EEPROM parameters (the magic value in the entry's data field).
>>
>> So even when module autoload and device / driver matching works, the
>> driver probe function fails because if (client->dev.of_node) the
>> driver attempts to get the entry data using
>> of_device_get_match_data(), which is obviously wrong since the
>> compatible string in the dev node isn't present in the OF table.
>>
>> The id->driver_data from the I2C table should be used instead since
>> that's the table that matches in this case.
>>
>> One option is to fallback to id->driver_data if
>> of_device_get_match_data() fails, but that's just an (ugly)
>> workaround. So I agree with you that the best option is to wait for
>> the DTS patches to land first.
>
> Which means new kernels won't work with old DTBs. Oops...
> I'm afraid that needs to be fixed.  People care about DTB backward
> compatibility on many platforms.
>

Right, I've yet to find one of those mythical platforms that ship old
DTBs with new kernels, but I agree with you since people seem to care
about backward compatibility (at least on theory).

So I see two options then:

1) Use the workaround I mentioned and lookup the I2C device ID table
entry data if of_device_get_match_data() fails

2) Only call of_device_get_match_data() if (dev->of_node &&
of_match_device(dev->driver->of_match_table, dev))

Not sure what's the preferred idiom for these cases.

To good thing about keeping backward compatibility is that Wolfram
would be able to pick the driver patch even before the DTS patches
land.

Best regards,
Javier

Powered by blists - more mailing lists