| Message ID | 20220707132054.GA10610@logan-ThinkPad-T14-Gen-1 |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] The Yosemite V3.5 is a facebook multi-node server platform that host four OCP server. The BMC in the Yosemite V3.5 platform based on AST2600 SoC. | expand |
On Thu, Jul 7, 2022 at 3:20 PM Logananth Sundararaj <logananth13.hcl@gmail.com> wrote: > > This patch adds linux device tree entry related to > Yosemite V3.5 specific devices connected to BMC SoC. > > Signed-off-by: Logananth Sundararaj <logananth_s@hcl.com> Something went wrong with the patch description, it looks like you dropped the subject and sent the first paragraph as the subject instead. > +/ { > + model = "Facebook fby35"; > + compatible = "facebook,fby35", "aspeed,ast2600"; > + > + aliases { > + serial4 = &uart5; > + }; Why not start at serial0 here? > + chosen { > + stdout-path = &uart5; > + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; > + }; The bootargs should really come from the boot loader. Why do you need the vmalloc=384M? That seems excessive. > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x80000000 0x80000000>; > + }; The memory location and size is usually also set by the boot loader Arnd
On Thu, Jul 07, 2022 at 03:33:48PM +0200, Arnd Bergmann wrote: > On Thu, Jul 7, 2022 at 3:20 PM Logananth Sundararaj > <logananth13.hcl@gmail.com> wrote: > > > > This patch adds linux device tree entry related to > > Yosemite V3.5 specific devices connected to BMC SoC. > > > > Signed-off-by: Logananth Sundararaj <logananth_s@hcl.com> > > > Something went wrong with the patch description, it looks like you dropped > the subject and sent the first paragraph as the subject instead. > > > +/ { > > + model = "Facebook fby35"; > > + compatible = "facebook,fby35", "aspeed,ast2600"; > > + > > + aliases { > > + serial4 = &uart5; > > + }; > > Why not start at serial0 here? Hey, Facebook person jumping in here (using a personal email): I think you're right, it should be like this: aliases { serial0 = &uart5; serial1 = &uart1; serial2 = &uart2; serial3 = &uart3; serial4 = &uart4; }; > > > + chosen { > > + stdout-path = &uart5; > > + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; > > + }; Also: if we do serial0 = &uart5, it should be console=ttyS0, not ttyS4. > > The bootargs should really come from the boot loader. What if we want to boot the kernel by itself with QEMU? It's kinda annoying to have to specify '-append "console=ttyS0,57600n8...' everytime, or to have to use a wrapper script. But, it's also a source of bugs: I realized yesterday the dts we were using here: https://github.com/facebook/openbmc-linux/blob/e26c76992e0761d9e440ff514538009384c094b4/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts Has the wrong console setting. Booting the kernel directly is actualy really useful for us, because U-Boot literally takes 4+ minutes in QEMU because we execute in-place from flash for most of it. > > Why do you need the vmalloc=384M? That seems excessive. Yeah I'm not sure what that is about, would need to find the person who wrote this originally. Speaking of which: The dts I linked above, from our repo, how come this patch is not just a copy of that? > > > + > > + memory@80000000 { > > + device_type = "memory"; > > + reg = <0x80000000 0x80000000>; > > + }; > > The memory location and size is usually also set by the boot loader Yeah not sure what happens if I remove this and boot the kernel directly in QEMU, would we need to specify the RAM size explicitly in console args? Hmmm, I'll check it out. Thanks for your comments! Peter > > Arnd
On Thu, Jul 14, 2022 at 8:05 PM Peter Delevoryas <peter@pjd.dev> wrote: > On Thu, Jul 07, 2022 at 03:33:48PM +0200, Arnd Bergmann wrote: > > > + model = "Facebook fby35"; > > > + compatible = "facebook,fby35", "aspeed,ast2600"; > > > + > > > + aliases { > > > + serial4 = &uart5; > > > + }; > > > > Why not start at serial0 here? > > Hey, Facebook person jumping in here (using a personal email): > > I think you're right, it should be like this: > > aliases { > serial0 = &uart5; > serial1 = &uart1; > serial2 = &uart2; > serial3 = &uart3; > serial4 = &uart4; > }; Are you actually using all five uarts though? > > > + chosen { > > > + stdout-path = &uart5; > > > + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; > > > + }; > > Also: if we do serial0 = &uart5, it should be console=ttyS0, not ttyS4. > > > > > The bootargs should really come from the boot loader. > > What if we want to boot the kernel by itself with QEMU? It's kinda annoying to > have to specify '-append "console=ttyS0,57600n8...' everytime, or to have to use > a wrapper script. But, it's also a source of bugs: I realized yesterday the > dts we were using here: > > https://github.com/facebook/openbmc-linux/blob/e26c76992e0761d9e440ff514538009384c094b4/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts > > Has the wrong console setting. You can encode the uart settings like stdout-path = "serial0:115200n8" the rest really should be passed on the command line, not in the DT shipped with the kernel. Arnd
On Thu, Jul 14, 2022 at 10:15:15PM +0200, Arnd Bergmann wrote: > On Thu, Jul 14, 2022 at 8:05 PM Peter Delevoryas <peter@pjd.dev> wrote: > > On Thu, Jul 07, 2022 at 03:33:48PM +0200, Arnd Bergmann wrote: > > > > + model = "Facebook fby35"; > > > > + compatible = "facebook,fby35", "aspeed,ast2600"; > > > > + > > > > + aliases { > > > > + serial4 = &uart5; > > > > + }; > > > > > > Why not start at serial0 here? > > > > Hey, Facebook person jumping in here (using a personal email): > > > > I think you're right, it should be like this: > > > > aliases { > > serial0 = &uart5; > > serial1 = &uart1; > > serial2 = &uart2; > > serial3 = &uart3; > > serial4 = &uart4; > > }; > > Are you actually using all five uarts though? Actually yes, I should have mentioned this in my previous message. YosemiteV3.5 is similar to YosemiteV3, which you can see here: https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server This dts is for the BMC on the sled baseboard, and it manages the 4 slots in the sled. Each slot has a "Bridge Interconnect" (BIC) (an AST1030) that manages the slot CPU/etc. uart1 is connected to a uart on slot1's BIC, uart2 to slot2, etc. We also have a work-in-progress QEMU model for this: https://lore.kernel.org/qemu-devel/20220714154456.2565189-1-clg@kaod.org/ > > > > > + chosen { > > > > + stdout-path = &uart5; > > > > + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; > > > > + }; > > > > Also: if we do serial0 = &uart5, it should be console=ttyS0, not ttyS4. > > > > > > > > The bootargs should really come from the boot loader. > > > > What if we want to boot the kernel by itself with QEMU? It's kinda annoying to > > have to specify '-append "console=ttyS0,57600n8...' everytime, or to have to use > > a wrapper script. But, it's also a source of bugs: I realized yesterday the > > dts we were using here: > > > > https://github.com/facebook/openbmc-linux/blob/e26c76992e0761d9e440ff514538009384c094b4/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts > > > > Has the wrong console setting. > > You can encode the uart settings like > > stdout-path = "serial0:115200n8" > > the rest really should be passed on the command line, not in > the DT shipped with the kernel. > > Arnd
On Thu, Jul 14, 2022 at 01:24:59PM -0700, Peter Delevoryas wrote: > On Thu, Jul 14, 2022 at 10:15:15PM +0200, Arnd Bergmann wrote: > > On Thu, Jul 14, 2022 at 8:05 PM Peter Delevoryas <peter@pjd.dev> wrote: > > > On Thu, Jul 07, 2022 at 03:33:48PM +0200, Arnd Bergmann wrote: > > > > > + model = "Facebook fby35"; > > > > > + compatible = "facebook,fby35", "aspeed,ast2600"; > > > > > + > > > > > + aliases { > > > > > + serial4 = &uart5; > > > > > + }; > > > > > > > > Why not start at serial0 here? > > > > > > Hey, Facebook person jumping in here (using a personal email): > > > > > > I think you're right, it should be like this: > > > > > > aliases { > > > serial0 = &uart5; > > > serial1 = &uart1; > > > serial2 = &uart2; > > > serial3 = &uart3; > > > serial4 = &uart4; > > > }; > > > > Are you actually using all five uarts though? > > Actually yes, I should have mentioned this in my previous message. > > YosemiteV3.5 is similar to YosemiteV3, which you can see here: > > https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server > > This dts is for the BMC on the sled baseboard, and it manages the 4 slots in the > sled. Each slot has a "Bridge Interconnect" (BIC) (an AST1030) that manages the > slot CPU/etc. uart1 is connected to a uart on slot1's BIC, uart2 to slot2, etc. > > We also have a work-in-progress QEMU model for this: > > https://lore.kernel.org/qemu-devel/20220714154456.2565189-1-clg@kaod.org/ > > > > > > > > + chosen { > > > > > + stdout-path = &uart5; > > > > > + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; > > > > > + }; > > > > > > Also: if we do serial0 = &uart5, it should be console=ttyS0, not ttyS4. > > > > > > > > > > > The bootargs should really come from the boot loader. > > > > > > What if we want to boot the kernel by itself with QEMU? It's kinda annoying to > > > have to specify '-append "console=ttyS0,57600n8...' everytime, or to have to use > > > a wrapper script. But, it's also a source of bugs: I realized yesterday the > > > dts we were using here: > > > > > > https://github.com/facebook/openbmc-linux/blob/e26c76992e0761d9e440ff514538009384c094b4/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts > > > > > > Has the wrong console setting. > > > > You can encode the uart settings like > > > > stdout-path = "serial0:115200n8" > > > > the rest really should be passed on the command line, not in > > the DT shipped with the kernel. Oh sorry, I missed this comment: That sounds good, I'm fine with that. We should remove the bootargs then. Thanks, Peter > > > > Arnd
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 7e0934180724..58add093e5fb 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -1465,6 +1465,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \ aspeed-bmc-facebook-cloudripper.dtb \ aspeed-bmc-facebook-cmm.dtb \ aspeed-bmc-facebook-elbert.dtb \ + aspeed-bmc-facebook-fby35.dtb \ aspeed-bmc-facebook-fuji.dtb \ aspeed-bmc-facebook-galaxy100.dtb \ aspeed-bmc-facebook-minipack.dtb \ diff --git a/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts new file mode 100644 index 000000000000..32262cf1d9ea --- /dev/null +++ b/arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts @@ -0,0 +1,266 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// Copyright (c) 2020 Facebook Inc. + +/dts-v1/; + +#include "aspeed-g6.dtsi" +#include <dt-bindings/gpio/aspeed-gpio.h> +#include <dt-bindings/i2c/i2c.h> + +/ { + model = "Facebook fby35"; + compatible = "facebook,fby35", "aspeed,ast2600"; + + aliases { + serial4 = &uart5; + }; + + chosen { + stdout-path = &uart5; + bootargs = "console=ttyS4,57600n8 root=/dev/ram rw vmalloc=384M"; + }; + + memory@80000000 { + device_type = "memory"; + reg = <0x80000000 0x80000000>; + }; + + iio-hwmon { + compatible = "iio-hwmon"; + io-channels = <&adc0 0>, <&adc0 1>, <&adc0 2>, <&adc0 3>, + <&adc0 4>, <&adc0 5>, <&adc0 6>, <&adc0 7>, + <&adc1 0>, <&adc1 1>, <&adc1 2>, <&adc1 3>, + <&adc1 4>, <&adc1 5>, <&adc1 6>; + }; + spi_gpio: spi-gpio { + status = "okay"; + compatible = "spi-gpio"; + #address-cells = <1>; + #size-cells = <0>; + + gpio-sck = <&gpio0 ASPEED_GPIO(X, 3) GPIO_ACTIVE_HIGH>; + gpio-mosi = <&gpio0 ASPEED_GPIO(X, 4) GPIO_ACTIVE_HIGH>; + gpio-miso = <&gpio0 ASPEED_GPIO(X, 5) GPIO_ACTIVE_HIGH>; + num-chipselects = <1>; + cs-gpios = <&gpio0 ASPEED_GPIO(X, 0) GPIO_ACTIVE_LOW>; + + tpmdev@0 { + compatible = "tcg,tpm_tis-spi"; + spi-max-frequency = <33000000>; + reg = <0>; + }; + }; + +}; + +&mac3 { + status = "okay"; + + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_rmii4_default>; + no-hw-checksum; + use-ncsi; + mlx,multi-host; + ncsi-ctrl,start-redo-probe; + ncsi-ctrl,no-channel-monitor; + ncsi-package = <1>; + ncsi-channel = <1>; + ncsi-rexmit = <1>; + ncsi-timeout = <2>; +}; + +&uart1 { + status = "okay"; +}; + +&uart2 { + status = "okay"; +}; + +&uart3 { + status = "okay"; +}; + +&uart4 { + status = "okay"; +}; + +&uart5 { + status = "okay"; + compatible = "snps,dw-apb-uart"; +}; + +&wdt1 { + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_wdtrst1_default>; + aspeed,reset-type = "soc"; + aspeed,external-signal; + aspeed,ext-push-pull; + aspeed,ext-active-high; + aspeed,ext-pulse-duration = <256>; +}; + +&rtc { + status = "okay"; +}; + +&fmc { + status = "okay"; + flash@0 { + status = "okay"; + m25p,fast-read; + label = "spi0.1"; + spi-max-frequency = <50000000>; + #include "openbmc-flash-layout-128.dtsi" + }; + flash@1 { + status = "okay"; + m25p,fast-read; + label = "spi0.0"; + spi-max-frequency = <50000000>; + #include "openbmc-flash-layout.dtsi" + }; +}; + +&i2c0 { + //Host1 IPMB bus + status = "okay"; + multi-master; + ipmb0@10 { + compatible = "ipmb-dev"; + reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>; + i2c-protocol; + }; +}; + +&i2c1 { + //Host2 IPMB bus + status = "okay"; + multi-master; + ipmb1@10 { + compatible = "ipmb-dev"; + reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>; + i2c-protocol; + }; +}; + +&i2c2 { + //Host3 IPMB bus + status = "okay"; + multi-master; + ipmb2@10 { + compatible = "ipmb-dev"; + reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>; + i2c-protocol; + }; +}; + +&i2c3 { + //Host1 IPMB bus + status = "okay"; + multi-master; + ipmb3@10 { + compatible = "ipmb-dev"; + reg = <(0x10 | I2C_OWN_SLAVE_ADDRESS)>; + i2c-protocol; + }; +}; + +&i2c4 { + status = "okay"; +}; + +&i2c5 { + status = "okay"; +}; + +&i2c6 { + status = "okay"; +}; + +&i2c7 { + status = "okay"; +}; + +&i2c8 { + //NIC SENSOR TEMP + status = "okay"; + tmp421@1f { + compatible = "ti,tmp421"; + reg = <0x1f>; + }; +}; + +&i2c9 { + // Debug-Card IPMB bus + status = "okay"; + multi-master; + ipmb9@30 { + compatible = "ipmb-dev"; + reg = <(0x30 | I2C_OWN_SLAVE_ADDRESS)>; + i2c-protocol; + }; +}; + +&i2c10 { + status = "okay"; +}; + +&i2c11 { + status = "okay"; + //FRU EEPROM + eeprom@51 { + compatible = "atmel,24c64"; + reg = <0x51>; + pagesize = <32>; + }; +}; + +&i2c12 { + status = "okay"; + //INLET TEMP + tmp75@4e { + compatible = "ti,tmp75"; + reg = <0x4e>; + }; + //OUTLET TEMP + tmp75@4f { + compatible = "ti,tmp75"; + reg = <0x4f>; + }; +}; + +&i2c13 { + status = "okay"; +}; + +&adc0 { + ref_voltage = <2500>; + status = "okay"; + + pinctrl-0 = <&pinctrl_adc0_default &pinctrl_adc1_default + &pinctrl_adc2_default &pinctrl_adc3_default + &pinctrl_adc4_default &pinctrl_adc5_default + &pinctrl_adc6_default &pinctrl_adc7_default>; +}; + +&adc1 { + ref_voltage = <2500>; + status = "okay"; + + pinctrl-0 = <&pinctrl_adc8_default &pinctrl_adc9_default + &pinctrl_adc10_default &pinctrl_adc11_default + &pinctrl_adc12_default &pinctrl_adc13_default>; +}; +&ehci0 { + status = "okay"; +}; + +&ehci1 { + status = "okay"; +}; + +&uhci { + status = "okay"; +};
This patch adds linux device tree entry related to Yosemite V3.5 specific devices connected to BMC SoC. Signed-off-by: Logananth Sundararaj <logananth_s@hcl.com> --- --- v3 - addressed v2 patch comments. --- v2 - Enabled i2c drivers. --- v1 - Initial draft. --- --- arch/arm/boot/dts/Makefile | 1 + .../boot/dts/aspeed-bmc-facebook-fby35.dts | 266 ++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 arch/arm/boot/dts/aspeed-bmc-facebook-fby35.dts