Hi, Rob, On 12/21/23 07:20, Tudor Ambarus wrote: > > > On 12/20/23 15:07, Rob Herring wrote: >> On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: >>> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) >>> clock management unit. >>> >>> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> >>> --- >>> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- >>> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ >>> 2 files changed, 109 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> index 3eebc03a309b..ba54c13c55bc 100644 >>> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> @@ -30,14 +30,15 @@ properties: >>> - google,gs101-cmu-top >>> - google,gs101-cmu-apm >>> - google,gs101-cmu-misc >>> + - google,gs101-cmu-peric0 >>> >>> clocks: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 3 >>> >>> clock-names: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 3 >>> >>> "#clock-cells": >>> const: 1 >>> @@ -88,6 +89,26 @@ allOf: >>> - const: dout_cmu_misc_bus >>> - const: dout_cmu_misc_sss >>> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: google,gs101-cmu-peric0 >>> + >>> + then: >>> + properties: >>> + clocks: >>> + items: >>> + - description: External reference clock (24.576 MHz) >>> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) >>> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) >>> + >>> + clock-names: >>> + items: >>> + - const: oscclk >>> + - const: dout_cmu_peric0_bus >>> + - const: dout_cmu_peric0_ip >> >> 'bus' and 'ip' are sufficient because naming is local to the module. The >> same is true on 'dout_cmu_misc_bus'. As that has not made a release, >> please fix all of them. >> > > Ok, will fix them shortly. Thanks, Rob! I tried your suggestion at https://lore.kernel.org/linux-arm-kernel/c6cc6e74-6c3d-439b-8dc1-bc50a88a3d8f@xxxxxxxxxx/ and we noticed that we'd have to update the clock driver as well. These CMUs set the DT clock-name of the parent clock in the driver in struct samsung_cmu_info::clk_name[]. The driver then tries to enable the parent clock based on the clock-name in exynos_arm64_register_cmu(). In order to enable the parent clock of the CMU the following would be needed in the driver: diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c index 68a27b78b00b..e91836ea3a98 100644 --- a/drivers/clk/samsung/clk-gs101.c +++ b/drivers/clk/samsung/clk-gs101.c @@ -2476,7 +2476,7 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = { .nr_clk_ids = CLKS_NR_MISC, .clk_regs = misc_clk_regs, .nr_clk_regs = ARRAY_SIZE(misc_clk_regs), - .clk_name = "dout_cmu_misc_bus", + .clk_name = "bus", }; I think I lean towards keeping the name as it was because it's clearer what are the clock dependencies in the driver. "dout_cmu_misc_bus" is the clock name used when defining the clock: DIV(CLK_DOUT_CMU_MISC_BUS, "dout_cmu_misc_bus", "gout_cmu_misc_bus", CLK_CON_DIV_CLKCMU_MISC_BUS, 0, 4), The other exynos clock drivers are using too the driver's clock names for the clock-names device tree property. For consistency I'd keep it the same. If you have a stronger opinion than mine, please tell and I'll happily update the driver. Thanks, ta
