[PATCH dev-4.19 v3] Add Quanta GSJ board device tree v2.
Avi Fishman
avifishman70 at gmail.com
Tue Mar 5 03:03:08 AEDT 2019
On Mon, Mar 4, 2019 at 4:43 PM Fran Hsu (徐誌謙) <Fran.Hsu at quantatw.com> wrote:
>
> 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.
Joel, for gmac they don't use ncsi and indeed "use-ncsi" is not included.
Your next remark on emc0 is probably relevant.
> >
> > > + 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
> > >
--
Regards,
Avi
More information about the openbmc
mailing list