On Mon, May 22, 2017 at 9:46 AM, Javier Martinez Canillas <jav...@dowhile0.org> wrote: > Hello Geert and Rob, > > On Mon, May 22, 2017 at 4:26 PM, Rob Herring <r...@kernel.org> wrote: >> On Mon, May 22, 2017 at 9:23 AM, Geert Uytterhoeven >> <ge...@linux-m68k.org> wrote: >>> Hi Javier, >>> >>> On Mon, May 22, 2017 at 4:01 PM, Javier Martinez Canillas >>> <jav...@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@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
