| Message ID | 20230530090758.1652329-2-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | dt-bindings: timer: Microchip AT91 convert to YAML | expand |
On 30/05/2023 11:07, Claudiu Beznea wrote: > Convert Microchip AT91 PIT bindings to YAML. Along with it clocks and > clock-names bindings were added as the drivers needs it to ensure proper > hardware functionality. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > .../devicetree/bindings/arm/atmel-sysregs.txt | 12 --- > .../bindings/timer/atmel,at91sam9260-pit.yaml | 99 +++++++++++++++++++ > 2 files changed, 99 insertions(+), 12 deletions(-) > create mode 100644 Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml > > diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt > index 67a66bf74895..54d3f586403e 100644 > --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt > +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt > @@ -4,18 +4,6 @@ Chipid required properties: > - compatible: Should be "atmel,sama5d2-chipid" or "microchip,sama7g5-chipid" > - reg : Should contain registers location and length > > -PIT Timer required properties: > -- compatible: Should be "atmel,at91sam9260-pit" > -- reg: Should contain registers location and length > -- interrupts: Should contain interrupt for the PIT which is the IRQ line > - shared across all System Controller members. > - > -PIT64B Timer required properties: > -- compatible: Should be "microchip,sam9x60-pit64b" > -- reg: Should contain registers location and length > -- interrupts: Should contain interrupt for PIT64B timer > -- clocks: Should contain the available clock sources for PIT64B timer. > - > System Timer (ST) required properties: > - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd" > - reg: Should contain registers location and length > diff --git a/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml > new file mode 100644 > index 000000000000..d0f3f80db4cb > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml > @@ -0,0 +1,99 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/timer/atmel,at91sam9260-pit.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip AT91 Periodic Interval Timer (PIT) > + > +maintainers: > + - Claudiu Beznea <claudiu.beznea@microchip.com> > + > +description: > + Microchip AT91 periodic interval timer provides the operating system scheduler > + interrupt. It is designed to offer maximum accuracy and efficient management, > + even for systems with long response time. > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: microchip,sama7g5-pit64b From where do you have this compatible? Wasn't in old binding and commit msg does not explain it. > + - const: microchip,sam9x60-pit64b > + - items: > + enum: These are not items. Just enum.. Does it even work? > + - atmel,at91sam9260-pit > + - microchip,sam9x60-pit64b > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 1 > + maxItems: 2 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: atmel,at91sam9260-pit > + then: > + properties: > + interrupts: > + description: > + Shared interrupt between all system controller members (power management > + controller, watchdog, PIT, reset controller, real-time timer, real-time > + clock, memory controller, debug unit, system timer). > + clocks: > + maxItems: 1 > + > + else: > + properties: > + clocks: > + minItems: 2 > + clock-names: > + items: > + - const: pclk > + - const: gclk interrupts? They are still required, so why no description here? > + required: > + - clock-names > + > +unevaluatedProperties: false additionalProperties:false instead > + Best regards, Krzysztof
On 31.05.2023 11:55, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 30/05/2023 11:07, Claudiu Beznea wrote: >> Convert Microchip AT91 PIT bindings to YAML. Along with it clocks and >> clock-names bindings were added as the drivers needs it to ensure proper >> hardware functionality. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> .../devicetree/bindings/arm/atmel-sysregs.txt | 12 --- >> .../bindings/timer/atmel,at91sam9260-pit.yaml | 99 +++++++++++++++++++ >> 2 files changed, 99 insertions(+), 12 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >> >> diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >> index 67a66bf74895..54d3f586403e 100644 >> --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >> +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >> @@ -4,18 +4,6 @@ Chipid required properties: >> - compatible: Should be "atmel,sama5d2-chipid" or "microchip,sama7g5-chipid" >> - reg : Should contain registers location and length >> >> -PIT Timer required properties: >> -- compatible: Should be "atmel,at91sam9260-pit" >> -- reg: Should contain registers location and length >> -- interrupts: Should contain interrupt for the PIT which is the IRQ line >> - shared across all System Controller members. >> - >> -PIT64B Timer required properties: >> -- compatible: Should be "microchip,sam9x60-pit64b" >> -- reg: Should contain registers location and length >> -- interrupts: Should contain interrupt for PIT64B timer >> -- clocks: Should contain the available clock sources for PIT64B timer. >> - >> System Timer (ST) required properties: >> - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd" >> - reg: Should contain registers location and length >> diff --git a/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >> new file mode 100644 >> index 000000000000..d0f3f80db4cb >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >> @@ -0,0 +1,99 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/timer/atmel,at91sam9260-pit.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Microchip AT91 Periodic Interval Timer (PIT) >> + >> +maintainers: >> + - Claudiu Beznea <claudiu.beznea@microchip.com> >> + >> +description: >> + Microchip AT91 periodic interval timer provides the operating system scheduler >> + interrupt. It is designed to offer maximum accuracy and efficient management, >> + even for systems with long response time. >> + >> +properties: >> + compatible: >> + oneOf: >> + - items: >> + - const: microchip,sama7g5-pit64b > >>From where do you have this compatible? Wasn't in old binding and commit > msg does not explain it. ok, I'll update it in the commit message. It is from the available device trees. > > >> + - const: microchip,sam9x60-pit64b >> + - items: >> + enum: > > These are not items. Just enum.. Does it even work? Yes, it compiles w/o issues. I'll update it anyway. > >> + - atmel,at91sam9260-pit >> + - microchip,sam9x60-pit64b >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + minItems: 1 >> + maxItems: 2 >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: atmel,at91sam9260-pit >> + then: >> + properties: >> + interrupts: >> + description: >> + Shared interrupt between all system controller members (power management >> + controller, watchdog, PIT, reset controller, real-time timer, real-time >> + clock, memory controller, debug unit, system timer). >> + clocks: >> + maxItems: 1 >> + >> + else: >> + properties: >> + clocks: >> + minItems: 2 >> + clock-names: >> + items: >> + - const: pclk >> + - const: gclk > > interrupts? They are still required, so why no description here? It was here in the previous versions but Conor suggested to remove it as it was nothing specific about this description. For the if-then branch I kept it to specify that the interrupt is share with other devices. In this branch the interrupt is only for the timer itself. With this, would you still prefer to add it back? > >> + required: >> + - clock-names >> + >> +unevaluatedProperties: false > > additionalProperties:false instead Having additionalProperties:false instead of unevaluatedProperties: false thows the following error on make dt_binding_check and make dtbs_check: Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.example.dtb: timer@f0028000: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+' > >> + > > Best regards, > Krzysztof >
On 09/06/2023 12:22, Claudiu.Beznea@microchip.com wrote: > On 31.05.2023 11:55, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 30/05/2023 11:07, Claudiu Beznea wrote: >>> Convert Microchip AT91 PIT bindings to YAML. Along with it clocks and >>> clock-names bindings were added as the drivers needs it to ensure proper >>> hardware functionality. >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >>> --- >>> .../devicetree/bindings/arm/atmel-sysregs.txt | 12 --- >>> .../bindings/timer/atmel,at91sam9260-pit.yaml | 99 +++++++++++++++++++ >>> 2 files changed, 99 insertions(+), 12 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >>> index 67a66bf74895..54d3f586403e 100644 >>> --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >>> +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt >>> @@ -4,18 +4,6 @@ Chipid required properties: >>> - compatible: Should be "atmel,sama5d2-chipid" or "microchip,sama7g5-chipid" >>> - reg : Should contain registers location and length >>> >>> -PIT Timer required properties: >>> -- compatible: Should be "atmel,at91sam9260-pit" >>> -- reg: Should contain registers location and length >>> -- interrupts: Should contain interrupt for the PIT which is the IRQ line >>> - shared across all System Controller members. >>> - >>> -PIT64B Timer required properties: >>> -- compatible: Should be "microchip,sam9x60-pit64b" >>> -- reg: Should contain registers location and length >>> -- interrupts: Should contain interrupt for PIT64B timer >>> -- clocks: Should contain the available clock sources for PIT64B timer. >>> - >>> System Timer (ST) required properties: >>> - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd" >>> - reg: Should contain registers location and length >>> diff --git a/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >>> new file mode 100644 >>> index 000000000000..d0f3f80db4cb >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml >>> @@ -0,0 +1,99 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/timer/atmel,at91sam9260-pit.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Microchip AT91 Periodic Interval Timer (PIT) >>> + >>> +maintainers: >>> + - Claudiu Beznea <claudiu.beznea@microchip.com> >>> + >>> +description: >>> + Microchip AT91 periodic interval timer provides the operating system scheduler >>> + interrupt. It is designed to offer maximum accuracy and efficient management, >>> + even for systems with long response time. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: microchip,sama7g5-pit64b >> >> >From where do you have this compatible? Wasn't in old binding and commit >> msg does not explain it. > > ok, I'll update it in the commit message. It is from the available device > trees. Add them in next patch, so the conversion is only conversion. > >> >> >>> + - const: microchip,sam9x60-pit64b >>> + - items: >>> + enum: >> >> These are not items. Just enum.. Does it even work? > > Yes, it compiles w/o issues. I'll update it anyway. Yeah, but does it work as intended? This should allow any order of below compatibles - from 1 to 2, so totally not what you wanted. > >> >>> + - atmel,at91sam9260-pit >>> + - microchip,sam9x60-pit64b >>> + properties: >>> + clocks: >>> + minItems: 2 >>> + clock-names: >>> + items: >>> + - const: pclk >>> + - const: gclk >> >> interrupts? They are still required, so why no description here? > > It was here in the previous versions but Conor suggested to remove it as it > was nothing specific about this description. For the if-then branch I kept > it to specify that the interrupt is share with other devices. In this > branch the interrupt is only for the timer itself. With this, would you > still prefer to add it back? I just don't understand why interrupts are in one arm of the if: and not in the other. > >> >>> + required: >>> + - clock-names >>> + >>> +unevaluatedProperties: false >> >> additionalProperties:false instead > > Having additionalProperties:false instead of unevaluatedProperties: false > thows the following error on make dt_binding_check and make dtbs_check: > > Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.example.dtb: > timer@f0028000: 'clock-names' does not match any of the regexes: > 'pinctrl-[0-9]+' So it nicely points to something you need to fix in the binding. Best regards, Krzysztof
On 09.06.2023 13:48, Krzysztof Kozlowski wrote: [ ... ] > >>>> + properties: >>>> + clocks: >>>> + minItems: 2 >>>> + clock-names: >>>> + items: >>>> + - const: pclk >>>> + - const: gclk >>> >>> interrupts? They are still required, so why no description here? >> >> It was here in the previous versions but Conor suggested to remove it as it >> was nothing specific about this description. For the if-then branch I kept >> it to specify that the interrupt is share with other devices. In this >> branch the interrupt is only for the timer itself. With this, would you >> still prefer to add it back? > > I just don't understand why interrupts are in one arm of the if: and not > in the other. > As previously mentioned, Conor suggested to have it like this.
On 09/06/2023 14:09, Claudiu.Beznea@microchip.com wrote: >>>> >>>> interrupts? They are still required, so why no description here? >>> >>> It was here in the previous versions but Conor suggested to remove it as it >>> was nothing specific about this description. For the if-then branch I kept >>> it to specify that the interrupt is share with other devices. In this >>> branch the interrupt is only for the timer itself. With this, would you >>> still prefer to add it back? >> >> I just don't understand why interrupts are in one arm of the if: and not >> in the other. >> > > As previously mentioned, Conor suggested to have it like this. > ok Best regards, Krzysztof
On 09.06.2023 15:18, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 09/06/2023 14:09, Claudiu.Beznea@microchip.com wrote: >>>>> >>>>> interrupts? They are still required, so why no description here? >>>> >>>> It was here in the previous versions but Conor suggested to remove it as it >>>> was nothing specific about this description. For the if-then branch I kept >>>> it to specify that the interrupt is share with other devices. In this >>>> branch the interrupt is only for the timer itself. With this, would you >>>> still prefer to add it back? >>> >>> I just don't understand why interrupts are in one arm of the if: and not >>> in the other. >>> >> >> As previously mentioned, Conor suggested to have it like this. >> > > ok Would you still prefer to add back interrupt description on both branches of if? > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt index 67a66bf74895..54d3f586403e 100644 --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt @@ -4,18 +4,6 @@ Chipid required properties: - compatible: Should be "atmel,sama5d2-chipid" or "microchip,sama7g5-chipid" - reg : Should contain registers location and length -PIT Timer required properties: -- compatible: Should be "atmel,at91sam9260-pit" -- reg: Should contain registers location and length -- interrupts: Should contain interrupt for the PIT which is the IRQ line - shared across all System Controller members. - -PIT64B Timer required properties: -- compatible: Should be "microchip,sam9x60-pit64b" -- reg: Should contain registers location and length -- interrupts: Should contain interrupt for PIT64B timer -- clocks: Should contain the available clock sources for PIT64B timer. - System Timer (ST) required properties: - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd" - reg: Should contain registers location and length diff --git a/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml new file mode 100644 index 000000000000..d0f3f80db4cb --- /dev/null +++ b/Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/timer/atmel,at91sam9260-pit.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip AT91 Periodic Interval Timer (PIT) + +maintainers: + - Claudiu Beznea <claudiu.beznea@microchip.com> + +description: + Microchip AT91 periodic interval timer provides the operating system scheduler + interrupt. It is designed to offer maximum accuracy and efficient management, + even for systems with long response time. + +properties: + compatible: + oneOf: + - items: + - const: microchip,sama7g5-pit64b + - const: microchip,sam9x60-pit64b + - items: + enum: + - atmel,at91sam9260-pit + - microchip,sam9x60-pit64b + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + minItems: 1 + maxItems: 2 + +required: + - compatible + - reg + - interrupts + - clocks + +allOf: + - if: + properties: + compatible: + contains: + const: atmel,at91sam9260-pit + then: + properties: + interrupts: + description: + Shared interrupt between all system controller members (power management + controller, watchdog, PIT, reset controller, real-time timer, real-time + clock, memory controller, debug unit, system timer). + clocks: + maxItems: 1 + + else: + properties: + clocks: + minItems: 2 + clock-names: + items: + - const: pclk + - const: gclk + required: + - clock-names + +unevaluatedProperties: false + +examples: + - | + /* AT91RM9200 */ + #include <dt-bindings/clock/at91.h> + #include <dt-bindings/interrupt-controller/irq.h> + + pit: timer@fffffe40 { + compatible = "atmel,at91sam9260-pit"; + reg = <0xfffffe40 0x10>; + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; + clocks = <&pmc PMC_TYPE_CORE PMC_MCK>; + }; + + - | + /* SAM9X60 */ + #include <dt-bindings/clock/at91.h> + #include <dt-bindings/interrupt-controller/irq.h> + + pit64b: timer@f0028000 { + compatible = "microchip,sam9x60-pit64b"; + reg = <0xf0028000 0x100>; + interrupts = <37 IRQ_TYPE_LEVEL_HIGH 7>; + clocks = <&pmc PMC_TYPE_PERIPHERAL 37>, <&pmc PMC_TYPE_GCK 37>; + clock-names = "pclk", "gclk"; + }; + +...
Convert Microchip AT91 PIT bindings to YAML. Along with it clocks and clock-names bindings were added as the drivers needs it to ensure proper hardware functionality. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- .../devicetree/bindings/arm/atmel-sysregs.txt | 12 --- .../bindings/timer/atmel,at91sam9260-pit.yaml | 99 +++++++++++++++++++ 2 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 Documentation/devicetree/bindings/timer/atmel,at91sam9260-pit.yaml