[PATCH v4 00/20] eeprom: at24: Add OF device ID table

[Rob Herring]

On Mon, May 22, 2017 at 9:46 AM, Javier Martinez Canillas
<javier at dowhile0.org> wrote:
> Hello Geert and Rob,
>
> On Mon, May 22, 2017 at 4:26 PM, Rob Herring <robh at kernel.org> wrote:
>> On Mon, May 22, 2017 at 9:23 AM, Geert Uytterhoeven
>> <geert at linux-m68k.org> wrote:
>>> Hi Javier,
>>>
>>> On Mon, May 22, 2017 at 4:01 PM, Javier Martinez Canillas
>>> <javier at dowhile0.org> wrote:
>>>> This series is a follow-up to patch [0] that added an OF device ID table
>>>> to the at24 EEPROM driver. As you suggested [1], this version instead of
>>>> adding entries for every used <vendor,device> tuple, only adds a single
>>>> entry for each chip type using the "atmel" vendor as a generic fallback.
>>>>
>>>> This is a re-spin that addresses some issues pointed out by Rob Herring.
>>>>
>>>> The first patch documents in the DT binding what's the correct vendor to
>>>> use and what are the ones that are being deprecated. The second one adds
>>>> the OF device ID table for the at24 driver and the next patches use this
>>>> vendor in the compatible string to each DTS that defines a compatible I2C
>>>> EEPROM device node.
>>>>
>>>> Patches can be applied independently since the DTS changes without driver
>>>> changes are no-op and the OF table won't be used without the DTS changes.
>>>>
>>>> [0]: https://lkml.org/lkml/2017/3/14/589
>>>> [1]: https://lkml.org/lkml/2017/3/15/99
>>>>
>>>> Best regards,
>>>> Javier
>>>>
>>>> Changes in v4:
>>>> - Document the manufacturers that have been deprecated (Rob Herring).
>>>> - Only use the atmel manufacturer in the compatible string instead of
>>>>   keeping the deprecated ones (Rob Herring).
>>>
>>> I think you're referring to this (https://lkml.org/lkml/2017/4/19/1136)?
>>>
>>> | > --- a/arch/arm/boot/dts/am335x-baltos.dtsi
>>> | > +++ b/arch/arm/boot/dts/am335x-baltos.dtsi
>>> | > @@ -255,7 +255,7 @@
>>> | >     };
>>> | >
>>> | >     at24 at 50 {
>>> | > -           compatible = "at24,24c02";
>>> | > +           compatible = "at24,24c02", "atmel,24c02";
>>> |
>>> | I think you can just drop the at24 compatibles. A new kernel doesn't
>>> | need it. An old kernel ignores the manufacturer. I checked that u-boot
>>> | only matches on "atmel,*", so okay there. Don't know about the *BSDs. I
>>> | couldn't find anything.
>>>
>>> I think you misunderstood what Rob means.
>>>
>>> In the case above it makes sense to drop the first compatible, as "at24" is
>>> not a manufacturer, but refers to ATMEL's "AT24" line of i2c FLASH ROMs.
>>>
>>> However, in cases where a real vendor/part combo is specified, like on
>>> r8a7791-koelsch.dts:
>>>
>>> -               compatible = "renesas,24c02";
>>> +               compatible = "atmel,24c02";
>>>
>>> you do want to keep the real vendor/part combo, i.e.
>>>
>>>      compatible = "renesas,24c02", "atmel,24c02";
>>
>> Yes, Geert is correct.
>>
>
> Sorry for misunderstanding your previous comment...
>
> How this should be documented in the DT binding? Should I include
> "renesas" as a valid manufacturer or only list the used
> <vendor,device> pairs (i.e: "renesas,24c02")?

However you are handling any of the vendors. I'll have to go look.

> I also wonder why this is really needed if AFAIU "renesas,24c02" is
> compatible with "atmel,24c02". IOW, the driver doesn't need to
> differentiate between the two since the devices are the same and will
> always match using "atmel,24c02".

It is needed, so that when a difference is found, it can be handled
without updating the DT.

Rob