Convert Broadcom Kona family clock controller unit (CCU) bindings
to DT schema.
Signed-off-by: Stanislav Jakubek <[email protected]>
---
.../bindings/clock/brcm,kona-ccu.txt | 138 ---------------
.../bindings/clock/brcm,kona-ccu.yaml | 158 ++++++++++++++++++
2 files changed, 158 insertions(+), 138 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/brcm,kona-ccu.txt
create mode 100644 Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
diff --git a/Documentation/devicetree/bindings/clock/brcm,kona-ccu.txt b/Documentation/devicetree/bindings/clock/brcm,kona-ccu.txt
deleted file mode 100644
index 8e5a7d868557..000000000000
--- a/Documentation/devicetree/bindings/clock/brcm,kona-ccu.txt
+++ /dev/null
@@ -1,138 +0,0 @@
-Broadcom Kona Family Clocks
-
-This binding is associated with Broadcom SoCs having "Kona" style
-clock control units (CCUs). A CCU is a clock provider that manages
-a set of clock signals. Each CCU is represented by a node in the
-device tree.
-
-This binding uses the common clock binding:
- Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Required properties:
-- compatible
- Shall have a value of the form "brcm,<model>-<which>-ccu",
- where <model> is a Broadcom SoC model number and <which> is
- the name of a defined CCU. For example:
- "brcm,bcm11351-root-ccu"
- The compatible strings used for each supported SoC family
- are defined below.
-- reg
- Shall define the base and range of the address space
- containing clock control registers
-- #clock-cells
- Shall have value <1>. The permitted clock-specifier values
- are defined below.
-- clock-output-names
- Shall be an ordered list of strings defining the names of
- the clocks provided by the CCU.
-
-Device tree example:
-
- slave_ccu: slave_ccu {
- compatible = "brcm,bcm11351-slave-ccu";
- reg = <0x3e011000 0x0f00>;
- #clock-cells = <1>;
- clock-output-names = "uartb",
- "uartb2",
- "uartb3",
- "uartb4";
- };
-
- ref_crystal_clk: ref_crystal {
- #clock-cells = <0>;
- compatible = "fixed-clock";
- clock-frequency = <26000000>;
- };
-
- uart@3e002000 {
- compatible = "brcm,bcm11351-dw-apb-uart", "snps,dw-apb-uart";
- reg = <0x3e002000 0x1000>;
- clocks = <&slave_ccu BCM281XX_SLAVE_CCU_UARTB3>;
- interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
- reg-shift = <2>;
- reg-io-width = <4>;
- };
-
-BCM281XX family
----------------
-CCU compatible string values for SoCs in the BCM281XX family are:
- "brcm,bcm11351-root-ccu"
- "brcm,bcm11351-aon-ccu"
- "brcm,bcm11351-hub-ccu"
- "brcm,bcm11351-master-ccu"
- "brcm,bcm11351-slave-ccu"
-
-The following table defines the set of CCUs and clock specifiers for
-BCM281XX family clocks. When a clock consumer references a clocks,
-its symbolic specifier (rather than its numeric index value) should
-be used. These 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
-
-
-BCM21664 family
----------------
-CCU compatible string values for SoCs in the BCM21664 family are:
- "brcm,bcm21664-root-ccu"
- "brcm,bcm21664-aon-ccu"
- "brcm,bcm21664-master-ccu"
- "brcm,bcm21664-slave-ccu"
-
-The following table defines the set of CCUs and clock specifiers for
-BCM21664 family clocks. When a clock consumer references a clocks,
-its symbolic specifier (rather than its numeric index value) should
-be used. These 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
diff --git a/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml b/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
new file mode 100644
index 000000000000..80c834951d6d
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/brcm,kona-ccu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom Kona family clock control units (CCU)
+
+maintainers:
+ - Florian Fainelli <[email protected]>
+ - Ray Jui <[email protected]>
+ - Scott Branden <[email protected]>
+
+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
+ - 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
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clock-output-names
+
+additionalProperties: false
+
+examples:
+ - |
+ clock-controller@3e011000 {
+ compatible = "brcm,bcm11351-slave-ccu";
+ reg = <0x3e011000 0x0f00>;
+ #clock-cells = <1>;
+ clock-output-names = "uartb",
+ "uartb2",
+ "uartb3",
+ "uartb4",
+ "ssp0",
+ "ssp2",
+ "bsc1",
+ "bsc2",
+ "bsc3",
+ "pwm";
+ };
+...
--
2.34.1
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 <[email protected]>
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
On Mon, Oct 23, 2023 at 09:54:49AM +0200, Krzysztof Kozlowski wrote:
> 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 <[email protected]>
>
> 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.
Hi Krzysztof, you're correct that I just copied this from the old bindings.
brcm,iproc-clocks.yaml has a similar table, so I thought this would be fine.
I'm OK with dropping it, but how should I document the clock-output-names
values then? A bunch of if-then blocks (per compatible)? Or should I not even
bother and just keep minItems/maxItems without documenting the values?
>
> > + - 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
I've also noticed that dtbs_check gives out warnings(?) like this for
bcm21664 ccu nodes:
/arch/arm/boot/dts/broadcom/bcm21664-garnet.dtb:
root_ccu@35001000: clock-output-names: ['frac_1m'] is too short
from schema $id: http://devicetree.org/schemas/clock/brcm,kona-ccu.yaml#
and this maxItems:8 seems to me like the culprit (since the bcm11351 if-then
doesn't have that). Seems to me like it also overrides the minItems to be 8
as well. I don't understand why it would do that though.
I suppose just adding minItems: 1 would be the correct fix in this case?
> > + 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.
Ack.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - '#clock-cells'
> > + - clock-output-names
> > +
> > +additionalProperties: false
> > +
> Best regards,
> Krzysztof
>
Thanks for the feedback,
Stanislav
On 23/10/2023 22:17, Stanislav Jakubek wrote:
> On Mon, Oct 23, 2023 at 09:54:49AM +0200, Krzysztof Kozlowski wrote:
>> 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 <[email protected]>
>>
>> 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.
>
> Hi Krzysztof, you're correct that I just copied this from the old bindings.
> brcm,iproc-clocks.yaml has a similar table, so I thought this would be fine.
> I'm OK with dropping it, but how should I document the clock-output-names
> values then?
Your schema does not document them, so I don't understand what would you
loose.
> A bunch of if-then blocks (per compatible)? Or should I not even
> bother and just keep minItems/maxItems without documenting the values?
But what do you want to document exactly? Only number of items is
reasonable to constrain and it can be done with if:then blocks.
>
>>
>>> + - 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
>
> I've also noticed that dtbs_check gives out warnings(?) like this for
> bcm21664 ccu nodes:
>
> /arch/arm/boot/dts/broadcom/bcm21664-garnet.dtb:
> root_ccu@35001000: clock-output-names: ['frac_1m'] is too short
> from schema $id: http://devicetree.org/schemas/clock/brcm,kona-ccu.yaml#
>
> and this maxItems:8 seems to me like the culprit (since the bcm11351 if-then
> doesn't have that). Seems to me like it also overrides the minItems to be 8
> as well. I don't understand why it would do that though.
>
> I suppose just adding minItems: 1 would be the correct fix in this case?
https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
Best regards,
Krzysztof
On Mon, Oct 23, 2023 at 10:17:22PM +0200, Stanislav Jakubek wrote:
> On Mon, Oct 23, 2023 at 09:54:49AM +0200, Krzysztof Kozlowski wrote:
> > 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 <[email protected]>
> >
> > 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.
>
> Hi Krzysztof, you're correct that I just copied this from the old bindings.
> brcm,iproc-clocks.yaml has a similar table, so I thought this would be fine.
> I'm OK with dropping it, but how should I document the clock-output-names
> values then? A bunch of if-then blocks (per compatible)? Or should I not even
> bother and just keep minItems/maxItems without documenting the values?
>
> >
> > > + - 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
>
> I've also noticed that dtbs_check gives out warnings(?) like this for
> bcm21664 ccu nodes:
>
> /arch/arm/boot/dts/broadcom/bcm21664-garnet.dtb:
> root_ccu@35001000: clock-output-names: ['frac_1m'] is too short
> from schema $id: http://devicetree.org/schemas/clock/brcm,kona-ccu.yaml#
>
> and this maxItems:8 seems to me like the culprit (since the bcm11351 if-then
> doesn't have that). Seems to me like it also overrides the minItems to be 8
> as well. I don't understand why it would do that though.
Indeed it does. That should be fixed soon such that minItems/maxItems
will never be added implicitly to if/then/else schemas.
Rob