Re: [RFC PATCH v2 1/4] dt-bindings: clock: renesas: Document RZ/V2H(P) SoC CPG

Geert Uytterhoeven
2024-06-26T09:35:36Z

Hi Krzysztof,

On Thu, Jun 13, 2024 at 2:57 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 13/06/2024 11:53, Lad, Prabhakar wrote:
> > On Tue, Jun 11, 2024 at 8:02 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >> On 11/06/2024 01:32, Prabhakar wrote:
> >>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>>
> >>> 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@bp.renesas.com>

> >>> + '#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.

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.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds