On 26/06/2024 11:35, Geert Uytterhoeven wrote: > Hi Krzysztof, > > On Thu, Jun 13, 2024 at 2:57 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >> On 13/06/2024 11:53, Lad, Prabhakar wrote: >>> On Tue, Jun 11, 2024 at 8:02 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >>>> On 11/06/2024 01:32, Prabhakar wrote: >>>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> >>>>> >>>>> Document the device tree bindings for the Renesas RZ/V2H(P) SoC >>>>> Clock Pulse Generator (CPG). >>>>> >>>>> CPG block handles the below operations: >>>>> - Generation and control of clock signals for the IP modules >>>>> - Generation and control of resets >>>>> - Control over booting >>>>> - Low power consumption and power supply domains >>>>> >>>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > >>>>> + '#clock-cells': >>>>> + description: | >>>>> + - For CPG core clocks, the two clock specifier cells must be "CPG_CORE" >>>>> + and a core clock reference, as defined in >>>>> + <dt-bindings/clock/r9a09g057-cpg.h>, >>>> >>>> So second cell is not used? >>>> >>> It will be used for blocks using core clocks. >>> >>>>> + - For module clocks, the two clock specifier cells must be "CPG_MOD" and >>>>> + a module number. The module number is calculated as the CLKON register >>>>> + offset index multiplied by 16, plus the actual bit in the register >>>>> + used to turn the CLK ON. For example, for CGC_GIC_0_GICCLK, the >>>>> + calculation is (1 * 16 + 3) = 19. >>>> >>>> You should not have different values. Make it const: 1 and just use IDs. >>>> >>> Are you suggesting not to differentiate between core/mod clocks. They >>> are differentiated because the MOD clocks can turned ON/OFF but where >>> as with the core clocks we cannot turn them ON/OF so the driver needs >>> to know this, hence two specifiers are used. >> >> Every driver knows it... I am really, what is the problem here? Are you >> saying the drivers create some unknown clocks? > > The driver knows for sure which clocks are module clocks, and thus can > be used for power management. To simplify the driver, two separate > numbers spaces are used: > 1. Core clock numbers come from IDs in the DT binding headers, > 2. Module clock numbers come straight[1] from the hardware docs. > As the latter are fixed, merging them into a single number space in > a future-proof way is hard[2], the bindings use 2 clock cells. IIUC, your module clock numbers are not DT ABI and should not be put into the binding headers. I think that's the case currently, right? If above is correct, considering your explanation I am fine. Thanks for the time to make it clear. > > Alternatively, a unified number space using IDs in the DT binding > headers could be used, as you suggest. > > [1] "straight" may be a misnomer here, as the DT writer still has to > calculate the number from register index and bit index: > > n = register index * 16 + bit index > > i.e. register index 1 and register bit 3 become 19. > > In the R-Car series, this is handled slightly more elegant > (IMHO ;-), and easier to the human eye, by using a sparse > number space: > > n = register index * 100 + bit index > > i.e. register index 1 and register bit 3 become 103. > Which also matches how the bits were named in older SH-Mobile > hardware docs. > > [2] One could use an offset to indicate core or module clocks, but > future SoCs in the family may have more clocks. > Best regards, Krzysztof