On 09/06/2023 12:22, Claudiu.Beznea@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx> >>> --- >>> .../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@xxxxxxxxxxxxx> >>> + >>> +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
