linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Conor Dooley <conor@kernel.org>,
	Hal Feng <hal.feng@linux.starfivetech.com>
Cc: linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Marc Zyngier <maz@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Emil Renner Berthing <kernel@esmil.dk>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 27/30] RISC-V: Add initial StarFive JH7110 device tree
Date: Mon, 3 Oct 2022 09:45:57 +0200	[thread overview]
Message-ID: <7b253e51-30d4-7a71-2b14-7b987c3045fc@linaro.org> (raw)
In-Reply-To: <Yzgb0GzpLsV3RJyk@spud>

On 01/10/2022 12:52, Conor Dooley wrote:
> On Fri, Sep 30, 2022 at 03:49:14PM +0800, Hal Feng wrote:
>> From: Emil Renner Berthing <kernel@esmil.dk>
>>
>> Add initial device tree for the JH7110 RISC-V SoC by
>> StarFive Technology Ltd.
>>
>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
>> Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com>
>> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com>
> 
> There's little point reviewing this dt since there's a load of issues
> that you can trivially find by running dtbs_check/dt_binding_check, but

Yep...

> this SoB change is wrong - if Emil wrote the patch, then Jianlong's SoB
> is either redundant or should be accompanied by a Co-developed-by tag.

Depends. Jianlong might have just rebased the patch.

