[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3df06d79-ea51-4202-8cc8-468f741603bf@linaro.org>
Date: Mon, 23 Oct 2023 09:54:49 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Stanislav Jakubek <stano.jakubek@...il.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>
Cc: bcm-kernel-feedback-list@...adcom.com, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Artur Weber <aweber.kernel@...il.com>
Subject: Re: [PATCH] dt-bindings: clock: brcm,kona-ccu: convert to YAML
On 22/10/2023 13:31, Stanislav Jakubek wrote:
> Convert Broadcom Kona family clock controller unit (CCU) bindings
> to DT schema.
>
> Signed-off-by: Stanislav Jakubek <stano.jakubek@...il.com>
Thank you for your patch. There is something to discuss/improve.
> +description:
> + Broadcom "Kona" style clock control unit (CCU) is a clock provider that
> + manages a set of clock signals.
> +
> +properties:
> + compatible:
> + enum:
> + - brcm,bcm11351-aon-ccu
> + - brcm,bcm11351-hub-ccu
> + - brcm,bcm11351-master-ccu
> + - brcm,bcm11351-root-ccu
> + - brcm,bcm11351-slave-ccu
> + - brcm,bcm21664-aon-ccu
> + - brcm,bcm21664-master-ccu
> + - brcm,bcm21664-root-ccu
> + - brcm,bcm21664-slave-ccu
> +
> + reg:
> + maxItems: 1
> +
> + '#clock-cells':
> + const: 1
> +
> + clock-output-names:
> + minItems: 1
> + maxItems: 10
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,bcm11351-aon-ccu
> + - brcm,bcm11351-hub-ccu
> + - brcm,bcm11351-master-ccu
> + - brcm,bcm11351-root-ccu
> + - brcm,bcm11351-slave-ccu
> + then:
> + properties:
> + clock-output-names:
> + description: |
> + The following table defines the set of CCUs and clock specifiers
> + for BCM281XX family clocks.
> + These clock specifiers are defined in:
> + "include/dt-bindings/clock/bcm281xx.h"
> +
> + CCU Clock Type Index Specifier
> + --- ----- ---- ----- ---------
> + root frac_1m peri 0 BCM281XX_ROOT_CCU_FRAC_1M
> +
> + aon hub_timer peri 0 BCM281XX_AON_CCU_HUB_TIMER
> + aon pmu_bsc peri 1 BCM281XX_AON_CCU_PMU_BSC
> + aon pmu_bsc_var peri 2 BCM281XX_AON_CCU_PMU_BSC_VAR
> +
> + hub tmon_1m peri 0 BCM281XX_HUB_CCU_TMON_1M
> +
> + master sdio1 peri 0 BCM281XX_MASTER_CCU_SDIO1
> + master sdio2 peri 1 BCM281XX_MASTER_CCU_SDIO2
> + master sdio3 peri 2 BCM281XX_MASTER_CCU_SDIO3
> + master sdio4 peri 3 BCM281XX_MASTER_CCU_SDIO4
> + master dmac peri 4 BCM281XX_MASTER_CCU_DMAC
> + master usb_ic peri 5 BCM281XX_MASTER_CCU_USB_IC
> + master hsic2_48m peri 6 BCM281XX_MASTER_CCU_HSIC_48M
> + master hsic2_12m peri 7 BCM281XX_MASTER_CCU_HSIC_12M
> +
> + slave uartb peri 0 BCM281XX_SLAVE_CCU_UARTB
> + slave uartb2 peri 1 BCM281XX_SLAVE_CCU_UARTB2
> + slave uartb3 peri 2 BCM281XX_SLAVE_CCU_UARTB3
> + slave uartb4 peri 3 BCM281XX_SLAVE_CCU_UARTB4
> + slave ssp0 peri 4 BCM281XX_SLAVE_CCU_SSP0
> + slave ssp2 peri 5 BCM281XX_SLAVE_CCU_SSP2
> + slave bsc1 peri 6 BCM281XX_SLAVE_CCU_BSC1
> + slave bsc2 peri 7 BCM281XX_SLAVE_CCU_BSC2
> + slave bsc3 peri 8 BCM281XX_SLAVE_CCU_BSC3
> + slave pwm peri 9 BCM281XX_SLAVE_CCU_PWM
I don't really understand why this is in the binding schema. I guess you
wanted to copy it from the old binding, but, unless there is real reason
for it, don't. The clock IDs should be in the header file and that's it.
Nothing here.
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,bcm21664-aon-ccu
> + - brcm,bcm21664-master-ccu
> + - brcm,bcm21664-root-ccu
> + - brcm,bcm21664-slave-ccu
> + then:
> + properties:
> + clock-output-names:
> + maxItems: 8
> + description: |
> + The following table defines the set of CCUs and clock specifiers
> + for BCM21664 family clocks.
> + These clock specifiers are defined in:
> + "include/dt-bindings/clock/bcm21664.h"
> +
> + CCU Clock Type Index Specifier
> + --- ----- ---- ----- ---------
> + root frac_1m peri 0 BCM21664_ROOT_CCU_FRAC_1M
> +
> + aon hub_timer peri 0 BCM21664_AON_CCU_HUB_TIMER
> +
> + master sdio1 peri 0 BCM21664_MASTER_CCU_SDIO1
> + master sdio2 peri 1 BCM21664_MASTER_CCU_SDIO2
> + master sdio3 peri 2 BCM21664_MASTER_CCU_SDIO3
> + master sdio4 peri 3 BCM21664_MASTER_CCU_SDIO4
> + master sdio1_sleep peri 4 BCM21664_MASTER_CCU_SDIO1_SLEEP
> + master sdio2_sleep peri 5 BCM21664_MASTER_CCU_SDIO2_SLEEP
> + master sdio3_sleep peri 6 BCM21664_MASTER_CCU_SDIO3_SLEEP
> + master sdio4_sleep peri 7 BCM21664_MASTER_CCU_SDIO4_SLEEP
> +
> + slave uartb peri 0 BCM21664_SLAVE_CCU_UARTB
> + slave uartb2 peri 1 BCM21664_SLAVE_CCU_UARTB2
> + slave uartb3 peri 2 BCM21664_SLAVE_CCU_UARTB3
> + slave uartb4 peri 3 BCM21664_SLAVE_CCU_UARTB4
> + slave bsc1 peri 4 BCM21664_SLAVE_CCU_BSC1
> + slave bsc2 peri 5 BCM21664_SLAVE_CCU_BSC2
> + slave bsc3 peri 6 BCM21664_SLAVE_CCU_BSC3
> + slave bsc4 peri 7 BCM21664_SLAVE_CCU_BSC4
Same comments.
In any case, allOf: goes after required: block.
> +
> +required:
> + - compatible
> + - reg
> + - '#clock-cells'
> + - clock-output-names
> +
> +additionalProperties: false
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists