Re: [PATCH v4 03/14] dt-bindings: i2c: add bindings for microchip mpfs i2c

From: conor dooley
Date: Thu Jan 20 2022 - 10:42:13 EST


>Hi Conor,
>
>On Thu, Jan 20, 2022 at 2:42 PM <Conor.Dooley@xxxxxxxxxxxxx> wrote:
>> On 20/01/2022 08:30, Geert Uytterhoeven wrote:
>> > On Mon, Jan 17, 2022 at 12:06 PM <conor.dooley@xxxxxxxxxxxxx> wrote:
>> > Wouldn't it be more logical to have:
>> >
>> > items:
>> > - const: microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
>> > - const: microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core
>> >
>> > ?
>> This would be fine for mpfs-i2c since corei2c is a "superset" - but how
>> would that look for the fabric core? I don't think falling back from the
>> fabric core onto the "hard" one makes sense. This would mean the
>> following two entries:
>>
>> i2c2: i2c@44000000 { //fabric
>> compatible = "microchip,corei2c-rtl-v7";
>> };
>> i2c1: i2c@2010b000 { //"hard" mpfs peripheral
>> compatible = "microchip,mpfs-i2c", "microchip,corei2c-rtl-v7";
>> };
>
>Oops, I missed that you have both forms.
>But in se, they're the same IP core, just hard vs. soft? Then the
>below makes sense.
A lot (but not all) of the peripherals on Polarfire SoC are "subsets"
of the IP cores: I think corei2c is almost identical but for others
the hard version has some of the optional features disabled or slight
changes made.

If the IP is already written why not use it ;)
>
>> But this generates errors in dt_binding_check w/ your suggestion - so
>> how about the following (similar to ti,omap4-i2c.yaml):
>>
>> compatible:
>> oneOf:
>> - items:
>> - const: microchip,mpfs-i2c # Microchip PolarFire...
>> - const: microchip,corei2c-rtl-v7 # Microchip Fabric...
>> - const: microchip,corei2c-rtl-v7 # Microchip Fabric...
>>
>> Is there a prettier way than this duplication?
>
>I'm afraid not, and the above scheme is used a lot.
Fair enough!
>
>> > If the IP core is reused, it can become:
>> >
>> > items:
>> > - enum:
>> > - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
>> > - microchip,<foo>-i2c # ...
>> > - const: microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core
>> >
>> > That way the driver can just match on the second (fallback) value,
>> > and no further driver changes will be needed (until v8 or later).