> 
> Ditto for patch 28/30 "RISC-V: Add StarFive JH7110 VisionFive2 board
> device tree".
> 
>> ---
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++++++++++++++++
>>  1 file changed, 449 insertions(+)
>>  create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> new file mode 100644
>> index 000000000000..46f418d4198a
>> --- /dev/null
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> 
>> +
>> +	osc: osc {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +	};
>> +
>> +	clk_rtc: clk_rtc {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +	};
>> +
>> +	gmac0_rmii_refin: gmac0_rmii_refin {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <50000000>;
> 
> I assume, given osc has it's frequency set in the board dts, that these
> are all oscillators on the SoC?
> 
>> +	};
>> +
>> +	gmac0_rgmii_rxin: gmac0_rgmii_rxin {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <125000000>;
>> +	};
>> +
>> +	gmac1_rmii_refin: gmac1_rmii_refin {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <50000000>;
>> +	};
>> +
>> +	gmac1_rgmii_rxin: gmac1_rgmii_rxin {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <125000000>;
>> +	};
>> +
>> +	i2stx_bclk_ext: i2stx_bclk_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <12288000>;
>> +	};
>> +
>> +	i2stx_lrck_ext: i2stx_lrck_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <192000>;
>> +	};
>> +
>> +	i2srx_bclk_ext: i2srx_bclk_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <12288000>;
>> +	};
>> +
>> +	i2srx_lrck_ext: i2srx_lrck_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <192000>;
>> +	};
>> +
>> +	tdm_ext: tdm_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <49152000>;
>> +	};
>> +
>> +	mclk_ext: mclk_ext {
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <49152000>;
>> +	};
> 
>> +		syscrg: syscrg@13020000 {
> 
> The generic node name for syscons is just "syscon" afaik.

Yes.

> 
>> +			compatible = "syscon", "simple-mfd";

And this is not allowed. Needs specific compatible.


>> +			reg = <0x0 0x13020000 0x0 0x10000>;
>> +
> 
>> +		aoncrg: aoncrg@17000000 {
> 
> Again, syscon as the node name?

Yes.

> 
>> +			compatible = "syscon", "simple-mfd";

And this is a NAK.

>> +			reg = <0x0 0x17000000 0x0 0x10000>;
>> +
>> +		gpio: gpio@13040000 {
> 
> Someone else (Krzysztof maybe?) should comment, but is "pinctrl" not the
> genric node name for pinctrl nodes?

Yes, for pin controller nodes, this should be "pinctrl" and schema
requires it. The problem was that his driver did not use generic pinctrl
bindings, which is no-go on its own.

This could be a gpio controller (so "gpio" would be fine), although
compatible suggests otherwise.


Best regards,
Krzysztof


  reply	other threads:[~2022-10-03  8:22 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 14:31 [PATCH v1 00/30] Basic StarFive JH7110 RISC-V SoC support Hal Feng
2022-09-29 14:31 ` [PATCH v1 01/30] dt-bindings: riscv: Add StarFive JH7110 bindings Hal Feng
2022-09-29 14:34   ` Krzysztof Kozlowski
2022-10-08  3:44     ` Hal Feng
2022-09-29 14:31 ` [PATCH v1 02/30] dt-bindings: timer: Add StarFive JH7110 clint Hal Feng
2022-09-29 14:34   ` Krzysztof Kozlowski
2022-09-29 14:31 ` [PATCH v1 03/30] dt-bindings: interrupt-controller: Add StarFive JH7110 plic Hal Feng
2022-09-29 14:35   ` Krzysztof Kozlowski
2022-09-29 14:31 ` [PATCH v1 04/30] dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs Hal Feng
2022-09-29 14:36   ` Krzysztof Kozlowski
2022-09-29 15:33   ` Conor Dooley
2022-10-03  9:26     ` Ben Dooks
2022-10-08 18:54       ` Hal Feng
2022-09-29 14:32 ` [PATCH v1 05/30] soc: sifive: l2 cache: Convert to platform driver Hal Feng
2022-09-29 15:32   ` Conor Dooley
2022-09-29 17:57   ` Ben Dooks
2022-10-05 13:44     ` Emil Renner Berthing
2022-10-05 13:48       ` Ben Dooks
2022-10-05 13:55         ` Emil Renner Berthing
2022-10-05 14:05           ` Conor Dooley
2022-10-08 18:07             ` Hal Feng
2022-09-29 14:32 ` [PATCH v1 06/30] soc: sifive: l2 cache: Add StarFive JH71x0 support Hal Feng
2022-09-29 14:32 ` [PATCH v1 07/30] reset: starfive: jh7100: Use 32bit I/O on 32bit registers Hal Feng
2022-09-29 14:32 ` [PATCH v1 08/30] reset: starfive: jh7100: Use regmap APIs to operate registers Hal Feng
2022-09-29 14:32 ` [PATCH v1 09/30] reset: starfive: jh7100: Move necessary properties to device tree Hal Feng
2022-09-30 20:49   ` Rob Herring
2022-10-05 13:20     ` Emil Renner Berthing
2022-09-29 14:32 ` [PATCH v1 10/30] reset: starfive: Rename 'reset-starfive-jh7100.c' to 'reset-starfive.c' Hal Feng
2022-09-29 14:45 ` [PATCH v1 00/30] Basic StarFive JH7110 RISC-V SoC support Krzysztof Kozlowski
2022-09-29 17:59   ` Conor Dooley
2022-10-01  1:13     ` hal.feng
2022-09-29 16:35 ` [PATCH v1 11/30] dt-bindings: reset: Add StarFive JH7110 reset definitions Hal Feng
2022-09-29 17:51 ` [PATCH v1 12/30] dt-bindings: reset: Add starfive,jh7110-reset bindings Hal Feng
2022-09-29 18:21   ` Rob Herring
2022-09-29 18:40     ` Rob Herring
2022-09-29 18:43   ` Rob Herring
2022-10-11 15:30     ` Hal Feng
2022-10-11 16:36       ` Krzysztof Kozlowski
2022-10-12 13:16         ` Hal Feng
2022-10-12 13:33           ` Krzysztof Kozlowski
2022-10-12 14:05             ` Conor Dooley
2022-10-12 15:21               ` Hal Feng
2022-10-12 14:53             ` Hal Feng
2022-10-12  8:01       ` Emil Renner Berthing
2022-09-29 17:53 ` [PATCH v1 13/30] reset: starfive: Add StarFive JH7110 SoC support Hal Feng
2022-09-29 17:54 ` [PATCH v1 14/30] clk: starfive: Factor out common clock driver code Hal Feng
2022-09-30 21:43   ` Stephen Boyd
2022-09-29 17:56 ` [PATCH v1 15/30] clk: starfive: Use regmap APIs to operate registers Hal Feng
2022-09-30 21:48   ` Stephen Boyd
2022-10-05 13:14     ` Emil Renner Berthing
2022-10-12 23:05       ` Stephen Boyd
2022-10-23  4:11         ` Hal Feng
2022-10-23 10:25           ` Conor Dooley
2022-10-28  3:16             ` Hal Feng
2022-10-27  1:26           ` Stephen Boyd
2022-10-28  2:46             ` Hal Feng
2022-09-29 17:56 ` [PATCH v1 16/30] dt-bindings: clock: Add StarFive JH7110 system clock definitions Hal Feng
2022-09-29 22:26 ` [PATCH v1 17/30] dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings Hal Feng
2022-09-30  1:55   ` Rob Herring
2022-09-30 10:58   ` Krzysztof Kozlowski
2022-10-11 17:52     ` Hal Feng
2022-09-30  1:50 ` [PATCH v1 18/30] clk: starfive: Add StarFive JH7110 system clock driver Hal Feng
2022-09-30  5:49 ` [PATCH v1 19/30] dt-bindings: clock: Add StarFive JH7110 always-on definitions Hal Feng
2022-09-30  5:56 ` [PATCH v1 20/30] dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings Hal Feng
2022-09-30 10:59   ` Krzysztof Kozlowski
2022-10-11 18:01     ` Hal Feng
2022-09-30 12:51   ` Rob Herring
2022-09-30  6:03 ` [PATCH v1 21/30] clk: starfive: Add StarFive JH7110 always-on clock driver Hal Feng
2022-09-30  6:08 ` [PATCH v1 22/30] pinctrl: Create subdirectory for StarFive drivers Hal Feng
2022-10-04  8:43   ` Linus Walleij
2022-09-30  6:14 ` [PATCH v1 23/30] pinctrl: starfive: Rename "pinctrl-starfive" to "pinctrl-starfive-jh7100" Hal Feng
2022-09-30 21:28   ` Rob Herring
2022-10-04  8:48     ` Linus Walleij
2022-10-04  8:58       ` Conor Dooley
2022-10-04  9:13         ` Linus Walleij
2022-10-04  9:21           ` Conor Dooley
2022-10-04  9:24             ` Conor Dooley
2022-10-06  9:07       ` Geert Uytterhoeven
2022-09-30  7:33 ` [PATCH v1 24/30] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions Hal Feng
2022-09-30 11:00   ` Krzysztof Kozlowski
2022-09-30  7:38 ` [PATCH v1 25/30] dt-bindings: pinctrl: Add StarFive JH7110 pinctrl bindings Hal Feng
2022-09-30 11:05   ` Krzysztof Kozlowski
2022-09-30 12:16   ` Rob Herring
2022-10-20  7:28   ` Icenowy Zheng
2022-09-30  7:43 ` [PATCH v1 26/30] pinctrl: starfive: Add StarFive JH7110 driver Hal Feng
2022-10-01 14:35   ` kernel test robot
2022-10-04  8:56   ` Linus Walleij
2022-10-05 13:31     ` Emil Renner Berthing
2022-10-14  2:05       ` Hal Feng
2022-09-30  7:49 ` [PATCH v1 27/30] RISC-V: Add initial StarFive JH7110 device tree Hal Feng
2022-10-01 10:52   ` Conor Dooley
2022-10-03  7:45     ` Krzysztof Kozlowski [this message]
2022-10-14  9:41     ` Hal Feng
2022-09-30  7:53 ` [PATCH v1 28/30] RISC-V: Add StarFive JH7110 VisionFive2 board " Hal Feng
2022-10-01 11:14   ` Conor Dooley
2022-10-29  8:18     ` Hal Feng
2022-09-30  9:06 ` [PATCH v1 29/30] RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW Hal Feng
2022-09-30 20:54   ` Ben Dooks
2022-09-30 21:41     ` Conor Dooley
2022-10-14  3:24       ` Hal Feng
2022-09-30 12:23 ` [PATCH v1 30/30] RISC-V: Add StarFive JH7100 and JH7110 SoC Kconfig options Hal Feng
2022-09-30 12:37   ` Conor Dooley
2022-10-11 18:32     ` Hal Feng
2022-10-05 13:05 ` [PATCH v1 00/30] Basic StarFive JH7110 RISC-V SoC support Emil Renner Berthing
2022-10-08  3:18   ` Hal Feng

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=7b253e51-30d4-7a71-2b14-7b987c3045fc@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=conor@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=hal.feng@linux.starfivetech.com \
    --cc=kernel@esmil.dk \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=maz@kernel.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=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).