[PATCH dev-4.19 v3] Add Quanta GSJ board device tree v2.

Fran Hsu (徐誌謙) Fran.Hsu at quantatw.com
Tue Mar 5 01:43:11 AEDT 2019


Hi Joel,
    Thanks for your suggestion.
I will push the new patch for your review soon.
Regarding the part of nuvoton-npcm730.dtsi.
Please have a look at this link https://patchwork.ozlabs.org/patch/1045899/
This patch was pushed by Nuvoton Inc and wait for the review.
Thank you.
Fran
> -----Original Message-----
> From: Joel Stanley <joel at jms.id.au>
> Sent: Monday, March 4, 2019 9:10 AM
> To: Fran Hsu (徐誌謙) <Fran.Hsu at quantatw.com>
> Cc: Benjamin Fair <benjaminfair at google.com>; Emily Shaffer
> <emilyshaffer at google.com>; OpenBMC Maillist <openbmc at lists.ozlabs.org>
> Subject: Re: [PATCH dev-4.19 v3] Add Quanta GSJ board device tree v2.
> 
> Hello Fran,
> 
> Thanks for the patch.
> 
> On Thu, 28 Feb 2019 at 20:19, <fran.hsu at quantatw.com> wrote:
> >
> > From: FranHsu <Fran.Hsu at quantatw.com>
> >
> > Add device tree source include file nuvoton-npcm730-gsj-gpio.dtsi for setting
> pinctrl definition.
> 
> I suggest you use the commit message to describe the system you are adding.
> See some of the aspeed systems for an example:
> 
> https://git.kernel.org/torvalds/c/f88bc8e15f1
> 
>  > ARM: dts: aspeed: Add Qanta Q71L BMC machine  >  > The Qanta Q71L
> BMC is an ASPEED ast2400 based BMC that is part of a Qanta x86 server.
> 
> > Tested: This commit could build successfully with the Nuvoton kernel branch
> "Poleg-4.19.16-OpenBMC".
> >     And it could be booted on EVB.
> 
> As you're asking this to be applied to the dev-4.19 branch, you should be
> testing it on top of the latest dev-4.19 kernel tree. Similarly, if you were
> submitting it for mainline inclusion, you should test on top of the latest release
> of mainline.
> 
> I recommend submitting this patch for upstream inclusion, as we do for the
> aspeed device trees.
> 
> > Signed-off-by: FranHsu <Fran.Hsu at quantatw.com>
> > ---
> >  .../boot/dts/nuvoton-npcm730-gsj-gpio.dtsi    | 2452
> +++++++++++++++++
> >  arch/arm/boot/dts/nuvoton-npcm730-gsj.dts     |  550 ++++
> >  2 files changed, 3002 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
> >  create mode 100644 arch/arm/boot/dts/nuvoton-npcm730-gsj.dts
> >
> > diff --git a/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
> > b/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
> > new file mode 100644
> > index 000000000000..cf51cf3281cb
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nuvoton-npcm730-gsj-gpio.dtsi
> > @@ -0,0 +1,2452 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 Nuvoton Technology tomer.maimon at nuvoton.com //
> > +Update by Fran.Hsu at qunatatw.com
> 
> If your changes are copyrightable, then please add a copyright line with the
> copyright owner (your company?). You can chose to have your name there too,
> but this is not required as the git history will contain your name if someone
> needs this information in the future.
> 
> > diff --git a/arch/arm/boot/dts/nuvoton-npcm730-gsj.dts
> > b/arch/arm/boot/dts/nuvoton-npcm730-gsj.dts
> > new file mode 100644
> > index 000000000000..411387ee2f62
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/nuvoton-npcm730-gsj.dts
> > @@ -0,0 +1,550 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2019 Quanta Computer lnc. Fran.Hsu at quantatw.com
> 
> This copyright line looks good.
> 
> > +
> > +/dts-v1/;
> > +#include "nuvoton-npcm730.dtsi"
> 
> This file does not exist, so your patch breaks the kernel build. This is why it is
> important to test on the branch you intend the patch to apply to.
> 
> Please send a patch to add nuvoton-npcm730.dtsi.
> 
> > +#include "nuvoton-npcm733-gsj-gpio.dtsi"
> > +/ {
> > +       model = "Quanta GSJ Board (Device Tree v2))";
> > +       compatible = "nuvoton,npcm750";
> > +
> > +       aliases {
> > +               ethernet1 = &gmac0;
> > +               serial3 = &serial3;
> > +               udc9 = &udc9;
> > +               i2c0 = &i2c0;
> > +               i2c1 = &i2c1;
> > +               i2c2 = &i2c2;
> > +               i2c3 = &i2c3;
> > +               i2c4 = &i2c4;
> > +               i2c5 = &i2c5;
> > +               i2c6 = &i2c6;
> > +               i2c7 = &i2c7;
> > +               i2c8 = &i2c8;
> > +               i2c9 = &i2c9;
> > +               i2c10 = &i2c10;
> > +               i2c11 = &i2c11;
> > +               i2c12 = &i2c12;
> > +               i2c13 = &i2c13;
> > +               i2c14 = &i2c14;
> > +               i2c15 = &i2c15;
> > +               spi0 = &spi0;
> > +               spi1 = &spi1;
> > +               fiu0 = &fiu0;
> > +               fiu1 = &fiu3;
> > +               fiu2 = &fiux;
> > +       };
> 
> Do you need all of these aliases? Most of the numbering (eg. i2c) should come
> out the same without these.
> 
> > +
> > +       chosen {
> > +               stdout-path = &serial3;
> > +       };
> > +
> > +       memory {
> > +               reg = <0 0x40000000>;
> > +       };
> > +
> > +       ahb {
> > +               gmac0: eth at f0802000 {
> > +                       reg = <0xf0802000 0x2000>;
> 
> I haven't seen the npcm730 dtsi, but I assume it is similar to the
> npcm750 which already contains the full ethernet node. If this is the case, you
> only need to do this:
> 
> &gmac0 {
>     phy-mode = "rgmii-id";
>     status = "okay";
> };
> 
> > +                       phy-mode = "rgmii-id";
> 
> I don't think the phy-mode property make sense if it's using NCSI.
> 
> > +                       status = "okay";
> > +               };
> > +               emc0: eth at f0825000 {
> > +                       phy-mode = "rmii";
> 
> I don't think the phy-mode makes sense if it's using NCSI.
> 
> We should change the NCSI layer to support phy-mode = "ncsi".
> 
> > +                       use-ncsi; /* add this to support ncsi */
> 
> You can remove this comment.
> 
> > +                       status = "okay";
> > +               };
> > +               ehci1: usb at f0806000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               ohci1: ohci at f0807000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               udc9:udc at f0839000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               aes:aes at f0858000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               sha:sha at f085a000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               fiu0: fiu at fb000000 {
> > +                       pinctrl-names = "default";
> > +                       pinctrl-0 = <&spi0cs1_pins>;
> > +                       status = "okay";
> > +                       spi-nor at 0 {
> > +                               compatible = "jedec,spi-nor";
> > +                               #address-cells = <1>;
> > +                               #size-cells = <1>;
> > +                               reg = <0>;
> > +                               spi-rx-bus-width = <2>;
> > +                               partitions at 80000000 {
> > +                                               compatible =
> "fixed-partitions";
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <1>;
> > +                                               bmc at 0{
> > +                                                       label =
> "bmc";
> > +                                                       reg =
> <0x000000 0x4000000>;
> > +                                               };
> > +                                               u-boot at 0 {
> > +                                                       label =
> "u-boot";
> > +                                                       reg =
> <0x0000000 0x80000>;
> > +                                               };
> > +
> envparam at 100000 {
> > +                                                       label =
> "env-param";
> > +                                                       reg =
> <0x0100000 0x40000>;
> > +                                               };
> > +                                               kernel at 200000 {
> > +                                                       label =
> "kernel";
> > +                                                       reg =
> <0x0200000 0x580000>;
> > +                                               };
> > +                                               rofs at C00000 {
> > +                                                       label =
> "rofs";
> > +                                                       reg =
> <0x0C00000 0x2600000>;
> > +                                               };
> > +                                               rwfs at 3200000 {
> > +                                                       label =
> "rwfs";
> > +                                                       reg =
> <0x3200000 0xE00000>;
> > +                                               };
> > +                               };
> > +                       };
> > +               };
> > +
> > +               fiu3: fiu at c0000000 {
> > +                       pinctrl-0 = <&spi3_pins>;
> > +                       status = "disabled";
> 
> It is unnecessary to add the node and then set it to disabled. You can simply
> omit these nodes.
> 
> Reading the rest of this device tree, it looks like a number of the contents
> should be in the SoC definition (npcm730.dsti) so it can be shared among
> systems that use the npcm730.
> 
> Cheers,
> 
> Joel
> 
> > +               };
> > +
> > +               fiux: fiu at fb001000 {
> > +                       status = "disabled";
> > +                       spix-mode;
> > +               };
> > +
> > +               sdhci0: sdhci at f0842000 {
> > +                       status = "disabled";
> > +               };
> > +
> > +               pcimbox: pcimbox at f0848000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               vcd: vcd at f0810000 {
> > +                       status = "disabled";
> > +               };
> > +
> > +               ece: ece at f0820000 {
> > +                       status = "okay";
> > +               };
> > +
> > +               apb {
> > +
> > +                       watchdog1: watchdog at 901C {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       rng: rng at b000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       serial0: serial at 1000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       serial1: serial at 2000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       serial2: serial at 3000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       serial3: serial at 4000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       adc: adc at c000 {
> > +                               status = "okay";
> > +                       };
> > +                       otp:otp at 189000 {
> > +                               status = "okay";
> > +                       };
> > +
> > +                       i2c0: i2c at 80000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "disabled";
> > +                       };
> > +
> > +                       i2c1: i2c at 81000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               lm75 at 5c {
> > +                                       compatible =
> "maxim,max31725";
> > +                                       reg = <0x5c>;
> > +                                       status = "okay";
> > +                               };
> > +                       };
> > +
> > +                       i2c2: i2c at 82000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               lm75 at 5c {
> > +                                       compatible =
> "maxim,max31725";
> > +                                       reg = <0x5c>;
> > +                                       status = "okay";
> > +                               };
> > +                       };
> > +
> > +                       i2c3: i2c at 83000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               lm75 at 5c {
> > +                                       compatible =
> "maxim,max31725";
> > +                                       reg = <0x5c>;
> > +                                       status = "okay";
> > +                               };
> > +                       };
> > +
> > +                       i2c4: i2c at 84000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               lm75 at 5c {
> > +                                       compatible =
> "maxim,max31725";
> > +                                       reg = <0x5c>;
> > +                                       status = "okay";
> > +                               };
> > +                       };
> > +
> > +                       i2c5: i2c at 85000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "disabled";
> > +                       };
> > +
> > +                       i2c6: i2c at 86000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "disabled";
> > +                       };
> > +
> > +                       i2c7: i2c at 87000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "disabled";
> > +                       };
> > +
> > +                       i2c8: i2c at 88000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                       };
> > +
> > +                       i2c9: i2c at 89000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               eeprom at 55 {
> > +                                       compatible =
> "atmel,24c64";
> > +                                       reg = <0x55>;
> > +                               };
> > +                       };
> > +
> > +                       i2c10: i2c at 8a000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                               eeprom at 55 {
> > +                                       compatible =
> "atmel,24c64";
> > +                                       reg = <0x55>;
> > +                               };
> > +                       };
> > +
> > +                       i2c11: i2c at 8b000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +
> > +                               /* P12V Quarter Brick DC/DC Power
> Module Q54SH12050 @60 */
> > +                               power-brick at 36 {
> > +                                       compatible =
> "delta,dps800";
> > +                                       reg = <0x36>;
> > +                               };
> > +                               hotswap at 15 {
> > +                                       compatible = "ti,lm5066i";
> > +                                       reg = <0x15>;
> > +                               };
> > +                       };
> > +
> > +                       i2c12: i2c at 8c000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                       };
> > +
> > +                       i2c13: i2c at 8d000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                       };
> > +
> > +                       i2c14: i2c at 8e000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +                       };
> > +
> > +                       i2c15: i2c at 8f000 {
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               bus-frequency = <100000>;
> > +                               status = "okay";
> > +
> > +                               i2c-switch at 75 {
> > +                                       compatible =
> "nxp,pca9548";
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
> > +                                       reg = <0x75>;
> > +                                       i2c-mux-idle-disconnect;
> > +
> > +                                       i2c_u20: i2c-bus at 0 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <0>;
> > +                                       };
> > +
> > +                                       i2c_u21: i2c-bus at 1 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <1>;
> > +                                       };
> > +
> > +                                       i2c_u22: i2c-bus at 2 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <2>;
> > +                                       };
> > +
> > +                                       i2c_u23: i2c-bus at 3 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <3>;
> > +                                       };
> > +
> > +                                       i2c_u24: i2c-bus at 4 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <4>;
> > +                                       };
> > +
> > +                                       i2c_u25: i2c-bus at 5 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <5>;
> > +                                       };
> > +
> > +                                       i2c_u26: i2c-bus at 6 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <6>;
> > +                                       };
> > +
> > +                                       i2c_u27: i2c-bus at 7 {
> > +                                               #address-cells =
> <1>;
> > +                                               #size-cells = <0>;
> > +                                               reg = <7>;
> > +                                       };
> > +                               };
> > +                       };
> > +
> > +                       pwm_fan:pwm-fan-controller at 103000 {
> > +                               pinctrl-names = "default";
> > +                               pinctrl-0 = <   &pwm0_pins
> &pwm1_pins &pwm2_pins
> > +                                               &fanin0_pins
> &fanin1_pins
> > +                                               &fanin2_pins
> &fanin3_pins
> > +                                               &fanin4_pins
> &fanin5_pins>;
> > +                               status = "okay";
> > +                               fan at 0 {
> > +                                       reg = <0x00>;
> > +                                       fan-tach-ch = /bits/ 8 <0x00
> 0x01>;
> > +                                       cooling-levels = <127 255>;
> > +                               };
> > +                               fan at 1 {
> > +                                       reg = <0x01>;
> > +                                       fan-tach-ch = /bits/ 8 <0x02
> 0x03>;
> > +                                       cooling-levels = /bits/ 8
> <127 255>;
> > +                               };
> > +                               fan at 2 {
> > +                                       reg = <0x02>;
> > +                                       fan-tach-ch = /bits/ 8 <0x04
> 0x05>;
> > +                                       cooling-levels = /bits/ 8
> <127 255>;
> > +                               };
> > +                               fan at 3 {
> > +                                       reg = <0x03>;
> > +                                       fan-tach-ch = /bits/ 8 <0x06
> 0x07>;
> > +                                       cooling-levels = /bits/ 8
> <127 255>;
> > +                               };
> > +                               fan at 4 {
> > +                                       reg = <0x04>;
> > +                                       fan-tach-ch = /bits/ 8 <0x08
> 0x09>;
> > +                                       cooling-levels = /bits/ 8
> <127 255>;
> > +                               };
> > +                               fan at 5 {
> > +                                       reg = <0x05>;
> > +                                       fan-tach-ch = /bits/ 8 <0x0A
> 0x0B>;
> > +                                       cooling-levels = /bits/ 8
> <127 255>;
> > +                               };
> > +                       };
> > +
> > +                       spi0: spi at 200000 {
> > +                               status = "disabled";
> > +                       };
> > +
> > +                       spi1: spi at 201000 {
> > +                               status = "disabled";
> > +                       };
> > +
> > +               };
> > +       };
> > +
> > +       pinctrl: pinctrl at f0800000 {
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <
> > +                               /* GPI pins*/
> > +                               &gpio8_pins
> > +                               &gpio9_pins
> > +                               &gpio12_pins
> > +                               &gpio13_pins
> > +                               &gpio14_pins
> > +                               &gpio60_pins
> > +                               &gpio83_pins
> > +                               &gpio91_pins
> > +                               &gpio92_pins
> > +                               &gpio95_pins
> > +                               &gpio136_pins
> > +                               &gpio137_pins
> > +                               &gpio141_pins
> > +                               &gpio144_pins
> > +                               &gpio145_pins
> > +                               &gpio146_pins
> > +                               &gpio147_pins
> > +                               &gpio148_pins
> > +                               &gpio149_pins
> > +                               &gpio150_pins
> > +                               &gpio151_pins
> > +                               &gpio152_pins
> > +                               &gpio153_pins
> > +                               &gpio154_pins
> > +                               &gpio155_pins
> > +                               &gpio156_pins
> > +                               &gpio157_pins
> > +                               &gpio158_pins
> > +                               &gpio159_pins
> > +                               &gpio161_pins
> > +                               &gpio162_pins
> > +                               &gpio163_pins
> > +                               &gpio164_pins
> > +                               &gpio165_pins
> > +                               &gpio166_pins
> > +                               &gpio167_pins
> > +                               &gpio168_pins
> > +                               &gpio169_pins
> > +                               &gpio170_pins
> > +                               &gpio177_pins
> > +                               &gpio191_pins
> > +                               &gpio192_pins
> > +                               &gpio203_pins
> > +                               /* GPO pins*/
> > +                               &gpio0od_pins
> > +                               &gpio1od_pins
> > +                               &gpio2od_pins
> > +                               &gpio3od_pins
> > +                               &gpio4od_pins
> > +                               &gpio5od_pins
> > +                               &gpio6od_pins
> > +                               &gpio7od_pins
> > +                               &gpio10od_pins
> > +                               &gpio11od_pins
> > +                               &gpio15od_pins
> > +                               &gpio17od_pins
> > +                               &gpio18od_pins
> > +                               &gpio19od_pins
> > +                               &gpio24od_pins
> > +                               &gpio25od_pins
> > +                               &gpio37od_pins
> > +                               &gpio59od_pins
> > +                               &gpio72od_pins
> > +                               &gpio73od_pins
> > +                               &gpio74od_pins
> > +                               &gpio75od_pins
> > +                               &gpio76od_pins
> > +                               &gpio77od_pins
> > +                               &gpio78od_pins
> > +                               &gpio79od_pins
> > +                               &gpio84od_pins
> > +                               &gpio85od_pins
> > +                               &gpio86od_pins
> > +                               &gpio87od_pins
> > +                               &gpio88od_pins
> > +                               &gpio89od_pins
> > +                               &gpio90od_pins
> > +                               &gpio93od_pins
> > +                               &gpio94od_pins
> > +                               &gpio125od_pins
> > +                               &gpio126od_pins
> > +                               &gpio127od_pins
> > +                               &gpio142od_pins
> > +                               &gpio143ol_pins
> > +                               &gpio175od_pins
> > +                               &gpio176od_pins
> > +                               &gpio190od_pins
> > +                               &gpio194od_pins
> > +                               &gpio195od_pins
> > +                               &gpio196od_pins
> > +                               &gpio197od_pins
> > +                               &gpio198od_pins
> > +                               &gpio199od_pins
> > +                               &gpio200od_pins
> > +                               &gpio202od_pins
> > +                               >;
> > +       };
> > +
> > +
> > +
> > +       leds {
> > +               compatible = "gpio-leds";
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&gpio143ol_pins>;
> > +
> > +               led {
> > +                       label = "led-bmc-live";
> > +                       gpios = <&gpio4 15 GPIO_ACTIVE_HIGH>;
> > +                       linux,default-trigger = "heartbeat";
> > +               };
> > +       };
> > +};
> > +
> > --
> > 2.21.0
> >


More information about the openbmc mailing list