| Message ID | 20240621042737.674128-3-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mips: Support for RTL9302C | expand |
On Fri, Jun 21, 2024 at 04:27:33PM +1200, Chris Packham wrote: > Add the devicetree schema for the realtek,otto-timer present on a number > of Realtek SoCs. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > .../bindings/timer/realtek,otto-timer.yaml | 54 +++++++++++++++++++ > 1 file changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > > diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > new file mode 100644 > index 000000000000..b6e85aadbc99 > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/timer/realtek,otto-timer.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek Otto SoCs Timer/Counter > + > +description: > + Realtek SoCs support a number of timers/counters. These are used > + as a per CPU clock event generator and an overall CPU clocksource. > + > +maintainers: > + - Chris Packham <chris.packham@alliedtelesis.co.nz> > + > +properties: > + $nodename: > + pattern: "^timer@[0-9a-f]+$" > + > + compatible: > + items: > + - enum: > + - realtek,rtl930x-timer > + - const: realtek,otto-timer > + reg: > + minItems: 5 > + maxItems: 5 Since minitems == maxitems, can you just make this a list, and define what they all are? Ditto interrupts. reg: items: - foo - bar - baz etc. > + > + clocks: > + maxItems: 1 > + > + interrupts: > + minItems: 5 > + maxItems: 5 > + > +required: > + - compatible > + - reg > + - clocks > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + timer0: timer@3200 { The label here isn't needed FYI. Thanks, Conor. > + compatible = "realtek,rtl930x-timer", "realtek,otto-timer"; > + reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>, > + <0x3230 0x10>, <0x3240 0x10>; > + > + interrupt-parent = <&intc>; > + interrupts = <7 4>, <8 4>, <9 4>, <10 4>, <11 4>; > + clocks = <&lx_clk>; > + }; > -- > 2.45.2 >
On 21/06/2024 06:27, Chris Packham wrote: > Add the devicetree schema for the realtek,otto-timer present on a number > of Realtek SoCs. Please order your patches correctly: bindings always go before users. A nit, subject: drop second/last, redundant "schema for". The "dt-bindings" prefix is already stating that these are bindings (so schema). See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > .../bindings/timer/realtek,otto-timer.yaml | 54 +++++++++++++++++++ > 1 file changed, 54 insertions(+) > create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > > diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > new file mode 100644 > index 000000000000..b6e85aadbc99 > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/timer/realtek,otto-timer.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Realtek Otto SoCs Timer/Counter > + > +description: > + Realtek SoCs support a number of timers/counters. These are used > + as a per CPU clock event generator and an overall CPU clocksource. > + > +maintainers: > + - Chris Packham <chris.packham@alliedtelesis.co.nz> > + > +properties: > + $nodename: > + pattern: "^timer@[0-9a-f]+$" > + > + compatible: > + items: > + - enum: > + - realtek,rtl930x-timer No wildcards. > + - const: realtek,otto-timer Do you have access to datasheet of all Otto SoCs and can you confirm that all of them have the same timer programming interface? Just drop generic compatible and use SoCs compatible. Blank line. > + reg: > + minItems: 5 > + maxItems: 5 Instead list and describe the items. > + > + clocks: > + maxItems: 1 > + > + interrupts: > + minItems: 5 > + maxItems: 5 Instead list and describe the items. > + > +required: > + - compatible > + - reg > + - clocks > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + timer0: timer@3200 { Drop unused label > + compatible = "realtek,rtl930x-timer", "realtek,otto-timer"; > + reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>, > + <0x3230 0x10>, <0x3240 0x10>; > + > + interrupt-parent = <&intc>; > + interrupts = <7 4>, <8 4>, <9 4>, <10 4>, <11 4>; Use proper defines for the flags. > + clocks = <&lx_clk>; > + }; Best regards, Krzysztof
(resend as plain text) On 23/06/24 00:11, Conor Dooley wrote: > On Fri, Jun 21, 2024 at 04:27:33PM +1200, Chris Packham wrote: >> Add the devicetree schema for the realtek,otto-timer present on a number >> of Realtek SoCs. >> >> Signed-off-by: Chris Packham<chris.packham@alliedtelesis.co.nz> >> --- >> .../bindings/timer/realtek,otto-timer.yaml | 54 +++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml >> >> diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml >> new file mode 100644 >> index 000000000000..b6e85aadbc99 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml >> @@ -0,0 +1,54 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id:http://devicetree.org/schemas/timer/realtek,otto-timer.yaml# >> +$schema:http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Realtek Otto SoCs Timer/Counter >> + >> +description: >> + Realtek SoCs support a number of timers/counters. These are used >> + as a per CPU clock event generator and an overall CPU clocksource. >> + >> +maintainers: >> + - Chris Packham<chris.packham@alliedtelesis.co.nz> >> + >> +properties: >> + $nodename: >> + pattern: "^timer@[0-9a-f]+$" >> + >> + compatible: >> + items: >> + - enum: >> + - realtek,rtl930x-timer I'll change this to rtl9302 >> + - const: realtek,otto-timer >> + reg: >> + minItems: 5 >> + maxItems: 5 > Since minitems == maxitems, can you just make this a list, and define > what they all are? Ditto interrupts. This is where more conditions might need to be added. The rtl9302 is a single core SoC. So technically it only needs 2 timers (the hardware still has 5 but 3 would be unused at the moment). The rtl9312 is a dual core SoC so needs 3 timers (I won't be looking at that platform for a while). So I think maybe maxItems should stay at 5 but minItems should be set based on the compatible. > reg: > items: > - foo > - bar > - baz > > etc. I can do. But they'd all be something like cpuN-event. The way the driver is written it grabs a timer for each CPU and uses the next one for a global timer. >> + >> + clocks: >> + maxItems: 1 >> + >> + interrupts: >> + minItems: 5 >> + maxItems: 5 >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - interrupts >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + timer0: timer@3200 { > The label here isn't needed FYI. Will remove. >> + compatible = "realtek,rtl930x-timer", "realtek,otto-timer"; >> + reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>, >> + <0x3230 0x10>, <0x3240 0x10>; >> + >> + interrupt-parent = <&intc>; >> + interrupts = <7 4>, <8 4>, <9 4>, <10 4>, <11 4>; Will switch to using proper IRQ flags. >> + clocks = <&lx_clk>; >> + }; >> -- >> 2.45.2 >>
On Sun, Jun 23, 2024 at 09:23:55PM +0000, Chris Packham wrote: > (resend as plain text) > > On 23/06/24 00:11, Conor Dooley wrote: > > On Fri, Jun 21, 2024 at 04:27:33PM +1200, Chris Packham wrote: > >> Add the devicetree schema for the realtek,otto-timer present on a number > >> of Realtek SoCs. > >> > >> Signed-off-by: Chris Packham<chris.packham@alliedtelesis.co.nz> > >> --- > >> .../bindings/timer/realtek,otto-timer.yaml | 54 +++++++++++++++++++ > >> 1 file changed, 54 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > >> new file mode 100644 > >> index 000000000000..b6e85aadbc99 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml > >> @@ -0,0 +1,54 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id:http://devicetree.org/schemas/timer/realtek,otto-timer.yaml# > >> +$schema:http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Realtek Otto SoCs Timer/Counter > >> + > >> +description: > >> + Realtek SoCs support a number of timers/counters. These are used > >> + as a per CPU clock event generator and an overall CPU clocksource. > >> + > >> +maintainers: > >> + - Chris Packham<chris.packham@alliedtelesis.co.nz> > >> + > >> +properties: > >> + $nodename: > >> + pattern: "^timer@[0-9a-f]+$" > >> + > >> + compatible: > >> + items: > >> + - enum: > >> + - realtek,rtl930x-timer > > I'll change this to rtl9302 > > >> + - const: realtek,otto-timer > >> + reg: > >> + minItems: 5 > >> + maxItems: 5 > > Since minitems == maxitems, can you just make this a list, and define > > what they all are? Ditto interrupts. > > This is where more conditions might need to be added. The rtl9302 is a > single core SoC. So technically it only needs 2 timers (the hardware > still has 5 but 3 would be unused at the moment). The rtl9312 is a dual > core SoC so needs 3 timers (I won't be looking at that platform for a > while). So I think maybe maxItems should stay at 5 but minItems should > be set based on the compatible. Sounds good to me. > > reg: > > items: > > - foo > > - bar > > - baz > > > > etc. > > I can do. But they'd all be something like cpuN-event. The way the > driver is written it grabs a timer for each CPU and uses the next one > for a global timer. I think it's fine if they all have very simplistic names, their roles should be documented somehow. Cheers, Conor.
diff --git a/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml new file mode 100644 index 000000000000..b6e85aadbc99 --- /dev/null +++ b/Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/timer/realtek,otto-timer.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Realtek Otto SoCs Timer/Counter + +description: + Realtek SoCs support a number of timers/counters. These are used + as a per CPU clock event generator and an overall CPU clocksource. + +maintainers: + - Chris Packham <chris.packham@alliedtelesis.co.nz> + +properties: + $nodename: + pattern: "^timer@[0-9a-f]+$" + + compatible: + items: + - enum: + - realtek,rtl930x-timer + - const: realtek,otto-timer + reg: + minItems: 5 + maxItems: 5 + + clocks: + maxItems: 1 + + interrupts: + minItems: 5 + maxItems: 5 + +required: + - compatible + - reg + - clocks + - interrupts + +additionalProperties: false + +examples: + - | + timer0: timer@3200 { + compatible = "realtek,rtl930x-timer", "realtek,otto-timer"; + reg = <0x3200 0x10>, <0x3210 0x10>, <0x3220 0x10>, + <0x3230 0x10>, <0x3240 0x10>; + + interrupt-parent = <&intc>; + interrupts = <7 4>, <8 4>, <9 4>, <10 4>, <11 4>; + clocks = <&lx_clk>; + };
Add the devicetree schema for the realtek,otto-timer present on a number of Realtek SoCs. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- .../bindings/timer/realtek,otto-timer.yaml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Documentation/devicetree/bindings/timer/realtek,otto-timer.yaml