Re: [PATCH v5 1/7] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

Hey Xingyu,

Couple nitpick items to be fixed if you resubmit.

On Tue, Jun 13, 2023 at 08:58:46PM +0800, Xingyu Wu wrote:
> +  This PLL are high speed, low jitter frequency synthesizers in JH7110.

nit: These PLLs are

> +  Each PLL clocks work in integer mode or fraction mode by some dividers,
> +  and the configuration registers and dividers are set in several syscon
> +  registers. So pll node should be a child of SYS-SYSCON node.

nit: Each PLL can work in integer or fractional mode, with controlled by
configuration registers in the sys syscon.

> +  The formula for calculating frequency is that,

nit: s/ that//

> +examples:
> +  - |
> +    pll-clock-controller {

nit: s/pll-//

> diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
> index 06257bfd9ac1..086a6ddcf380 100644
> --- a/include/dt-bindings/clock/starfive,jh7110-crg.h
> +++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
> @@ -6,6 +6,12 @@
>  #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
>  #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
>  
> +/* PLL clocks */
> +#define JH7110_CLK_PLL0_OUT			0
> +#define JH7110_CLK_PLL1_OUT			1
> +#define JH7110_CLK_PLL2_OUT			2
> +#define JH7110_PLLCLK_END			3

Please CC me on the patches fixing this for U-Boot :)

Nitpicking aside, which only needs to be fixed if you end up submitting
a new version for other reasons,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]