On 05/05/2023 14:46, Claudiu.Beznea@xxxxxxxxxxxxx wrote: > On 04.05.2023 15:47, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 04/05/2023 08:07, Claudiu Beznea wrote: >>> Convert Atmel PMC documentation to yaml. >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> >> >> Thank you for your patch. There is something to discuss/improve. >> >> >>> diff --git a/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >>> new file mode 100644 >>> index 000000000000..c4023c3a85f2 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/atmel,at91rm9200-pmc.yaml >>> @@ -0,0 +1,161 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: "http://devicetree.org/schemas/clock/atmel,at91rm9200-pmc.yaml#" >>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> >> Drop quotes from both. >> >>> + >>> +title: Atmel Power Management Controller (PMC) >>> + >>> +maintainers: >>> + - Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx> >>> + >>> +description: >>> + The power management controller optimizes power consumption by controlling all >>> + system and user peripheral clocks. The PMC enables/disables the clock inputs >>> + to many of the peripherals and to the processor. >>> + >>> +properties: >>> + compatible: >>> + oneOf: >>> + - items: >>> + - const: atmel,at91sam9260-pmc >> >> Why this is separate, not part of bottom enum? > > Current device trees uses the following compatibles (among others): > - "atmel,at91sam9260-pmc, "syscon" or > - "atmel,at91sam9g20-pmc", "atmel,at91sam9260-pmc, "syscon" > > I haven't found another way to make dtbs_check happy. > Is there another way for this? I mean the enum at the bottom of all compatibles. >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + >>> + pmc: clock-controller@f0018000 { >>> + compatible = "atmel,sama5d4-pmc", "syscon"; >>> + reg = <0xf0018000 0x120>; >>> + interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>; >> >> interrupt looks a bit odd. Are you sure it is correct? > > This example is from SAMA5D4 SoC which uses a vendor specific interrupt > controller (Atmel AIC) where: > - 1st cell is the interrupt number > - 2nd cell is the interrupt type (level/edge sensitive) > - 3rd cell is the IRQ priority ok Best regards, Krzysztof