| Message ID | 20180212025523.5566-1-chen.kenyy@inventec.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [linux,dev-4.13,v4] ARM: dts: aspeed: Add ARM system BMC device tree | expand |
Hi Ken, This is looking much better. Just a few small things to tweak before I can merge this. On Mon, Feb 12, 2018 at 1:25 PM, Ken Chen <chen.kenyy@inventec.com> wrote: > Add centriq2400-rep dts and modify platform initial This commit message needs some work. Take a look at the commit messages from the introduction of other machines: > commit 8f9bafbb92c0308cf8d33536803c822e14bed4d7 > Author: Joel Stanley <joel@jms.id.au> > Date: Wed Jan 4 16:31:28 2017 > > ARM: dts: aspeed: Add Romulus BMC platform > > Romulus is an OpenPower machine with an ast2500 BMC. It has NCSI > networking and 512MB of RAM. or > commit f88bc8e15f1c1154495107ed378cce271309349d > Author: Rick Altherr <raltherr@google.com> > Date: Tue Nov 28 23:11:05 2017 > > 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. > > Signed-off-by: Ken Chen <chen.kenyy@inventec.com> > > --- > v3->v4 > - Modify Makefile for centriq2400-rep dts > - Fix typo label > - Removed unnecessary define of spi2 and centriq2400-spi-hwmon device > - Removed lpc device > - Fix coding style > - Removed ssif device > - Removed max31790 device > - Removed register control of GPIO and Watchdog Great changelog. This is very useful to me as a reviewer. > --- > arch/arm/boot/dts/Makefile | 1 + > .../boot/dts/aspeed-bmc-arm-centriq2400-rep.dts | 276 +++++++++++++++++++++ > arch/arm/mach-aspeed/aspeed.c | 11 + > 3 files changed, 288 insertions(+) > create mode 100644 arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 7c54fc8..77aef19 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -993,6 +993,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += aspeed-bmc-opp-palmetto.dtb \ > aspeed-bmc-mellanox-msn.dtb \ > aspeed-bmc-quanta-q71l.dtb \ > aspeed-bmc-intel-s2600wf.dtb \ > + aspeed-bmc-arm-centriq2400-rep.dtb \ > aspeed-ast2500-evb.dtb > endif > > diff --git a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts > new file mode 100644 > index 0000000..6c7784f > --- /dev/null > +++ b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts > @@ -0,0 +1,276 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/dts-v1/; > + > +#include "aspeed-g5.dtsi" > +#include <dt-bindings/gpio/aspeed-gpio.h> > + > +/ { > + model = "Qualcomm Centriq 2400 REP AST2520"; May I ask, what does REP mean? > + compatible = "qualcomm,centriq2400-rep-bmc", "aspeed,ast2500"; > + > + chosen { > + stdout-path = &uart5; > + bootargs = "console=ttyS4,115200 earlyprintk"; > + }; > + > + memory { > + reg = <0x80000000 0x40000000>; > + }; > + > + iio-hwmon { > + compatible = "iio-hwmon"; > + io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, > + <&adc 4>, <&adc 5>, <&adc 6>, <&adc 8>; > + }; > + > + iio-hwmon-battery { > + compatible = "iio-hwmon"; > + io-channels = <&adc 7>; > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + uid_led { > + label = "UID_LED"; > + gpios = <&gpio ASPEED_GPIO(Q, 5) GPIO_ACTIVE_LOW>; > + }; > + > + ras_error_led { > + label = "RAS_ERROR_LED"; > + gpios = <&gpio ASPEED_GPIO(F, 6) GPIO_ACTIVE_LOW>; > + }; > + > + system_fault { > + label = "System_fault"; > + gpios = <&gpio ASPEED_GPIO(A, 1) GPIO_ACTIVE_LOW>; > + }; > + }; > +}; > + > +&fmc { > + status = "okay"; > + flash@0 { > + status = "okay"; > + m25p,fast-read; > + label = "bmc"; Still some strange whitespace here. Make sure your editor is using tabs. > +#include "openbmc-flash-layout.dtsi" > + }; > +}; > + > +&spi1 { > + status = "okay"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1_default>; > + flash@0 { > + status = "okay"; > + }; > +}; > + > +&spi2 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi2ck_default &pinctrl_spi2miso_default &pinctrl_spi2mosi_default &pinctrl_spi2cs0_default>; Andrew, can you please review the pinctrl bits? > +}; > + > +&uart3 { > + status = "okay"; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_txd3_default &pinctrl_rxd3_default>; > + current-speed = <115200>; > +}; > + > diff --git a/arch/arm/mach-aspeed/aspeed.c b/arch/arm/mach-aspeed/aspeed.c > index 5965bbf..8fce9bc 100644 > --- a/arch/arm/mach-aspeed/aspeed.c > +++ b/arch/arm/mach-aspeed/aspeed.c We do not have an aspeed.c in the dev-4.13 branch. I do not plan on merging one, as at this stage all of the functionality that was present in aspeed.c can be performed in other drivers. I suggest submitting the device tree without any change to aspeed.c. To review what the changes are: > static void __init do_common_setup(void) > { > /* Enable LPC FWH cycles, Enable LPC to AHB bridge */ > writel(0x00000500, AST_IO(AST_BASE_LPC | 0x80)); This is now done in drivers/misc/aspeed-lpc-ctrl.c > /* Set UART routing */ > writel(0x00000000, AST_IO(AST_BASE_LPC | 0x9c)); This is the default, so we do not need have this line. > > /* Setup scratch registers */ > writel(0x00000042, AST_IO(AST_BASE_LPC | 0x170)); > writel(0x00008000, AST_IO(AST_BASE_LPC | 0x174)); These are specific to the operation of Hostboot on OpenPower platforms. I suspect you do not require these lines. Cheers, Joel > @@ -232,6 +232,15 @@ static void __init do_mellanox_setup(void) > writel(reg, AST_IO(AST_BASE_SCU | 0x48)); > } > > + > +static void __init do_centriq2400rep_setup(void) > +{ > + u32 reg; > + > + do_common_setup(); > +} > + > + > #define SCU_PASSWORD 0x1688A8A8 > > static void __init aspeed_init_early(void) > @@ -284,6 +293,8 @@ static void __init aspeed_init_early(void) > do_lanyang_setup(); > if (of_machine_is_compatible("mellanox,msn-bmc")) > do_mellanox_setup(); > + if (of_machine_is_compatible("qualcomm,centriq2400-rep-bmc")) > + do_centriq2400rep_setup(); > } > > static void __init aspeed_map_io(void) > -- > 2.9.3 >
Hi Joel, 2018-02-12 15:53 GMT+08:00 Joel Stanley <joel@jms.id.au>: > Hi Ken, > > This is looking much better. Just a few small things to tweak before I > can merge this. > > On Mon, Feb 12, 2018 at 1:25 PM, Ken Chen <chen.kenyy@inventec.com> wrote: >> Add centriq2400-rep dts and modify platform initial > > This commit message needs some work. > > Take a look at the commit messages from the introduction of other machines: > >> commit 8f9bafbb92c0308cf8d33536803c822e14bed4d7 >> Author: Joel Stanley <joel@jms.id.au> >> Date: Wed Jan 4 16:31:28 2017 >> >> ARM: dts: aspeed: Add Romulus BMC platform >> >> Romulus is an OpenPower machine with an ast2500 BMC. It has NCSI >> networking and 512MB of RAM. > > or > >> commit f88bc8e15f1c1154495107ed378cce271309349d >> Author: Rick Altherr <raltherr@google.com> >> Date: Tue Nov 28 23:11:05 2017 >> >> 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. > > >> >> Signed-off-by: Ken Chen <chen.kenyy@inventec.com> >> >> --- >> v3->v4 >> - Modify Makefile for centriq2400-rep dts >> - Fix typo label >> - Removed unnecessary define of spi2 and centriq2400-spi-hwmon device >> - Removed lpc device >> - Fix coding style >> - Removed ssif device >> - Removed max31790 device >> - Removed register control of GPIO and Watchdog > > Great changelog. This is very useful to me as a reviewer. > >> --- >> arch/arm/boot/dts/Makefile | 1 + >> .../boot/dts/aspeed-bmc-arm-centriq2400-rep.dts | 276 +++++++++++++++++++++ >> arch/arm/mach-aspeed/aspeed.c | 11 + >> 3 files changed, 288 insertions(+) >> create mode 100644 arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts >> >> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile >> index 7c54fc8..77aef19 100644 >> --- a/arch/arm/boot/dts/Makefile >> +++ b/arch/arm/boot/dts/Makefile >> @@ -993,6 +993,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += aspeed-bmc-opp-palmetto.dtb \ >> aspeed-bmc-mellanox-msn.dtb \ >> aspeed-bmc-quanta-q71l.dtb \ >> aspeed-bmc-intel-s2600wf.dtb \ >> + aspeed-bmc-arm-centriq2400-rep.dtb \ >> aspeed-ast2500-evb.dtb >> endif >> >> diff --git a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts >> new file mode 100644 >> index 0000000..6c7784f >> --- /dev/null >> +++ b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts >> @@ -0,0 +1,276 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/dts-v1/; >> + >> +#include "aspeed-g5.dtsi" >> +#include <dt-bindings/gpio/aspeed-gpio.h> >> + >> +/ { >> + model = "Qualcomm Centriq 2400 REP AST2520"; > > May I ask, what does REP mean? > REP is reference evaluation platform. >> + compatible = "qualcomm,centriq2400-rep-bmc", "aspeed,ast2500"; >> + >> + chosen { >> + stdout-path = &uart5; >> + bootargs = "console=ttyS4,115200 earlyprintk"; >> + }; >> + >> + memory { >> + reg = <0x80000000 0x40000000>; >> + }; >> + >> + iio-hwmon { >> + compatible = "iio-hwmon"; >> + io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, >> + <&adc 4>, <&adc 5>, <&adc 6>, <&adc 8>; >> + }; >> + >> + iio-hwmon-battery { >> + compatible = "iio-hwmon"; >> + io-channels = <&adc 7>; >> + }; >> + >> + leds { >> + compatible = "gpio-leds"; >> + >> + uid_led { >> + label = "UID_LED"; >> + gpios = <&gpio ASPEED_GPIO(Q, 5) GPIO_ACTIVE_LOW>; >> + }; >> + >> + ras_error_led { >> + label = "RAS_ERROR_LED"; >> + gpios = <&gpio ASPEED_GPIO(F, 6) GPIO_ACTIVE_LOW>; >> + }; >> + >> + system_fault { >> + label = "System_fault"; >> + gpios = <&gpio ASPEED_GPIO(A, 1) GPIO_ACTIVE_LOW>; >> + }; >> + }; >> +}; >> + >> +&fmc { >> + status = "okay"; >> + flash@0 { >> + status = "okay"; >> + m25p,fast-read; >> + label = "bmc"; > > Still some strange whitespace here. Make sure your editor is using tabs. > >> +#include "openbmc-flash-layout.dtsi" >> + }; >> +}; >> + >> +&spi1 { >> + status = "okay"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_spi1_default>; >> + flash@0 { >> + status = "okay"; >> + }; >> +}; >> + >> +&spi2 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_spi2ck_default &pinctrl_spi2miso_default &pinctrl_spi2mosi_default &pinctrl_spi2cs0_default>; > > Andrew, can you please review the pinctrl bits? > >> +}; >> + >> +&uart3 { >> + status = "okay"; >> + >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_txd3_default &pinctrl_rxd3_default>; >> + current-speed = <115200>; >> +}; >> + > >> diff --git a/arch/arm/mach-aspeed/aspeed.c b/arch/arm/mach-aspeed/aspeed.c >> index 5965bbf..8fce9bc 100644 >> --- a/arch/arm/mach-aspeed/aspeed.c >> +++ b/arch/arm/mach-aspeed/aspeed.c > > We do not have an aspeed.c in the dev-4.13 branch. > > I do not plan on merging one, as at this stage all of the > functionality that was present in aspeed.c can be performed in other > drivers. I suggest submitting the device tree without any change to > aspeed.c. > OK, I will remove this. > To review what the changes are: > >> static void __init do_common_setup(void) >> { >> /* Enable LPC FWH cycles, Enable LPC to AHB bridge */ >> writel(0x00000500, AST_IO(AST_BASE_LPC | 0x80)); > > This is now done in drivers/misc/aspeed-lpc-ctrl.c > >> /* Set UART routing */ >> writel(0x00000000, AST_IO(AST_BASE_LPC | 0x9c)); > > This is the default, so we do not need have this line. > >> >> /* Setup scratch registers */ >> writel(0x00000042, AST_IO(AST_BASE_LPC | 0x170)); >> writel(0x00008000, AST_IO(AST_BASE_LPC | 0x174)); > > These are specific to the operation of Hostboot on OpenPower > platforms. I suspect you do not require these lines. > > Cheers, > > Joel > > > >> @@ -232,6 +232,15 @@ static void __init do_mellanox_setup(void) >> writel(reg, AST_IO(AST_BASE_SCU | 0x48)); >> } >> >> + >> +static void __init do_centriq2400rep_setup(void) >> +{ >> + u32 reg; >> + >> + do_common_setup(); >> +} >> + >> + >> #define SCU_PASSWORD 0x1688A8A8 >> >> static void __init aspeed_init_early(void) >> @@ -284,6 +293,8 @@ static void __init aspeed_init_early(void) >> do_lanyang_setup(); >> if (of_machine_is_compatible("mellanox,msn-bmc")) >> do_mellanox_setup(); >> + if (of_machine_is_compatible("qualcomm,centriq2400-rep-bmc")) >> + do_centriq2400rep_setup(); >> } >> >> static void __init aspeed_map_io(void) >> -- >> 2.9.3 >>
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 7c54fc8..77aef19 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -993,6 +993,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += aspeed-bmc-opp-palmetto.dtb \ aspeed-bmc-mellanox-msn.dtb \ aspeed-bmc-quanta-q71l.dtb \ aspeed-bmc-intel-s2600wf.dtb \ + aspeed-bmc-arm-centriq2400-rep.dtb \ aspeed-ast2500-evb.dtb endif diff --git a/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts new file mode 100644 index 0000000..6c7784f --- /dev/null +++ b/arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts @@ -0,0 +1,276 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +#include "aspeed-g5.dtsi" +#include <dt-bindings/gpio/aspeed-gpio.h> + +/ { + model = "Qualcomm Centriq 2400 REP AST2520"; + compatible = "qualcomm,centriq2400-rep-bmc", "aspeed,ast2500"; + + chosen { + stdout-path = &uart5; + bootargs = "console=ttyS4,115200 earlyprintk"; + }; + + memory { + reg = <0x80000000 0x40000000>; + }; + + iio-hwmon { + compatible = "iio-hwmon"; + io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, + <&adc 4>, <&adc 5>, <&adc 6>, <&adc 8>; + }; + + iio-hwmon-battery { + compatible = "iio-hwmon"; + io-channels = <&adc 7>; + }; + + leds { + compatible = "gpio-leds"; + + uid_led { + label = "UID_LED"; + gpios = <&gpio ASPEED_GPIO(Q, 5) GPIO_ACTIVE_LOW>; + }; + + ras_error_led { + label = "RAS_ERROR_LED"; + gpios = <&gpio ASPEED_GPIO(F, 6) GPIO_ACTIVE_LOW>; + }; + + system_fault { + label = "System_fault"; + gpios = <&gpio ASPEED_GPIO(A, 1) GPIO_ACTIVE_LOW>; + }; + }; +}; + +&fmc { + status = "okay"; + flash@0 { + status = "okay"; + m25p,fast-read; + label = "bmc"; +#include "openbmc-flash-layout.dtsi" + }; +}; + +&spi1 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi1_default>; + flash@0 { + status = "okay"; + }; +}; + +&spi2 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi2ck_default &pinctrl_spi2miso_default &pinctrl_spi2mosi_default &pinctrl_spi2cs0_default>; +}; + +&uart3 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_txd3_default &pinctrl_rxd3_default>; + current-speed = <115200>; +}; + +&uart5 { + status = "okay"; +}; + +&mac0 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>; +}; + +&i2c0 { + status = "okay"; + + pca9542@70 { + compatible = "pca9542"; + reg = <0x70>; + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + pca9546@77 { + compatible = "pca9546"; + reg = <0x77>; + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + eeprom@52 { + compatible = "atmel,24c02"; + reg = <0x52>; + }; + }; + i2c@2 { + #address-cells = <1>; + #size-cells = <0>; + reg = <2>; + eeprom@57 { + compatible = "atmel,24c02"; + reg = <0x57>; + }; + }; + }; + }; + i2c@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + pca9546@77 { + compatible = "pca9546"; + reg = <0x77>; + i2c@2 { + #address-cells = <1>; + #size-cells = <0>; + reg = <2>; + eeprom@57 { + compatible = "atmel,24c02"; + reg = <0x57>; + }; + }; + }; + }; + + }; +}; + +&i2c1 { + status = "okay"; + + tmp421@1e { + compatible = "ti,tmp421"; + reg = <0x1e>; + }; + tmp421@2a { + compatible = "ti,tmp421"; + reg = <0x2a>; + }; + tmp421@4e { + compatible = "ti,tmp421"; + reg = <0x4e>; + }; + tmp421@1c { + compatible = "ti,tmp421"; + reg = <0x1c>; + }; +}; + +&i2c2 { + status = "okay"; +}; + +&i2c3 { + status = "okay"; +}; + +&i2c4 { + status = "okay"; +}; + +&i2c5 { + status = "okay"; +}; + +&i2c6 { + status = "okay"; + + tmp421@1d { + compatible = "ti,tmp421"; + reg = <0x1d>; + }; + tmp421@1f { + compatible = "ti,tmp421"; + reg = <0x1f>; + }; + tmp421@4d { + compatible = "ti,tmp421"; + reg = <0x4d>; + }; + tmp421@4f { + compatible = "ti,tmp421"; + reg = <0x4f>; + }; + nvt210@4c { + compatible = "nvt210"; + reg = <0x4c>; + }; + eeprom@50 { + compatible = "atmel,24c128"; + reg = <0x50>; + pagesize = <128>; + }; +}; + +&i2c7 { + status = "okay"; +}; + +&i2c8 { + status = "okay"; + + pca9641@70 { + compatible = "nxp,pca9641"; + reg = <0x70>; + #address-cells = <1>; + #size-cells = <0>; + i2c-arb { + tmp421@1d { + compatible = "tmp421"; + reg = <0x1d>; + }; + adm1278@12 { + compatible = "adi,adm1278"; + reg = <0x12>; + Rsense = <500>; + }; + eeprom@50 { + compatible = "atmel,24c02"; + reg = <0x50>; + }; + ds1100@58 { + compatible = "ds1100"; + reg = <0x58>; + }; + }; + }; +}; + +&i2c9 { + status = "okay"; +}; + +&vuart { + status = "okay"; +}; + +&gfx { + status = "okay"; +}; + +&pinctrl { + aspeed,external-nodes = <&gfx &lhc>; +}; +&wdt2 { + status = "okay"; +}; + +&gpio { + pin_gpio_c7 { + gpio-hog; + gpios = <ASPEED_GPIO(C, 7) GPIO_ACTIVE_HIGH>; + output; + line-name = "BIOS_SPI_MUX_S"; + }; +} diff --git a/arch/arm/mach-aspeed/aspeed.c b/arch/arm/mach-aspeed/aspeed.c index 5965bbf..8fce9bc 100644 --- a/arch/arm/mach-aspeed/aspeed.c +++ b/arch/arm/mach-aspeed/aspeed.c @@ -232,6 +232,15 @@ static void __init do_mellanox_setup(void) writel(reg, AST_IO(AST_BASE_SCU | 0x48)); } + +static void __init do_centriq2400rep_setup(void) +{ + u32 reg; + + do_common_setup(); +} + + #define SCU_PASSWORD 0x1688A8A8 static void __init aspeed_init_early(void) @@ -284,6 +293,8 @@ static void __init aspeed_init_early(void) do_lanyang_setup(); if (of_machine_is_compatible("mellanox,msn-bmc")) do_mellanox_setup(); + if (of_machine_is_compatible("qualcomm,centriq2400-rep-bmc")) + do_centriq2400rep_setup(); } static void __init aspeed_map_io(void)
Add centriq2400-rep dts and modify platform initial Signed-off-by: Ken Chen <chen.kenyy@inventec.com> --- v3->v4 - Modify Makefile for centriq2400-rep dts - Fix typo label - Removed unnecessary define of spi2 and centriq2400-spi-hwmon device - Removed lpc device - Fix coding style - Removed ssif device - Removed max31790 device - Removed register control of GPIO and Watchdog --- arch/arm/boot/dts/Makefile | 1 + .../boot/dts/aspeed-bmc-arm-centriq2400-rep.dts | 276 +++++++++++++++++++++ arch/arm/mach-aspeed/aspeed.c | 11 + 3 files changed, 288 insertions(+) create mode 100644 arch/arm/boot/dts/aspeed-bmc-arm-centriq2400-rep.dts