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

[Javier Martinez Canillas]

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")?

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".

> Rob

Best regards,
Javier