diff mbox series

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

Message ID 20220117110755.3433142-4-conor.dooley@microchip.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Update the Icicle Kit device tree | expand

Commit Message

Conor Dooley Jan. 17, 2022, 11:07 a.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

Add device tree bindings for the i2c controller on
the Microchip PolarFire SoC.

Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/i2c/microchip,mpfs-i2c.yaml      | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml

Comments

Rob Herring (Arm) Jan. 19, 2022, 3:10 a.m. UTC | #1
On Mon, 17 Jan 2022 11:07:44 +0000, conor.dooley@microchip.com wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Add device tree bindings for the i2c controller on
> the Microchip PolarFire SoC.
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../bindings/i2c/microchip,mpfs-i2c.yaml      | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Geert Uytterhoeven Jan. 20, 2022, 8:30 a.m. UTC | #2
Hi Conor,

On Mon, Jan 17, 2022 at 12:06 PM <conor.dooley@microchip.com> wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Add device tree bindings for the i2c controller on
> the Microchip PolarFire SoC.
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MPFS I2C Controller Device Tree Bindings
> +
> +maintainers:
> +  - Daire McNamara <daire.mcnamara@microchip.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
> +      - microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core

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

?

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Conor Dooley Jan. 20, 2022, 1:42 p.m. UTC | #3
On 20/01/2022 08:30, Geert Uytterhoeven wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Conor,
> 
> On Mon, Jan 17, 2022 at 12:06 PM <conor.dooley@microchip.com> wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> Add device tree bindings for the i2c controller on
>> the Microchip PolarFire SoC.
>>
>> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks for your patch!
> 
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
>> @@ -0,0 +1,55 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip MPFS I2C Controller Device Tree Bindings
>> +
>> +maintainers:
>> +  - Daire McNamara <daire.mcnamara@microchip.com>
>> +
>> +allOf:
>> +  - $ref: /schemas/i2c/i2c-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
>> +      - microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core
> 
> 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";
};

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?
> 
> 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).
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>
Geert Uytterhoeven Jan. 20, 2022, 2:56 p.m. UTC | #4
Hi Conor,

On Thu, Jan 20, 2022 at 2:42 PM <Conor.Dooley@microchip.com> wrote:
> On 20/01/2022 08:30, Geert Uytterhoeven wrote:
> > On Mon, Jan 17, 2022 at 12:06 PM <conor.dooley@microchip.com> wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> Add device tree bindings for the i2c controller on
> >> the Microchip PolarFire SoC.
> >>
> >> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >
> > Thanks for your patch!
> >
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
> >> @@ -0,0 +1,55 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Microchip MPFS I2C Controller Device Tree Bindings
> >> +
> >> +maintainers:
> >> +  - Daire McNamara <daire.mcnamara@microchip.com>
> >> +
> >> +allOf:
> >> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> >> +
> >> +properties:
> >> +  compatible:
> >> +    enum:
> >> +      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
> >> +      - microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core
> >
> > 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.

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

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Conor Dooley Jan. 20, 2022, 3:42 p.m. UTC | #5
>Hi Conor,
>
>On Thu, Jan 20, 2022 at 2:42 PM <Conor.Dooley@microchip.com> wrote:
>> On 20/01/2022 08:30, Geert Uytterhoeven wrote:
>> > On Mon, Jan 17, 2022 at 12:06 PM <conor.dooley@microchip.com> 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).
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml b/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
new file mode 100644
index 000000000000..ced843e78844
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/microchip,mpfs-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip MPFS I2C Controller Device Tree Bindings
+
+maintainers:
+  - Daire McNamara <daire.mcnamara@microchip.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-i2c # Microchip PolarFire SoC compatible SoCs
+      - microchip,corei2c-rtl-v7 # Microchip Fabric based i2c IP core
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-frequency:
+    description: |
+      Desired I2C bus clock frequency in Hz. As only Standard and Fast
+      modes are supported, possible values are 100000 and 400000.
+    enum: [100000, 400000]
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    i2c@2010a000 {
+      compatible = "microchip,mpfs-i2c";
+      reg = <0x2010a000 0x1000>;
+      clocks = <&clkcfg CLK_I2C0>;
+      interrupt-parent = <&plic>;
+      interrupts = <58>;
+      clock-frequency = <100000>;
+    };
+...