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 <stano.jakubek@xxxxxxxxx> > > > > 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
