| Message ID | 20190413020111.23400-1-paul.walmsley@sifive.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | tty: serial: add DT bindings and serial driver for the SiFive FU540 UART | expand |
Hi Paul, Paul Walmsley <paul.walmsley@sifive.com> writes: > This series adds a serial driver, with console support, for the > UART IP block present on the SiFive FU540 SoC. The programming > model is straightforward, but unique. > > Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the > open-source FSBL (with appropriate patches to the DT data). > > This fifth version fixes a bug in the set_termios handler, > found by Andreas Schwab <schwab@suse.de>. > > The patches in this series can also be found, with the PRCI patches, > DT patches, and DT prerequisite patch, at: > > https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 I tried this branch, and it doesn't boot on my unleashed board. Here's the boot log when I pass the DT built from your branch via u-boot: https://termbin.com/rfp3. I also tried the same thing, but using the DT that's hard-coded into SBI/u-boot. That doesn't boot fully either[1], but one thing I noted is that with the DT from the kernel tree, the printk timestamps aren't moving. Maybe I'm still missing some kconfig options to enable the right clock and/or IRQ controllers? I'm using this fragment[2] on top of the default defconfig (arch/riscv/configs/defconfig). Could you share the defconfig you're using when testing your branch? Also for reference, I'm able to successfully build/boot the 5.1-rc1-unleashed branch from Atish's tree[3] using that kconfig fragment[2] (and the hard-coded DT from u-boot/SBI). Full log here[4]. Thanks, Kevin [1] https://termbin.com/wuc9 [2] CONFIG_CLK_SIFIVE=y CONFIG_CLK_SIFIVE_FU540_PRCI=y CONFIG_SERIAL_SIFIVE=y CONFIG_SERIAL_SIFIVE_CONSOLE=y CONFIG_SIFIVE_PLIC=y CONFIG_SPI=y CONFIG_SPI_SIFIVE=y CONFIG_GPIOLIB=y CONFIG_GPIO_SIFIVE=y CONFIG_PWM_SIFIVE=y CONFIG_CLK_U54_PRCI=y CONFIG_CLK_GEMGXL_MGMT=y [3] https://github.com/atishp04/linux/tree/5.1-rc1-unleashed [4] https://termbin.com/12bg
On 4/18/19 4:22 PM, Kevin Hilman wrote: > Hi Paul, > > Paul Walmsley <paul.walmsley@sifive.com> writes: > >> This series adds a serial driver, with console support, for the >> UART IP block present on the SiFive FU540 SoC. The programming >> model is straightforward, but unique. >> >> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the >> open-source FSBL (with appropriate patches to the DT data). >> >> This fifth version fixes a bug in the set_termios handler, >> found by Andreas Schwab <schwab@suse.de>. >> >> The patches in this series can also be found, with the PRCI patches, >> DT patches, and DT prerequisite patch, at: >> >> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 > > I tried this branch, and it doesn't boot on my unleashed board. > > Here's the boot log when I pass the DT built from your branch via > u-boot: https://termbin.com/rfp3. > Unfortunately, that won't work. The current DT modifications by OpenSBI. 1. Change hart status to "masked" from "okay". 2. M-mode interrupt masking in PLIC node. 3. Add a chosen node for serial access in U-Boot. You can ignore 3 for your use case. However, if you pass a dtb built from source code, that will have hart0 enabled and M-mode interrupts enabled in DT. Not sure if we should do these DT modifications in U-Boot as well. I also noticed that your kernel is booting only 1 hart. Just FYI: RISC-V SMP for U-Boot patches are merged in master. So you should be able to boot all cpus. You can ingore FU540_ENABLED_HART_MASK in OpenSBI build as well. Regards, Atish > I also tried the same thing, but using the DT that's hard-coded into > SBI/u-boot. That doesn't boot fully either[1], but one thing I noted is > that with the DT from the kernel tree, the printk timestamps aren't > moving. Maybe I'm still missing some kconfig options to enable the > right clock and/or IRQ controllers? I'm using this fragment[2] on top of > the default defconfig (arch/riscv/configs/defconfig). > > Could you share the defconfig you're using when testing your branch? > > Also for reference, I'm able to successfully build/boot the > 5.1-rc1-unleashed branch from Atish's tree[3] using that kconfig > fragment[2] (and the hard-coded DT from u-boot/SBI). Full log here[4]. > > Thanks, > > Kevin > > [1] https://termbin.com/wuc9 > [2] > CONFIG_CLK_SIFIVE=y > CONFIG_CLK_SIFIVE_FU540_PRCI=y > > CONFIG_SERIAL_SIFIVE=y > CONFIG_SERIAL_SIFIVE_CONSOLE=y > > CONFIG_SIFIVE_PLIC=y > CONFIG_SPI=y > CONFIG_SPI_SIFIVE=y > CONFIG_GPIOLIB=y > CONFIG_GPIO_SIFIVE=y > CONFIG_PWM_SIFIVE=y > > CONFIG_CLK_U54_PRCI=y > CONFIG_CLK_GEMGXL_MGMT=y > > [3] https://github.com/atishp04/linux/tree/5.1-rc1-unleashed > [4] https://termbin.com/12bg > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv >
Atish Patra <atish.patra@wdc.com> writes: > On 4/18/19 4:22 PM, Kevin Hilman wrote: >> Hi Paul, >> >> Paul Walmsley <paul.walmsley@sifive.com> writes: >> >>> This series adds a serial driver, with console support, for the >>> UART IP block present on the SiFive FU540 SoC. The programming >>> model is straightforward, but unique. >>> >>> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the >>> open-source FSBL (with appropriate patches to the DT data). >>> >>> This fifth version fixes a bug in the set_termios handler, >>> found by Andreas Schwab <schwab@suse.de>. >>> >>> The patches in this series can also be found, with the PRCI patches, >>> DT patches, and DT prerequisite patch, at: >>> >>> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 >> >> I tried this branch, and it doesn't boot on my unleashed board. >> >> Here's the boot log when I pass the DT built from your branch via >> u-boot: https://termbin.com/rfp3. >> > > Unfortunately, that won't work. The current DT modifications by OpenSBI. > > 1. Change hart status to "masked" from "okay". > 2. M-mode interrupt masking in PLIC node. > 3. Add a chosen node for serial access in U-Boot. > > You can ignore 3 for your use case. However, if you pass a dtb built > from source code, that will have hart0 enabled and M-mode interrupts > enabled in DT. Hmm, so what you're saying is there not currently any way to pass a DT built from source using OpenSBI + mainline u-boot? As a short-term workaround, is there a way to make these changes from the u-boot command-line after loading a DTB built from source into memory? If so, I could at least script that part. > Not sure if we should do these DT modifications in U-Boot as well. I guess so (and I'd be happy to test the patch.) Either that, or the upstream DTs (or code) should have those features to the right settings. Speaking of which, I tried to patch the DT from Paul's recent series[1] to make the necessary changes. I can see where to change cpu0 from "okay" to "masked", but I'm not so sure how to make the PLIC change. I was hoping to be able to review/test Paul's DT patches, but now I'm a bit confused as to how to do that. > I also noticed that your kernel is booting only 1 hart. > Just FYI: RISC-V SMP for U-Boot patches are merged in master. So you > should be able to boot all cpus. You can ingore FU540_ENABLED_HART_MASK > in OpenSBI build as well. Ah, nice. I've just updated to u-boot master branch with SMP enabled, and build a new openSBI (also from master branch) with u-boot payload. Using your v5.1-rc4_unleashed branch, I see 4 CPUs booting: https://termbin.com/kg13 Thanks, Kevin [1] https://lore.kernel.org/lkml/20190413020111.23400-1-paul.walmsley@sifive.com/T/#m77daa2857b76ec7cbca0672ad03ae286f61ca0e6
On 4/19/19 12:18 PM, Kevin Hilman wrote: > Atish Patra <atish.patra@wdc.com> writes: > >> On 4/18/19 4:22 PM, Kevin Hilman wrote: >>> Hi Paul, >>> >>> Paul Walmsley <paul.walmsley@sifive.com> writes: >>> >>>> This series adds a serial driver, with console support, for the >>>> UART IP block present on the SiFive FU540 SoC. The programming >>>> model is straightforward, but unique. >>>> >>>> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the >>>> open-source FSBL (with appropriate patches to the DT data). >>>> >>>> This fifth version fixes a bug in the set_termios handler, >>>> found by Andreas Schwab <schwab@suse.de>. >>>> >>>> The patches in this series can also be found, with the PRCI patches, >>>> DT patches, and DT prerequisite patch, at: >>>> >>>> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 >>> >>> I tried this branch, and it doesn't boot on my unleashed board. >>> >>> Here's the boot log when I pass the DT built from your branch via >>> u-boot: https://termbin.com/rfp3. >>> >> >> Unfortunately, that won't work. The current DT modifications by OpenSBI. >> >> 1. Change hart status to "masked" from "okay". >> 2. M-mode interrupt masking in PLIC node. >> 3. Add a chosen node for serial access in U-Boot. >> >> You can ignore 3 for your use case. However, if you pass a dtb built >> from source code, that will have hart0 enabled and M-mode interrupts >> enabled in DT. > > Hmm, so what you're saying is there not currently any way to pass a DT > built from source using OpenSBI + mainline u-boot? > OpenSBI can accept DT built from source with following build option. FW_PAYLOAD_FDT="<unleashed>.dtb" More documentation: https://github.com/riscv/opensbi/blob/master/docs/firmware/fw_payload.md > As a short-term workaround, is there a way to make these changes from > the u-boot command-line after loading a DTB built from source into > memory? If so, I could at least script that part. > > >> Not sure if we should do these DT modifications in U-Boot as well. > > I guess so (and I'd be happy to test the patch.) > > Either that, or the upstream DTs (or code) should have those features to > the right settings. > > Speaking of which, I tried to patch the DT from Paul's recent series[1] > to make the necessary changes. I can see where to change cpu0 from > "okay" to "masked", but I'm not so sure how to make the PLIC change. > Here is the code snippet of how OpenSBI modifies the DT. https://github.com/riscv/opensbi/blob/master/platform/sifive/fu540/platform.c#L53 If you just want to use custom built DTB, you can use OpenSBI build option instead of scripting these. We don't want OpenSBI to keep modifying the DT forever. But we have to do it until there is a better solution available. > I was hoping to be able to review/test Paul's DT patches, but now I'm a > bit confused as to how to do that. > >> I also noticed that your kernel is booting only 1 hart. >> Just FYI: RISC-V SMP for U-Boot patches are merged in master. So you >> should be able to boot all cpus. You can ingore FU540_ENABLED_HART_MASK >> in OpenSBI build as well. > > Ah, nice. > > I've just updated to u-boot master branch with SMP enabled, and build a > new openSBI (also from master branch) with u-boot payload. Using your > v5.1-rc4_unleashed branch, I see 4 CPUs booting: > https://termbin.com/kg13 > Great. Regards, Atish > Thanks, > > Kevin > > [1] https://lore.kernel.org/lkml/20190413020111.23400-1-paul.walmsley@sifive.com/T/#m77daa2857b76ec7cbca0672ad03ae286f61ca0e6 >
Atish Patra <atish.patra@wdc.com> writes: > On 4/19/19 12:18 PM, Kevin Hilman wrote: >> Atish Patra <atish.patra@wdc.com> writes: >> >>> On 4/18/19 4:22 PM, Kevin Hilman wrote: >>>> Hi Paul, >>>> >>>> Paul Walmsley <paul.walmsley@sifive.com> writes: >>>> >>>>> This series adds a serial driver, with console support, for the >>>>> UART IP block present on the SiFive FU540 SoC. The programming >>>>> model is straightforward, but unique. >>>>> >>>>> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the >>>>> open-source FSBL (with appropriate patches to the DT data). >>>>> >>>>> This fifth version fixes a bug in the set_termios handler, >>>>> found by Andreas Schwab <schwab@suse.de>. >>>>> >>>>> The patches in this series can also be found, with the PRCI patches, >>>>> DT patches, and DT prerequisite patch, at: >>>>> >>>>> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 >>>> >>>> I tried this branch, and it doesn't boot on my unleashed board. >>>> >>>> Here's the boot log when I pass the DT built from your branch via >>>> u-boot: https://termbin.com/rfp3. >>>> >>> >>> Unfortunately, that won't work. The current DT modifications by OpenSBI. >>> >>> 1. Change hart status to "masked" from "okay". >>> 2. M-mode interrupt masking in PLIC node. >>> 3. Add a chosen node for serial access in U-Boot. >>> >>> You can ignore 3 for your use case. However, if you pass a dtb built >>> from source code, that will have hart0 enabled and M-mode interrupts >>> enabled in DT. >> >> Hmm, so what you're saying is there not currently any way to pass a DT >> built from source using OpenSBI + mainline u-boot? >> > > OpenSBI can accept DT built from source with following build option. > > FW_PAYLOAD_FDT="<unleashed>.dtb" > > More documentation: > https://github.com/riscv/opensbi/blob/master/docs/firmware/fw_payload.md I'm aware of that method, but I'm looking for a way that doesn't require me to rebuild/reflash SBI every time. Basically, I want u-boot to TFTP the DTB (along with kernel and ramdisk) and boot it from memory. On all other DT platforms in kernelCI, we build DTB(s) along with the kernel (but not built into the kernel.) We then use the bootloader to load the kernel, DTB and ramdisk that we built. >> As a short-term workaround, is there a way to make these changes from >> the u-boot command-line after loading a DTB built from source into >> memory? If so, I could at least script that part. >> >> >>> Not sure if we should do these DT modifications in U-Boot as well. >> >> I guess so (and I'd be happy to test the patch.) >> >> Either that, or the upstream DTs (or code) should have those features to >> the right settings. >> >> Speaking of which, I tried to patch the DT from Paul's recent series[1] >> to make the necessary changes. I can see where to change cpu0 from >> "okay" to "masked", but I'm not so sure how to make the PLIC change. >> > > Here is the code snippet of how OpenSBI modifies the DT. > > https://github.com/riscv/opensbi/blob/master/platform/sifive/fu540/platform.c#L53 Thanks, based on that, I was able to modify the DTB I'm builing from source[1], but it still doesn't fully boot. Looks like Paul has so far only tested this with BBL + FSBL, so I think I'll wait to hear from him how that setup might be different from using OpenSBI + u-boot. Thanks for all the help, Kevin [1] diff --git a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi index dd3b9395cedf..299398c4201d 100644 --- a/arch/riscv/boot/dts/sifive/fu540-c000.dtsi +++ b/arch/riscv/boot/dts/sifive/fu540-c000.dtsi @@ -30,7 +30,7 @@ i-cache-size = <16384>; reg = <0>; riscv,isa = "rv64imac"; - status = "okay"; + status = "masked"; cpu0_intc: interrupt-controller { #interrupt-cells = <1>; compatible = "riscv,cpu-intc"; @@ -148,11 +148,11 @@ reg = <0x0 0xc000000 0x0 0x4000000>; interrupt-controller; interrupts-extended = < - &cpu0_intc 11 - &cpu1_intc 11 &cpu1_intc 9 - &cpu2_intc 11 &cpu2_intc 9 - &cpu3_intc 11 &cpu3_intc 9 - &cpu4_intc 11 &cpu4_intc 9>; + &cpu0_intc 0xffffffff + &cpu1_intc 0xffffffff &cpu1_intc 9 + &cpu2_intc 0xffffffff &cpu2_intc 9 + &cpu3_intc 0xffffffff &cpu3_intc 9 + &cpu4_intc 0xffffffff &cpu4_intc 9>; }; prci: clock-controller@10000000 { compatible = "sifive,fu540-c000-prci";
On Fri, 19 Apr 2019, Kevin Hilman wrote: > Looks like Paul has so far only tested this with BBL + FSBL, so I think > I'll wait to hear from him how that setup might be different from using > OpenSBI + u-boot. I'd recommend testing the DT patches with BBL and the open-source FSBL. That's the traditional way of booting RISC-V Linux systems. The goal is to transition to U-Boot over time. However, right now the U-boot port is still new. It wouldn't surprise me if we need to modify the kernel, U-boot, or the SBI layers as part of that process. In the short term, we need to get some sort of baseline DT data in place before more chips and boards start showing up. I'll post a separate message with details on how to reproduce the test setup that I'm using. - Paul
Hi Kevin, On Fri, 19 Apr 2019, Kevin Hilman wrote: > Atish Patra <atish.patra@wdc.com> writes: > > On 4/18/19 4:22 PM, Kevin Hilman wrote: > >> Paul Walmsley <paul.walmsley@sifive.com> writes: > >> > >>> This series adds a serial driver, with console support, for the > >>> UART IP block present on the SiFive FU540 SoC. The programming > >>> model is straightforward, but unique. > >>> > >>> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the > >>> open-source FSBL (with appropriate patches to the DT data). > >>> > >>> This fifth version fixes a bug in the set_termios handler, > >>> found by Andreas Schwab <schwab@suse.de>. > >>> > >>> The patches in this series can also be found, with the PRCI patches, > >>> DT patches, and DT prerequisite patch, at: > >>> > >>> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 ... > I've just updated to u-boot master branch with SMP enabled, and build a > new openSBI (also from master branch) with u-boot payload. Using your > v5.1-rc4_unleashed branch, I see 4 CPUs booting: > https://termbin.com/kg13 Now that the serial driver is working for you, care to send a Tested-by: ? thanks, - Paul
Paul Walmsley <paul.walmsley@sifive.com> writes: > On Fri, 19 Apr 2019, Kevin Hilman wrote: > >> Looks like Paul has so far only tested this with BBL + FSBL, so I think >> I'll wait to hear from him how that setup might be different from using >> OpenSBI + u-boot. > > I'd recommend testing the DT patches with BBL and the open-source FSBL. > That's the traditional way of booting RISC-V Linux systems. OK, but as you know, not the tradiaional way of booting most other linux systems. ;) I'm working on getting RISC-V supported in kernelCI in a fully-automated way, and I don't currently have the time to add add support for BBL+FSBL to kernelCI automation tooling, so having u-boot support is the best way to get support in kernelCI, IMO. Kevin
On Thu, 2 May 2019, Kevin Hilman wrote: > Paul Walmsley <paul.walmsley@sifive.com> writes: > > > I'd recommend testing the DT patches with BBL and the open-source FSBL. > > That's the traditional way of booting RISC-V Linux systems. > > OK, but as you know, not the tradiaional way of booting most other linux > systems. ;) > > I'm working on getting RISC-V supported in kernelCI in a fully-automated > way, and I don't currently have the time to add add support for BBL+FSBL > to kernelCI automation tooling, so having u-boot support is the best way > to get support in kernelCI, IMO. That's great. Please keep hacking away on RISC-V support for kernelCI. My point is just that the U-boot and OpenSBI software stack you're working with is not going to be useful for automatic tests of some kernel patches yet. That stack is still very new, and was written around a non-upstream set of DT data. We are in the process of posting and merging patches to fix that, but it's going to take a few releases of both the kernel and those other boot stack components until things are sorted out in a more durable way. - Paul
On Thu 18 Apr 2019 at 18:04, Atish Patra <atish.patra@wdc.com> wrote: > On 4/18/19 4:22 PM, Kevin Hilman wrote: >> Hi Paul, >> >> Paul Walmsley <paul.walmsley@sifive.com> writes: >> >>> This series adds a serial driver, with console support, for the >>> UART IP block present on the SiFive FU540 SoC. The programming >>> model is straightforward, but unique. >>> >>> Boot-tested on a SiFive FU540 HiFive-U board, using BBL and the >>> open-source FSBL (with appropriate patches to the DT data). >>> >>> This fifth version fixes a bug in the set_termios handler, >>> found by Andreas Schwab <schwab@suse.de>. >>> >>> The patches in this series can also be found, with the PRCI patches, >>> DT patches, and DT prerequisite patch, at: >>> >>> https://github.com/sifive/riscv-linux/tree/dev/paulw/serial-v5.1-rc4 >> >> I tried this branch, and it doesn't boot on my unleashed board. >> >> Here's the boot log when I pass the DT built from your branch via >> u-boot: https://termbin.com/rfp3. >> > > Unfortunately, that won't work. The current DT modifications by OpenSBI. > > 1. Change hart status to "masked" from "okay". > 2. M-mode interrupt masking in PLIC node. > 3. Add a chosen node for serial access in U-Boot. > > You can ignore 3 for your use case. However, if you pass a dtb built from source > code, that will have hart0 enabled and M-mode interrupts enabled in DT. Atish, I'm trying to get the kernel boot with the current linux kernel DT from Paul's patch series [0]. Could you point me to some documentation on 2. ? Or do you know of a way to disable M-mode interrupts from U-boot ? [0]: https://lore.kernel.org/patchwork/project/lkml/list/?series=390077 Thanks, Loys > > Not sure if we should do these DT modifications in U-Boot as well. >