| Message ID | 20220117110755.3433142-4-conor.dooley@microchip.com |
|---|---|
| State | New |
| Headers | show |
| Series | Update the Icicle Kit device tree | expand |
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 >
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
>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 --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>; + }; +...