Message ID | 20240610233221.242749-2-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Add CPG support for RZ/V2H(P) SoC | expand |
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> Since this is not a finished work (RFC), only limited review follows. > +$id: http://devicetree.org/schemas/clock/renesas,rzv2h-cpg.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas RZ/V2H(P) Clock Pulse Generator (CPG) > + > +maintainers: > + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + On Renesas RZ/V2H(P) SoCs, the CPG (Clock Pulse Generator) handles generation > + and control of clock signals for the IP modules, generation and control of resets, > + and control over booting, low power consumption and power supply domains. > + > +properties: > + compatible: > + const: renesas,r9a09g057-cpg > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + description: > + Clock source to CPG on QEXTAL pin. > + const: qextal > + > + '#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? > + - 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. > + const: 2 > + > + '#power-domain-cells': > + description: > + SoC devices that are part of the CPG/Module Standby Mode Clock Domain and > + can be power-managed through Module Standby should refer to the CPG device > + node in their "power-domains" property, as documented by the generic PM > + Domain bindings in Documentation/devicetree/bindings/power/power-domain.yaml. Drop description, it's redundant. > + const: 0 > + > + '#reset-cells': > + description: > + The single reset specifier cell must be the reset number. The reset number > + is calculated as the reset register offset index multiplied by 16, plus the > + actual bit in the register used to reset the specific IP block. For example, > + for SYS_0_PRESETN, the calculation is (3 * 16 + 0) = 48. > + const: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - '#clock-cells' > + - '#power-domain-cells' > + - '#reset-cells' > + > +additionalProperties: false > + > +examples: > + - | > + cpg: clock-controller@10420000 { Drop unused label. > + compatible = "renesas,r9a09g057-cpg"; Use 4 spaces for example indentation. > + }; Best regards, Krzysztof
Hi Krzysztof, Thank you for the review. 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> > > Since this is not a finished work (RFC), only limited review follows. > > > > +$id: http://devicetree.org/schemas/clock/renesas,rzv2h-cpg.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Renesas RZ/V2H(P) Clock Pulse Generator (CPG) > > + > > +maintainers: > > + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > + > > +description: | > > Do not need '|' unless you need to preserve formatting. > OK. > > + On Renesas RZ/V2H(P) SoCs, the CPG (Clock Pulse Generator) handles generation > > + and control of clock signals for the IP modules, generation and control of resets, > > + and control over booting, low power consumption and power supply domains. > > + > > +properties: > > + compatible: > > + const: renesas,r9a09g057-cpg > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + description: > > + Clock source to CPG on QEXTAL pin. > > + const: qextal > > + > > + '#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. > > + const: 2 > > + > > + '#power-domain-cells': > > + description: > > + SoC devices that are part of the CPG/Module Standby Mode Clock Domain and > > + can be power-managed through Module Standby should refer to the CPG device > > + node in their "power-domains" property, as documented by the generic PM > > + Domain bindings in Documentation/devicetree/bindings/power/power-domain.yaml. > > Drop description, it's redundant. > OK. > > + const: 0 > > + > > + '#reset-cells': > > + description: > > + The single reset specifier cell must be the reset number. The reset number > > + is calculated as the reset register offset index multiplied by 16, plus the > > + actual bit in the register used to reset the specific IP block. For example, > > + for SYS_0_PRESETN, the calculation is (3 * 16 + 0) = 48. > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - '#clock-cells' > > + - '#power-domain-cells' > > + - '#reset-cells' > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + cpg: clock-controller@10420000 { > > Drop unused label. > OK. > > + compatible = "renesas,r9a09g057-cpg"; > > Use 4 spaces for example indentation. > Sure, I will update it. Cheers, Prabhakar
Hi Geert, On Tue, Jun 11, 2024 at 12:32 AM Prabhakar <prabhakar.csengg@gmail.com> 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> > --- > Hi Geert, > > WRT XIN_{RTCCLK/AUDCLK/MAINCLK)clks going to CPG I have created an > internal request for clarification if the clocks are inputs to CPG > or to respective clocks. As the board schematic doesnt have any of > these. For now I have just kept qextal clk as input to CPG. > I have got the feedback from the manual team. The XIN_* clocks will be renamed as below (and the block diagram will be updated), XIN_MAINCLK -> QXCLK XIN_RTCCLK -> RTX_XCLK XIN_AUDCLK -> AUDIO_XCLK From section 4.2.1.7 Functional Block diagram (page 359) we have the below, RTXIN ----------------> PFC ------> RTX_XCLK --------> CPG QEXTAL--------------> PFC ------> QXCLK -------------> CPG AUDIO_EXTAL-----> PFC ------> AUDIO_XCLK ----> CPG How should we model this now, please provide your feedback? Cheers, Prabhakar
On 13/06/2024 11:53, Lad, Prabhakar wrote: > Hi Krzysztof, > > Thank you for the review. > > 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> >> >> Since this is not a finished work (RFC), only limited review follows. >> >> >>> +$id: http://devicetree.org/schemas/clock/renesas,rzv2h-cpg.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Renesas RZ/V2H(P) Clock Pulse Generator (CPG) >>> + >>> +maintainers: >>> + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >>> + >>> +description: | >> >> Do not need '|' unless you need to preserve formatting. >> > OK. > >>> + On Renesas RZ/V2H(P) SoCs, the CPG (Clock Pulse Generator) handles generation >>> + and control of clock signals for the IP modules, generation and control of resets, >>> + and control over booting, low power consumption and power supply domains. >>> + >>> +properties: >>> + compatible: >>> + const: renesas,r9a09g057-cpg >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + clock-names: >>> + description: >>> + Clock source to CPG on QEXTAL pin. >>> + const: qextal >>> + >>> + '#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? Anyway, that's not an argument for bindings. Fix your driver design. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml new file mode 100644 index 000000000000..03085308154c --- /dev/null +++ b/Documentation/devicetree/bindings/clock/renesas,rzv2h-cpg.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/renesas,rzv2h-cpg.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas RZ/V2H(P) Clock Pulse Generator (CPG) + +maintainers: + - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> + +description: | + On Renesas RZ/V2H(P) SoCs, the CPG (Clock Pulse Generator) handles generation + and control of clock signals for the IP modules, generation and control of resets, + and control over booting, low power consumption and power supply domains. + +properties: + compatible: + const: renesas,r9a09g057-cpg + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + description: + Clock source to CPG on QEXTAL pin. + const: qextal + + '#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>, + - 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. + const: 2 + + '#power-domain-cells': + description: + SoC devices that are part of the CPG/Module Standby Mode Clock Domain and + can be power-managed through Module Standby should refer to the CPG device + node in their "power-domains" property, as documented by the generic PM + Domain bindings in Documentation/devicetree/bindings/power/power-domain.yaml. + const: 0 + + '#reset-cells': + description: + The single reset specifier cell must be the reset number. The reset number + is calculated as the reset register offset index multiplied by 16, plus the + actual bit in the register used to reset the specific IP block. For example, + for SYS_0_PRESETN, the calculation is (3 * 16 + 0) = 48. + const: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - '#clock-cells' + - '#power-domain-cells' + - '#reset-cells' + +additionalProperties: false + +examples: + - | + cpg: clock-controller@10420000 { + compatible = "renesas,r9a09g057-cpg"; + reg = <0x10420000 0x10000>; + clocks = <&extal_clk>; + clock-names = "qextal"; + #clock-cells = <2>; + #power-domain-cells = <0>; + #reset-cells = <1>; + };