| Message ID | 20211116150119.2171-1-kernel@esmil.dk |
|---|---|
| Headers | show |
| Series | Basic StarFive JH7100 RISC-V SoC support | expand |
On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > This series adds support for the StarFive JH7100 RISC-V SoC. The SoC has > many devices that need non-coherent dma operations to work which isn't > upstream yet[1], so this just adds basic support to boot up, get a > serial console, blink an LED and reboot itself. Unlike the Allwinner D1 > this chip doesn't use any extra pagetable bits, but instead the DDR RAM > appears twice in the memory map, with and without the cache. > > The JH7100 is a test chip for the upcoming JH7110 and about 300 BeagleV > Starlight Beta boards were sent out with them as part of a now cancelled > BeagleBoard.org project. However StarFive has produced more of the > JH7100s and more boards will be available[2] to buy. I've seen pictures > of the new boards now, so hopefully before the end of the year. > > This series is also available at > https://github.com/esmil/linux/commits/starlight-minimal > ..but a more complete kernel including drivers for non-coherent > peripherals based on this series can be found at > https://github.com/starfive-tech/linux/tree/visionfive > > [1]: https://lore.kernel.org/linux-riscv/20210723214031.3251801-2-atish.patra@wdc.com/ > [2]: https://www.linkedin.com/pulse/starfive-release-open-source-single-board-platform-q3-2021-starfive/ Thanks for adding me to Cc, I've had a look at the series and didn't see anything wrong with it, and I'm happy to merge it through the SoC tree for the initial support in 5.17, provided you get an Ack from the arch/riscv maintainers for it. One general (minor) comment about the patches: please put your own 'Signed-off-by' into the last line of the patch description, below all the lines you took from other people, so instead of: | Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> | Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> | Acked-by: Rob Herring <robh@kernel.org> do this: | Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> | Acked-by: Rob Herring <robh@kernel.org> | Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> Regarding the coherency issue, it's a bit sad to see yet another hacky workaround in the hardware, but as you say this is unrelated to the driver series. I'd actually argue that this one isn't that different from the other hack you describe, except this steals the pagetable bits from the address instead of the reserved flags... Arnd
On Tue, Nov 16, 2021 at 6:09 PM Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: ... > One general (minor) comment about the patches: please put your own > 'Signed-off-by' > into the last line of the patch description, below all the lines you > took from other people, so > instead of: > > | Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > | Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > | Acked-by: Rob Herring <robh@kernel.org> > > do this: > > | Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > | Acked-by: Rob Herring <robh@kernel.org> > | Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> Why? Submitting Patches tells about chronological order and last SoB to be from the submitter. These both are correct. Note the difference between 'last SoB' and 'SoB to be last [line]'. Here is the excerpt: "Notably, the last Signed-off-by: must always be that of the developer submitting the patch."
On Tue, Nov 16, 2021 at 5:13 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Nov 16, 2021 at 6:09 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > Why? > Submitting Patches tells about chronological order and last SoB to be > from the submitter. > These both are correct. Note the difference between 'last SoB' and > 'SoB to be last [line]'. > > Here is the excerpt: > "Notably, the last Signed-off-by: must always be that of the developer > submitting the patch." I think having the S-o-b in the final line is far more common, and it does help identify who added the other tags, i.e. the person signing off immediately below. I don't reject patches that do this the other way round, but it's something that felt unusual here. Arnd
On Tue, 16 Nov 2021 at 17:44, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Nov 16, 2021 at 5:13 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Tue, Nov 16, 2021 at 6:09 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > > > Why? > > Submitting Patches tells about chronological order and last SoB to be > > from the submitter. > > These both are correct. Note the difference between 'last SoB' and > > 'SoB to be last [line]'. > > > > Here is the excerpt: > > "Notably, the last Signed-off-by: must always be that of the developer > > submitting the patch." > > I think having the S-o-b in the final line is far more common, and it does > help identify who added the other tags, i.e. the person signing off > immediately below. I don't reject patches that do this the other way round, > but it's something that felt unusual here. Then I'll stick to what's most common. In any case patch 12 and 16 got it wrong by both conventions. /Emil
On Tue, 16 Nov 2021 at 17:08, Arnd Bergmann <arnd@arndb.de> wrote: > On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > > > > This series adds support for the StarFive JH7100 RISC-V SoC. The SoC has > > many devices that need non-coherent dma operations to work which isn't > > upstream yet[1], so this just adds basic support to boot up, get a > > serial console, blink an LED and reboot itself. Unlike the Allwinner D1 > > this chip doesn't use any extra pagetable bits, but instead the DDR RAM > > appears twice in the memory map, with and without the cache. > > > > The JH7100 is a test chip for the upcoming JH7110 and about 300 BeagleV > > Starlight Beta boards were sent out with them as part of a now cancelled > > BeagleBoard.org project. However StarFive has produced more of the > > JH7100s and more boards will be available[2] to buy. I've seen pictures > > of the new boards now, so hopefully before the end of the year. > > > > This series is also available at > > https://github.com/esmil/linux/commits/starlight-minimal > > ..but a more complete kernel including drivers for non-coherent > > peripherals based on this series can be found at > > https://github.com/starfive-tech/linux/tree/visionfive > > > > [1]: https://lore.kernel.org/linux-riscv/20210723214031.3251801-2-atish.patra@wdc.com/ > > [2]: https://www.linkedin.com/pulse/starfive-release-open-source-single-board-platform-q3-2021-starfive/ > > Thanks for adding me to Cc, I've had a look at the series and didn't > see anything > wrong with it, and I'm happy to merge it through the SoC tree for the > initial support > in 5.17, provided you get an Ack from the arch/riscv maintainers for it. Cool! @Palmer, do you mind looking through this? Probably patch 1, 15 and 16 are the most relevant to you. > Regarding the coherency issue, it's a bit sad to see yet another hacky > workaround > in the hardware, but as you say this is unrelated to the driver > series. I'd actually > argue that this one isn't that different from the other hack you > describe, except > this steals the pagetable bits from the address instead of the reserved flags... Yeah, it's definitely a hack, but at least it's not using bits the spec said was reserved. Hopefully the JH7110 will be fully coherent or maybe implement the new Svpbmt extension. /Emil /Emil
On Tue, 16 Nov 2021 09:28:41 PST (-0800), kernel@esmil.dk wrote: > On Tue, 16 Nov 2021 at 17:08, Arnd Bergmann <arnd@arndb.de> wrote: >> On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: >> > >> > This series adds support for the StarFive JH7100 RISC-V SoC. The SoC has >> > many devices that need non-coherent dma operations to work which isn't >> > upstream yet[1], so this just adds basic support to boot up, get a >> > serial console, blink an LED and reboot itself. Unlike the Allwinner D1 >> > this chip doesn't use any extra pagetable bits, but instead the DDR RAM >> > appears twice in the memory map, with and without the cache. >> > >> > The JH7100 is a test chip for the upcoming JH7110 and about 300 BeagleV >> > Starlight Beta boards were sent out with them as part of a now cancelled >> > BeagleBoard.org project. However StarFive has produced more of the >> > JH7100s and more boards will be available[2] to buy. I've seen pictures >> > of the new boards now, so hopefully before the end of the year. >> > >> > This series is also available at >> > https://github.com/esmil/linux/commits/starlight-minimal >> > ..but a more complete kernel including drivers for non-coherent >> > peripherals based on this series can be found at >> > https://github.com/starfive-tech/linux/tree/visionfive >> > >> > [1]: https://lore.kernel.org/linux-riscv/20210723214031.3251801-2-atish.patra@wdc.com/ >> > [2]: https://www.linkedin.com/pulse/starfive-release-open-source-single-board-platform-q3-2021-starfive/ >> >> Thanks for adding me to Cc, I've had a look at the series and didn't >> see anything >> wrong with it, and I'm happy to merge it through the SoC tree for the >> initial support >> in 5.17, provided you get an Ack from the arch/riscv maintainers for it. > > Cool! > > @Palmer, do you mind looking through this? Probably patch 1, 15 and 16 > are the most relevant to you. > >> Regarding the coherency issue, it's a bit sad to see yet another hacky >> workaround >> in the hardware, but as you say this is unrelated to the driver >> series. I'd actually >> argue that this one isn't that different from the other hack you >> describe, except >> this steals the pagetable bits from the address instead of the reserved flags... > > Yeah, it's definitely a hack, but at least it's not using bits the > spec said was reserved. Hopefully the JH7110 will be fully coherent or > maybe implement the new Svpbmt extension. Sorry, this had been sitting on top of my inbox because I hadn't had a chance to figure this stuff out. Emil poked me on IRC about it, but I figured I'd just write it here so everyone can see: IMO there's a huge difference between the StarFive-flavored non-coherent stuff (which relies on physical aliasing) and the T-Head-flavored stuff (which uses page table bits): the PA-aliasing approach is allowed by the ISA, while the page table bits aren't (they're marked as reserved). IMO we should still figure out a way to take the T-Head stuff, as it's the real-ist hardware we have, but that's a whole different can of worms. My worry with this is I've yet to actually be convinced that either of these approaches work. Specifically, neither of them prevents M-mode from performing (either directly or as a side effect of something like speculation) accesses that violate the attributes we're ascribing to regions in Linux. IIRC I pointed that out in the Svpmbt patch set, which has exactly the same set of problems. That said, I don't really care all that much -- having something here is better than nothing, and we've always relied on the HW vendors just producing HW that works when it comes to any of the IO stuff (ie, even on coherent systems). These are all drivers so it's really up to those folks where the bar is, so as long as everyone's on the page about that you're not going to get any objections from me so Acked-by: Palmer Dabbelt <palmer@rivosinc.com> The SOC tree works for me. It'd be great to have a shared tag I where I can pull in at least the Kconfig.socs stuff, but if that's not easy then it's no big deal -- what's in flight there is pretty trivial on my end, so we can just deal with the merge conflicts. Thanks!
On Sat, 27 Nov 2021 at 02:30, Palmer Dabbelt <palmer@rivosinc.com> wrote: > On Tue, 16 Nov 2021 09:28:41 PST (-0800), kernel@esmil.dk wrote: > > On Tue, 16 Nov 2021 at 17:08, Arnd Bergmann <arnd@arndb.de> wrote: > >> On Tue, Nov 16, 2021 at 4:01 PM Emil Renner Berthing <kernel@esmil.dk> wrote: > >> > > >> > This series adds support for the StarFive JH7100 RISC-V SoC. The SoC has > >> > many devices that need non-coherent dma operations to work which isn't > >> > upstream yet[1], so this just adds basic support to boot up, get a > >> > serial console, blink an LED and reboot itself. Unlike the Allwinner D1 > >> > this chip doesn't use any extra pagetable bits, but instead the DDR RAM > >> > appears twice in the memory map, with and without the cache. > >> > > >> > The JH7100 is a test chip for the upcoming JH7110 and about 300 BeagleV > >> > Starlight Beta boards were sent out with them as part of a now cancelled > >> > BeagleBoard.org project. However StarFive has produced more of the > >> > JH7100s and more boards will be available[2] to buy. I've seen pictures > >> > of the new boards now, so hopefully before the end of the year. > >> > > >> > This series is also available at > >> > https://github.com/esmil/linux/commits/starlight-minimal > >> > ..but a more complete kernel including drivers for non-coherent > >> > peripherals based on this series can be found at > >> > https://github.com/starfive-tech/linux/tree/visionfive > >> > > >> > [1]: https://lore.kernel.org/linux-riscv/20210723214031.3251801-2-atish.patra@wdc.com/ > >> > [2]: https://www.linkedin.com/pulse/starfive-release-open-source-single-board-platform-q3-2021-starfive/ > >> > >> Thanks for adding me to Cc, I've had a look at the series and didn't > >> see anything > >> wrong with it, and I'm happy to merge it through the SoC tree for the > >> initial support > >> in 5.17, provided you get an Ack from the arch/riscv maintainers for it. > > > > Cool! > > > > @Palmer, do you mind looking through this? Probably patch 1, 15 and 16 > > are the most relevant to you. > > > >> Regarding the coherency issue, it's a bit sad to see yet another hacky > >> workaround > >> in the hardware, but as you say this is unrelated to the driver > >> series. I'd actually > >> argue that this one isn't that different from the other hack you > >> describe, except > >> this steals the pagetable bits from the address instead of the reserved flags... > > > > Yeah, it's definitely a hack, but at least it's not using bits the > > spec said was reserved. Hopefully the JH7110 will be fully coherent or > > maybe implement the new Svpbmt extension. > > Sorry, this had been sitting on top of my inbox because I hadn't had a > chance to figure this stuff out. Emil poked me on IRC about it, but I > figured I'd just write it here so everyone can see: > > IMO there's a huge difference between the StarFive-flavored non-coherent > stuff (which relies on physical aliasing) and the T-Head-flavored stuff > (which uses page table bits): the PA-aliasing approach is allowed by the > ISA, while the page table bits aren't (they're marked as reserved). IMO > we should still figure out a way to take the T-Head stuff, as it's the > real-ist hardware we have, but that's a whole different can of worms. > > My worry with this is I've yet to actually be convinced that either of > these approaches work. Specifically, neither of them prevents M-mode > from performing (either directly or as a side effect of something like > speculation) accesses that violate the attributes we're ascribing to > regions in Linux. IIRC I pointed that out in the Svpmbt patch set, > which has exactly the same set of problems. > > That said, I don't really care all that much -- having something here is > better than nothing, and we've always relied on the HW vendors just > producing HW that works when it comes to any of the IO stuff (ie, even > on coherent systems). These are all drivers so it's really up to those > folks where the bar is, so as long as everyone's on the page about that > you're not going to get any objections from me so > > Acked-by: Palmer Dabbelt <palmer@rivosinc.com> Thanks! Yeah, as you say something is better than nothing. I'd think it's helpful to have something to build and test the non-coherent dma functionality on, and the nice thing about the Starfive-flavour is that it will actually boot into an initramfs root and have a working serial console even without the non-coherent dmas working. > The SOC tree works for me. It'd be great to have a shared tag I where I > can pull in at least the Kconfig.socs stuff, but if that's not easy then > it's no big deal -- what's in flight there is pretty trivial on my end, > so we can just deal with the merge conflicts. I guess this is for the SoC maintainers to consider. Let me know if that's wrong and I need to do anything. /Emil