All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Xingyu Wu <xingyu.wu@starfivetech.com>
Cc: linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Conor Dooley <conor+dt@kernel.org>,
	Emil Renner Berthing <emil.renner.berthing@canonical.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Hal Feng <hal.feng@starfivetech.com>,
	William Qiu <william.qiu@starfivetech.com>,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Subject: Re: [PATCH v5 1/7] dt-bindings: clock: Add StarFive JH7110 PLL clock generator
Date: Tue, 13 Jun 2023 20:06:05 +0100	[thread overview]
Message-ID: <20230613-surrender-surcharge-8778867e58c1@spud> (raw)
In-Reply-To: <20230613125852.211636-2-xingyu.wu@starfivetech.com>

[-- 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 --]

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: Xingyu Wu <xingyu.wu@starfivetech.com>
Cc: linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Conor Dooley <conor+dt@kernel.org>,
	Emil Renner Berthing <emil.renner.berthing@canonical.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Hal Feng <hal.feng@starfivetech.com>,
	William Qiu <william.qiu@starfivetech.com>,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Subject: Re: [PATCH v5 1/7] dt-bindings: clock: Add StarFive JH7110 PLL clock generator
Date: Tue, 13 Jun 2023 20:06:05 +0100	[thread overview]
Message-ID: <20230613-surrender-surcharge-8778867e58c1@spud> (raw)
In-Reply-To: <20230613125852.211636-2-xingyu.wu@starfivetech.com>


[-- Attachment #1.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2023-06-13 19:06 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13 12:58 [PATCH v5 0/7] Add PLL clocks driver and syscon for StarFive JH7110 SoC Xingyu Wu
2023-06-13 12:58 ` Xingyu Wu
2023-06-13 12:58 ` [PATCH v5 1/7] dt-bindings: clock: Add StarFive JH7110 PLL clock generator Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 19:06   ` Conor Dooley [this message]
2023-06-13 19:06     ` Conor Dooley
2023-06-13 12:58 ` [PATCH v5 2/7] dt-bindings: soc: starfive: Add StarFive syscon module Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 18:31   ` Krzysztof Kozlowski
2023-06-13 18:31     ` Krzysztof Kozlowski
2023-06-28  6:44     ` Xingyu Wu
2023-06-28  6:44       ` Xingyu Wu
2023-06-28 17:34       ` Conor Dooley
2023-06-28 17:34         ` Conor Dooley
2023-06-29  6:42         ` Xingyu Wu
2023-06-29  6:42           ` Xingyu Wu
2023-06-29  9:01           ` Conor Dooley
2023-06-29  9:01             ` Conor Dooley
2023-06-13 18:32   ` Krzysztof Kozlowski
2023-06-13 18:32     ` Krzysztof Kozlowski
2023-06-13 12:58 ` [PATCH v5 3/7] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 18:34   ` Krzysztof Kozlowski
2023-06-13 18:34     ` Krzysztof Kozlowski
2023-06-13 19:17     ` Conor Dooley
2023-06-13 19:17       ` Conor Dooley
2023-06-13 12:58 ` [PATCH v5 4/7] clk: starfive: Add StarFive JH7110 PLL clock driver Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 12:58 ` [PATCH v5 5/7] clk: starfive: jh7110-sys: Add PLL clocks source from DTS Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 12:58 ` [PATCH v5 6/7] riscv: dts: starfive: jh7110: Add syscon nodes Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 19:58   ` Conor Dooley
2023-06-13 19:58     ` Conor Dooley
2023-06-13 12:58 ` [PATCH v5 7/7] riscv: dts: starfive: jh7110: Add PLL clock source in SYSCRG node Xingyu Wu
2023-06-13 12:58   ` Xingyu Wu
2023-06-13 19:55   ` Conor Dooley
2023-06-13 19:55     ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230613-surrender-surcharge-8778867e58c1@spud \
    --to=conor@kernel.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=emil.renner.berthing@canonical.com \
    --cc=hal.feng@starfivetech.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=william.qiu@starfivetech.com \
    --cc=xingyu.wu@starfivetech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.