| Message ID | 20181129130559.66732-3-chris.brandt@renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add Initial Device Tree for RZ/A2 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 4 warnings, 148 lines checked" |
On Thu, Nov 29, 2018 at 08:05:59AM -0500, Chris Brandt wrote: > Add support for Renesas RZ/A2M evaluation board. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > Documentation/devicetree/bindings/arm/shmobile.txt | 2 + > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/r7s9210-rza2mevb.dts | 133 +++++++++++++++++++++ > 3 files changed, 136 insertions(+) > create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts > > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt > index a94db59d8e2d..67a6810c4345 100644 > --- a/Documentation/devicetree/bindings/arm/shmobile.txt > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt > @@ -125,6 +125,8 @@ Boards: > compatible = "renesas,porter", "renesas,r8a7791" > - RSKRZA1 (YR0K77210C000BE) > compatible = "renesas,rskrza1", "renesas,r7s72100" > + - RZ/A2M Eval Board (RTK7921053S00000BE) > + compatible = "renesas,rza2mevb", "renesas,r7s9210" > - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > compatible = "renesas,rzn1d400-db", "renesas,r9a06g032" > - Salvator-X (RTP0RC7795SIPB0010S) Pleas separate the change above into a separate patch. Such changes typically go upstream via a separate branch. > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index b0e966d625b9..fbdd068fd8af 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \ > r7s72100-genmai.dtb \ > r7s72100-gr-peach.dtb \ > r7s72100-rskrza1.dtb \ > + r7s9210-rza2mevb.dtb \ > r8a73a4-ape6evm.dtb \ > r8a7740-armadillo800eva.dtb \ > r8a7743-iwg20d-q7.dtb \ > diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts > new file mode 100644 > index 000000000000..9570aeb8f1ce > --- /dev/null > +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts > @@ -0,0 +1,133 @@ > +/* > + * Device Tree Source for the RZA2MEVB board > + * > + * Copyright (C) 2018 Renesas Electronics > + * > + * This file is dual-licensed: you can use it either under the terms > + * of the GPL or the X11 license, at your option. Note that this dual > + * licensing only applies to this file, and not this project as a > + * whole. > + * > + * a) This file is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of the > + * License, or (at your option) any later version. > + * > + * This file is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * Or, alternatively, > + * > + * b) Permission is hereby granted, free of charge, to any person > + * obtaining a copy of this software and associated documentation > + * files (the "Software"), to deal in the Software without > + * restriction, including without limitation the rights to use, > + * copy, modify, merge, publish, distribute, sublicense, and/or > + * sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following > + * conditions: > + * > + * The above copyright notice and this permission notice shall be > + * included in all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ I am wondering about the motivation for dual-licensing this file. It does not seem to be something Renesas has done before with upstream DT. I am also wondering if the dual licence, if it remains, can be described using SPDX. > + > +/dts-v1/; > +#include "r7s9210.dtsi" > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/r7s9210-pinctrl.h> > + > +/ { > + model = "RZA2MEVB"; > + compatible = "renesas,rza2mevb", "renesas,r7s9210"; > + > + aliases { > + serial0 = &scif4; > + }; > + > + chosen { > + bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs"; > + stdout-path = "serial0:115200n8"; > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x40000000 0x00800000>; /* HyperRAM */ > + }; > + > + lbsc { > + #address-cells = <1>; > + #size-cells = <1>; > + }; > + > + leds { > + status = "okay"; > + compatible = "gpio-leds"; > + > + led0 { > + gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>; > + }; > + }; > + > + /* Cramfs XIP File System in QSPI */ > + qspi@20000000 { > + compatible = "mtd-rom"; > + probe-type = "direct-mapped"; /* XIP from QSPI */ > + reg = <0x20000000 0x4000000>; /* 64 MB*/ > + bank-width = <4>; > + device-width = <1>; > + #address-cells = <1>; > + #size-cells = <1>; > + partition@800000 { > + label ="rootfs"; > + reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */ > + read-only; > + }; > + }; > +}; > + > +/* EXTAL */ > +&extal_clk { > + clock-frequency = <24000000>; /* 24MHz */ > +}; > + > +/* RTC_X1 */ > +&rtc_x1_clk { > + clock-frequency = <32768>; > +}; > + > +&pinctrl { > + > + /* Serial Console */ > + scif4_pins: serial4 { > + pinmux = <RZA2_PINMUX(PORT9, 0, 4)>, /* TxD4 */ > + <RZA2_PINMUX(PORT9, 1, 4)>; /* RxD4 */ > + }; > +}; > + > +/* High resolution System tick timers */ > +&ostm0 { > + status = "okay"; > +}; > + > +&ostm1 { > + status = "okay"; > +}; > + > +/* Serial Console */ > +&scif4 { > + pinctrl-names = "default"; > + pinctrl-0 = <&scif4_pins>; > + > + status = "okay"; > +}; > -- > 2.16.1 >
Hi Simon, On Friday, November 30, 2018, Simon Horman wrote: > > + - RZ/A2M Eval Board (RTK7921053S00000BE) > > + compatible = "renesas,rza2mevb", "renesas,r7s9210" > > - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > > compatible = "renesas,rzn1d400-db", "renesas,r9a06g032" > > - Salvator-X (RTP0RC7795SIPB0010S) > > Pleas separate the change above into a separate patch. > Such changes typically go upstream via a separate branch. OK. Looking through the history log, I saw it different ways, so I was not sure. > I am wondering about the motivation for dual-licensing this file. > It does not seem to be something Renesas has done before with > upstream DT. > > I am also wondering if the dual licence, if it remains, can be > described using SPDX. A while back, I was reading/hearing about how board DT file do not have to be GPL and the user should not have to be forced to make this GPL. (Maybe at some ELC conference or on LWN or something) So in our RZ/A BSP that I release to customers I would use this dual license. You can see the exact same license in a number of dts files in mainline. Since there is no SPDX for this, I figure Rob might have some opinion on the matter. And, if I have to make it GLP for the mainline version, then I will just replace the license when I release the customer BSP (since I wrote it, I can do that). But, I would be nice to keep the BSP version as close to mainline as I can. Chris
Hi Chris, On Fri, Nov 30, 2018 at 1:20 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Friday, November 30, 2018, Simon Horman wrote: > > I am wondering about the motivation for dual-licensing this file. > > It does not seem to be something Renesas has done before with > > upstream DT. > > > > I am also wondering if the dual licence, if it remains, can be > > described using SPDX. > > A while back, I was reading/hearing about how board DT file do not have > to be GPL and the user should not have to be forced to make this GPL. > (Maybe at some ELC conference or on LWN or something) Indeed. Recently there have been some attempts to fix this. > So in our RZ/A BSP that I release to customers I would use this dual > license. You can see the exact same license in a number of dts files in > mainline. Note that your file includes #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/pinctrl/r7s9210-pinctrl.h> both of which are include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */ include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/* SPDX-License-Identifier: GPL-2.0 */ > Since there is no SPDX for this, I figure Rob might have some opinion on > the matter. > And, if I have to make it GLP for the mainline version, then I will just > replace the license when I release the customer BSP (since I wrote it, > I can do that). But, I would be nice to keep the BSP version as close to > mainline as I can. Documentation/process/license-rules.rst does have SPDX identifiers for dual-licensed files. Perhaps they match the license of your file? Gr{oetje,eeting}s, Geert
Hi Geert, On Friday, November 30, 2018 1, Geert wrote: > > So in our RZ/A BSP that I release to customers I would use this dual > > license. You can see the exact same license in a number of dts files in > > mainline. > > Note that your file includes > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/pinctrl/r7s9210-pinctrl.h> > > both of which are > > include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */ > include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/* > SPDX-License-Identifier: GPL-2.0 */ After I wrote that, I realized that we'll always have: #include "r7s9210.dtsi" Which I have as GPL-2.0....so the dual license idea in pointless. So to make things easy, I'll just drop the dual license thing and just use SPDX GPL-2.0 Chris
Hi Chris, On Fri, Nov 30, 2018 at 5:10 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Friday, November 30, 2018 1, Geert wrote: > > > So in our RZ/A BSP that I release to customers I would use this dual > > > license. You can see the exact same license in a number of dts files in > > > mainline. > > > > Note that your file includes > > > > #include <dt-bindings/gpio/gpio.h> > > #include <dt-bindings/pinctrl/r7s9210-pinctrl.h> > > > > both of which are > > > > include/dt-bindings/gpio/gpio.h:/* SPDX-License-Identifier: GPL-2.0 */ > > include/dt-bindings/pinctrl/r7s9210-pinctrl.h:/* > > SPDX-License-Identifier: GPL-2.0 */ > > After I wrote that, I realized that we'll always have: > #include "r7s9210.dtsi" > Which I have as GPL-2.0....so the dual license idea in pointless. > > So to make things easy, I'll just drop the dual license thing and just > use SPDX GPL-2.0 That's one solution. Another solution would be to relicense all DT binding definitions and DTS files. Cfr. e.g. commit d061864b89c3234b ("ARM: dt: relicense two DT binding IRQ headers"). Gr{oetje,eeting}s, Geert
Hi Chris, On Thu, Nov 29, 2018 at 2:07 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Add support for Renesas RZ/A2M evaluation board. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Thanks for your patch! > index 000000000000..9570aeb8f1ce > --- /dev/null > +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts > @@ -0,0 +1,133 @@ > +/* > + * Device Tree Source for the RZA2MEVB board > + * > + * Copyright (C) 2018 Renesas Electronics > + * > + * This file is dual-licensed: you can use it either under the terms > + * of the GPL or the X11 license, at your option. Note that this dual > + * licensing only applies to this file, and not this project as a > + * whole. Let's see about the license... > +/ { > + model = "RZA2MEVB"; > + compatible = "renesas,rza2mevb", "renesas,r7s9210"; > + > + aliases { > + serial0 = &scif4; > + }; > + > + chosen { > + bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs"; I wouldn't put "earlycon" in the default bootargs. It's meant for debugging. > + stdout-path = "serial0:115200n8"; > + }; > + > + memory@80000000 { 40000000 > + device_type = "memory"; > + reg = <0x40000000 0x00800000>; /* HyperRAM */ > + }; > + > + lbsc { > + #address-cells = <1>; > + #size-cells = <1>; > + }; > + > + leds { > + status = "okay"; Please drop this line, "okay" is the default. > + compatible = "gpio-leds"; > + > + led0 { > + gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>; There's a second (yellow green) LED connected to PC_1. Hence red { gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>; }; green { gpios = <&pinctrl RZA2_PIN(PORTC, 1) GPIO_ACTIVE_HIGH>; } > + }; > + }; > +&pinctrl { > + Please drop this blank line. > + /* Serial Console */ > + scif4_pins: serial4 { > + pinmux = <RZA2_PINMUX(PORT9, 0, 4)>, /* TxD4 */ > + <RZA2_PINMUX(PORT9, 1, 4)>; /* RxD4 */ > + }; > +}; Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for your review. On Tuesday, December 04, 2018 1, Geert Uytterhoeven wrote: > > + * This file is dual-licensed: you can use it either under the terms > > + * of the GPL or the X11 license, at your option. Note that this dual > > + * licensing only applies to this file, and not this project as a > > + * whole. > > Let's see about the license... I did find that I could use "SPDX-License-Identifier: (GPL-2.0+ OR MIT)", but, for the mainline version I will probably just make it GPL. The customer BSP version is going to be different anyway so I'll make that one a dual license. Besides, this board will only boot as XIP_KERNEL which is an impossible configuration in the mainline kernel, so this .dts file is mostly for checking the .dtsi file. Thanks, Chris
Hi Chris, On Tue, Dec 4, 2018 at 5:25 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > Besides, this board will only boot as XIP_KERNEL which is an impossible > configuration in the mainline kernel, so this .dts file is mostly for > checking the .dtsi file. Let's hope we can fix that anytime soon. Gr{oetje,eeting}s, Geert
On Thu, Nov 29, 2018 at 08:05:59AM -0500, Chris Brandt wrote: > Add support for Renesas RZ/A2M evaluation board. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > Documentation/devicetree/bindings/arm/shmobile.txt | 2 + > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/r7s9210-rza2mevb.dts | 133 +++++++++++++++++++++ > 3 files changed, 136 insertions(+) > create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts > > diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt > index a94db59d8e2d..67a6810c4345 100644 > --- a/Documentation/devicetree/bindings/arm/shmobile.txt > +++ b/Documentation/devicetree/bindings/arm/shmobile.txt > @@ -125,6 +125,8 @@ Boards: > compatible = "renesas,porter", "renesas,r8a7791" > - RSKRZA1 (YR0K77210C000BE) > compatible = "renesas,rskrza1", "renesas,r7s72100" > + - RZ/A2M Eval Board (RTK7921053S00000BE) > + compatible = "renesas,rza2mevb", "renesas,r7s9210" > - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) > compatible = "renesas,rzn1d400-db", "renesas,r9a06g032" > - Salvator-X (RTP0RC7795SIPB0010S) > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index b0e966d625b9..fbdd068fd8af 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \ > r7s72100-genmai.dtb \ > r7s72100-gr-peach.dtb \ > r7s72100-rskrza1.dtb \ > + r7s9210-rza2mevb.dtb \ > r8a73a4-ape6evm.dtb \ > r8a7740-armadillo800eva.dtb \ > r8a7743-iwg20d-q7.dtb \ > diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts > new file mode 100644 > index 000000000000..9570aeb8f1ce > --- /dev/null > +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts > @@ -0,0 +1,133 @@ > +/* > + * Device Tree Source for the RZA2MEVB board > + * > + * Copyright (C) 2018 Renesas Electronics > + * > + * This file is dual-licensed: you can use it either under the terms > + * of the GPL or the X11 license, at your option. Note that this dual > + * licensing only applies to this file, and not this project as a > + * whole. > + * > + * a) This file is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of the > + * License, or (at your option) any later version. > + * > + * This file is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * Or, alternatively, > + * > + * b) Permission is hereby granted, free of charge, to any person > + * obtaining a copy of this software and associated documentation > + * files (the "Software"), to deal in the Software without > + * restriction, including without limitation the rights to use, > + * copy, modify, merge, publish, distribute, sublicense, and/or > + * sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following > + * conditions: > + * > + * The above copyright notice and this permission notice shall be > + * included in all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ > + > +/dts-v1/; > +#include "r7s9210.dtsi" > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/r7s9210-pinctrl.h> > + > +/ { > + model = "RZA2MEVB"; > + compatible = "renesas,rza2mevb", "renesas,r7s9210"; > + > + aliases { > + serial0 = &scif4; > + }; > + > + chosen { > + bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs"; > + stdout-path = "serial0:115200n8"; > + }; > + > + memory@80000000 { Wrong unit-address. > + device_type = "memory"; > + reg = <0x40000000 0x00800000>; /* HyperRAM */ > + }; > + > + lbsc { > + #address-cells = <1>; > + #size-cells = <1>; > + }; > + > + leds { > + status = "okay"; gpio-leds shouldn't need this as it shouldn't be in the SoC file. > + compatible = "gpio-leds"; > + > + led0 { > + gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>; > + }; > + }; > + > + /* Cramfs XIP File System in QSPI */ > + qspi@20000000 { > + compatible = "mtd-rom"; This should be the actual Quad-SPI controller and then a flash device underneath it. It may work as-is initially, but eventually won't you need the flash details? > + probe-type = "direct-mapped"; /* XIP from QSPI */ > + reg = <0x20000000 0x4000000>; /* 64 MB*/ > + bank-width = <4>; > + device-width = <1>; > + #address-cells = <1>; > + #size-cells = <1>; > + partition@800000 { Best practice is grouping under a 'partitions' node. > + label ="rootfs"; > + reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */ > + read-only; > + }; > + };
On Tuesday, December 11, 2018, Rob Herring wrote: > > + /* Cramfs XIP File System in QSPI */ > > + qspi@20000000 { > > > > + compatible = "mtd-rom"; > > This should be the actual Quad-SPI controller and then a flash device > underneath it. It may work as-is initially, but eventually won't you > need the flash details? Not really. The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees the QSPI flash as a linear read-only address range). Basically, u-boot sets up the QSPI to look like a ROM area, and that's it. Everything is running directly from that QSPI flash, so you can't make any modifications to it while running, otherwise the whole system will go down. Therefore, there is no need to know anything about the actually Flash that is connected, because you are never allowed to interact with it (or even query it on boot). This board in particular only has 8MB of RAM. So, it really only works well as an XIP system (meaning both kernel and file system are XIP from Flash). Chris
+Kumar who was just asking me about this exact problem. On Wed, Dec 12, 2018 at 7:59 AM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > On Tuesday, December 11, 2018, Rob Herring wrote: > > > + /* Cramfs XIP File System in QSPI */ > > > + qspi@20000000 { > > > > > > > + compatible = "mtd-rom"; > > > > This should be the actual Quad-SPI controller and then a flash device > > underneath it. It may work as-is initially, but eventually won't you > > need the flash details? > > Not really. > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees > the QSPI flash as a linear read-only address range). Yes, those controllers are becoming pretty standard. > Basically, u-boot sets up the QSPI to look like a ROM area, and that's > it. Everything is running directly from that QSPI flash, so you can't > make any modifications to it while running, otherwise the whole system will > go down. Therefore, there is no need to know anything about the actually > Flash that is connected, because you are never allowed to interact with it > (or even query it on boot). Okay, but these days u-boot uses DT itself. So how does u-boot know how to set things up? It's going to need to know what controller and flash details. Or how does one do a flash update (in the field)? I think it would be better to have a property to say the flash is already setup and just use it. Or maybe it can be done with compatibles: spi@1234 { compatible = "vendor,soc-quadspi", "mmio-spi"; flash@0 { compatible = "some-flash-part", "mtd-rom"; }; }; Now the problem is how to define the address range as SPI flash devices are already defined to use SPI chip-select numbers. The easiest option from a DT standpoint is just look at the parent reg property and figure out the memory range (typically there are 2 and you want the big one). Or you could have some simple driver that knows which reg range to get for specific compatibles. Rob
On Wednesday, December 12, 2018 1, Rob Herring wrote: > > Not really. > > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees > > the QSPI flash as a linear read-only address range). > > Yes, those controllers are becoming pretty standard. Ya, my SoCs are not so special any more :( But, XIP_KERNEL does not seem to be mainstream either. > > Therefore, there is no need to know anything about the actually > > Flash that is connected, because you are never allowed to interact with > it > > (or even query it on boot). > > Okay, but these days u-boot uses DT itself. So how does u-boot know > how to set things up? It's going to need to know what controller and > flash details. That's all hand coded at the moment. As these Quad-SPI chips came on the market, each vendor had their own way to get in (and out) of these modes, and even the protocols were not the same. Sometimes even the same flash vendor would do things different form chip to chip. At this point, (at least for Renesas) I have u-boot code that at least points you in the right direction depending on what flash vendor you go with....but I can't guarantee you won't need to do some tweaking. > Or how does one do a flash update (in the field)? Well....for that there's a "special" driver that I wrote. Basically, going back to the ways we've done it with MCU for years: Copy code to RAM, shut off interrupts, jump to RAM, do your business, jump back to flash. Again, tweaking is needed depending on what SPI flash you use, so there is no magic 'one driver fits all'. > I think it would be better to have a property to say the flash is > already setup and just use it. Or maybe it can be done with > compatibles: > > spi@1234 { > compatible = "vendor,soc-quadspi", "mmio-spi"; > flash@0 { > compatible = "some-flash-part", "mtd-rom"; > }; > }; > > Now the problem is how to define the address range as SPI flash > devices are already defined to use SPI chip-select numbers. The > easiest option from a DT standpoint is just look at the parent reg > property and figure out the memory range (typically there are 2 and > you want the big one). Or you could have some simple driver that knows > which reg range to get for specific compatibles. I'm still confused on why I would want to do this as I want the kernel to not know nothing about the underlying HW so it never tries to access it any way. At the end of the day, all I want is for the kernel to do an ioremap of this area for me so my file system (XIP cramfs in this case) can access it. "mtd-rom" does this for me today. Just FYI, early in boot, the kernel already mapped part of my QSPI into virtual memory since we're most likely running an XIP_KERNEL. While these XIP Linux systems seem to function quiet well, I might need to continue to hide some of the inner details from mainline because they don't always fit into the direction of the kernel systems (ie, monster quad core systems with lots of DDR memory). The other XIP file system that we use more often (AXFS, not in mainline) does not require the partitions in DT at all, so it's really only needed when using cramfs. So for now, maybe it would be better to just remove this confusing portion of the dts. Chris
On Wed, Dec 12, 2018 at 12:03 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > On Wednesday, December 12, 2018 1, Rob Herring wrote: > > > Not really. > > > The "QSPI" actually works as a memory mapped SPI (as in, the CPU sees > > > the QSPI flash as a linear read-only address range). > > > > Yes, those controllers are becoming pretty standard. > > Ya, my SoCs are not so special any more :( > > But, XIP_KERNEL does not seem to be mainstream either. Part of the problem there is XIP_KERNEL doesn't work with multi-platform. I think that is mainly due to the dependency on run-time phys2virt patching. We should fix that such that either we can set a default phys_offset or possibly we could do some offline patching. > > > Therefore, there is no need to know anything about the actually > > > Flash that is connected, because you are never allowed to interact with > > it > > > (or even query it on boot). > > > > Okay, but these days u-boot uses DT itself. So how does u-boot know > > how to set things up? It's going to need to know what controller and > > flash details. > > That's all hand coded at the moment. As these Quad-SPI chips came on the > market, each vendor had their own way to get in (and out) of these > modes, and even the protocols were not the same. Sometimes even the same > flash vendor would do things different form chip to chip. At this point, > (at least for Renesas) I have u-boot code that at least points you in the > right direction depending on what flash vendor you go with....but I > can't guarantee you won't need to do some tweaking. Seems like DT would help you there... > > Or how does one do a flash update (in the field)? > Well....for that there's a "special" driver that I wrote. Basically, > going back to the ways we've done it with MCU for years: Copy code to RAM, > shut off interrupts, jump to RAM, do your business, jump back to flash. > > Again, tweaking is needed depending on what SPI flash you use, so there > is no magic 'one driver fits all'. > > > > I think it would be better to have a property to say the flash is > > already setup and just use it. Or maybe it can be done with > > compatibles: > > > > spi@1234 { > > compatible = "vendor,soc-quadspi", "mmio-spi"; > > flash@0 { > > compatible = "some-flash-part", "mtd-rom"; > > }; > > }; > > > > Now the problem is how to define the address range as SPI flash > > devices are already defined to use SPI chip-select numbers. The > > easiest option from a DT standpoint is just look at the parent reg > > property and figure out the memory range (typically there are 2 and > > you want the big one). Or you could have some simple driver that knows > > which reg range to get for specific compatibles. > > > I'm still confused on why I would want to do this as I want the kernel > to not know nothing about the underlying HW so it never tries to access > it any way. > At the end of the day, all I want is for the kernel to do an ioremap of > this area for me so my file system (XIP cramfs in this case) can access > it. "mtd-rom" does this for me today. And partition layout? If it was only what to map, we could add a property in chosen or perhaps a reserved-memory node. But I doubt it will remain that simple. What happens when you have clock management and need to keep the SPI clock enabled for example. The other option is just have your platform code instantiate an mtd-rom device. Either describe the h/w with DT or don't use DT. > Just FYI, early in boot, the kernel already mapped part of my QSPI into > virtual memory since we're most likely running an XIP_KERNEL. > > While these XIP Linux systems seem to function quiet well, I might need > to continue to hide some of the inner details from mainline because they > don't always fit into the direction of the kernel systems (ie, monster > quad core systems with lots of DDR memory). > > The other XIP file system that we use more often (AXFS, not in mainline) > does not require the partitions in DT at all, so it's really only > needed when using cramfs. So for now, maybe it would be better to just remove > this confusing portion of the dts. What does AXFS have that cramfs doesn't making it not need partitions? Rob
On Monday, December 17, 2018 1, Rob Herring wrote: > > But, XIP_KERNEL does not seem to be mainstream either. > > Part of the problem there is XIP_KERNEL doesn't work with > multi-platform. I think that is mainly due to the dependency on > run-time phys2virt patching. We should fix that such that either we > can set a default phys_offset or possibly we could do some offline > patching. Not really. I fixed all of that back in 4.6. The issue is more of the 'theoretical' kind. The ARM maintainer defines "multiplatform" as being able to build 1 binary image that can be run on multiple SoCs. But...that won't work when you have to define the ROM/RAM area at compile time. I prefer to think of "multiplatform" as a way of grouping like SoCs together (if you need to) and use DT to sort out any differences. Basically, I just need this simple patch in order to build and boot so I can test kernel releases: [arch/arm/Kconfig] config XIP_KERNEL bool "Kernel Execute-In-Place from ROM" - depends on !ARM_LPAE && !ARCH_MULTIPLATFORM + depends on !ARM_LPAE However...I've given up on trying to get XIP_KERNEL enabled in Kconfig. But, of course still keep an eye on all the 'code' in the ARM core to make sure no one breaks it. > > At the end of the day, all I want is for the kernel to do an ioremap of > > this area for me so my file system (XIP cramfs in this case) can access > > it. "mtd-rom" does this for me today. > > And partition layout? If it was only what to map, we could add a > property in chosen or perhaps a reserved-memory node. But I doubt it > will remain that simple. What happens when you have clock management > and need to keep the SPI clock enabled for example. Yup...there's the killer! You'll notice that in mainline RZ/A devices, there is no QSPI driver, and also the QSPI clock is left out of the list of HW clocks. Otherwise, as you mentioned, that 'unused' clock would get shut off at the end of boot...and you're whole system would stop immediately. > The other option is just have your platform code instantiate an > mtd-rom device. Either describe the h/w with DT or don't use DT. You mean in a 'board' file? BTW, I still use board files for things that are too complicated for DT. > > The other XIP file system that we use more often (AXFS, not in mainline) > > does not require the partitions in DT at all, so it's really only > > needed when using cramfs. So for now, maybe it would be better to just > remove > > this confusing portion of the dts. > > What does AXFS have that cramfs doesn't making it not need partitions? For AXFS, you pass in the physical address to your rootfs on the kernel command line and then AXFS does the ioreapping itself inside the driver. When Nicolas Pitre started adding XIP mode to cramfs last year, he also started out that way. But, that was rejected (as it was also rejected when DAX was first started). The file system people said 'file system drives should not have the ability to map physical memory themselves'. Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within *each file* select what pages will be XIP, and what ones will be compressed. This saves a lot on Flash space when only your executable and const pages are left as uncompressed XIP pages. Chris
On Mon, Dec 17, 2018 at 10:22 AM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > On Monday, December 17, 2018 1, Rob Herring wrote: > > > But, XIP_KERNEL does not seem to be mainstream either. > > > > Part of the problem there is XIP_KERNEL doesn't work with > > multi-platform. I think that is mainly due to the dependency on > > run-time phys2virt patching. We should fix that such that either we > > can set a default phys_offset or possibly we could do some offline > > patching. > > Not really. I fixed all of that back in 4.6. > > The issue is more of the 'theoretical' kind. > > The ARM maintainer defines "multiplatform" as being able to build 1 > binary image that can be run on multiple SoCs. > But...that won't work when you have to define the ROM/RAM area at > compile time. > > I prefer to think of "multiplatform" as a way of grouping like SoCs > together (if you need to) and use DT to sort out any differences. Yes, I'm aware of all that. I did a bunch of the work to enable it. > Basically, I just need this simple patch in order to build and boot so I > can test kernel releases: > > [arch/arm/Kconfig] > > config XIP_KERNEL > bool "Kernel Execute-In-Place from ROM" > - depends on !ARM_LPAE && !ARCH_MULTIPLATFORM > + depends on !ARM_LPAE Okay, I guess it's not that hard to enable if one wants. Perhaps all > However...I've given up on trying to get XIP_KERNEL enabled in Kconfig. > But, of course still keep an eye on all the 'code' in the ARM core to > make sure no one breaks it. > > > At the end of the day, all I want is for the kernel to do an ioremap of > > > this area for me so my file system (XIP cramfs in this case) can access > > > it. "mtd-rom" does this for me today. > > > > And partition layout? If it was only what to map, we could add a > > property in chosen or perhaps a reserved-memory node. But I doubt it > > will remain that simple. What happens when you have clock management > > and need to keep the SPI clock enabled for example. > > Yup...there's the killer! > You'll notice that in mainline RZ/A devices, there is no QSPI driver, > and also the QSPI clock is left out of the list of HW clocks. Otherwise, > as you mentioned, that 'unused' clock would get shut off at the end of > boot...and you're whole system would stop immediately. If you had a simple driver with the binding as I proposed, then it could do the ioremap and clock management. And you could also have specific SPI and flash drivers for when you need to flashing without changing the DT. > > The other option is just have your platform code instantiate an > > mtd-rom device. Either describe the h/w with DT or don't use DT. > > You mean in a 'board' file? Yes. > BTW, I still use board files for things that are too complicated for DT. As you should. > > > The other XIP file system that we use more often (AXFS, not in mainline) > > > does not require the partitions in DT at all, so it's really only > > > needed when using cramfs. So for now, maybe it would be better to just > > remove > > > this confusing portion of the dts. > > > > What does AXFS have that cramfs doesn't making it not need partitions? > > For AXFS, you pass in the physical address to your rootfs on the kernel > command line and then AXFS does the ioreapping itself inside the driver. > > When Nicolas Pitre started adding XIP mode to cramfs last year, he also > started out that way. But, that was rejected (as it was also rejected > when DAX was first started). The file system people said 'file system > drives should not have the ability to map physical memory themselves'. Right, I remember that now. > Another thing AXFS can do that Cramfs can't do (yet), is AXFS can within > *each file* select what pages will be XIP, and what ones will be > compressed. This saves a lot on Flash space when only your executable and > const pages are left as uncompressed XIP pages. > > > Chris >
On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote: > > Yup...there's the killer! > > You'll notice that in mainline RZ/A devices, there is no QSPI driver, > > and also the QSPI clock is left out of the list of HW clocks. Otherwise, > > as you mentioned, that 'unused' clock would get shut off at the end of > > boot...and you're whole system would stop immediately. > > If you had a simple driver with the binding as I proposed, then it > could do the ioremap and clock management. And you could also have > specific SPI and flash drivers for when you need to flashing without > changing the DT. I agree. At the same time, this particular XIP-QSPI HW is now used in the latest Renesas R-Car devices, and there has been some talk of upstreaming the driver for it (as in, using it in SPI mode instead of XIP mode). I'm waiting to see where that ends up, and if I should just use that SPI mode driver for the RZ/A series but simply put in a new property like "use-xip-mode;" that basically tells the driver to do nothing except enable the clock (again) so it doesn't get shut off at the end of boot. However, with your idea, I could also add in the io-remapping into that driver as well. The only thing I can't remember is if the cramfs-xip driver really needed to have an mtd partition with 'probe-type = "direct-mapped";' underneath it so it could more closely map different regions during run-time. And in that case, the QSPI driver should not be doing an io-remapping because it's not its responsibility (and might screw things up). Chris
Hi Chris, On Mon, Dec 17, 2018 at 8:40 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Monday, December 17, 2018 1, linux-renesas-soc-owner@vger.kernel.org wrote: > > > Yup...there's the killer! > > > You'll notice that in mainline RZ/A devices, there is no QSPI driver, > > > and also the QSPI clock is left out of the list of HW clocks. Otherwise, > > > as you mentioned, that 'unused' clock would get shut off at the end of > > > boot...and you're whole system would stop immediately. > > > > If you had a simple driver with the binding as I proposed, then it > > could do the ioremap and clock management. And you could also have > > specific SPI and flash drivers for when you need to flashing without > > changing the DT. > > I agree. > > At the same time, this particular XIP-QSPI HW is now used in the latest > Renesas R-Car devices, and there has been some talk of upstreaming the > driver for it (as in, using it in SPI mode instead of XIP mode). > I'm waiting to see where that ends up, and if I should just use that > SPI mode driver for the RZ/A series but simply put in a new property like > "use-xip-mode;" that basically tells the driver to do nothing except enable > the clock (again) so it doesn't get shut off at the end of boot. The "clock management" should simply be calls to pm_runtime_enable() and pm_runtime_get_sync(). Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt index a94db59d8e2d..67a6810c4345 100644 --- a/Documentation/devicetree/bindings/arm/shmobile.txt +++ b/Documentation/devicetree/bindings/arm/shmobile.txt @@ -125,6 +125,8 @@ Boards: compatible = "renesas,porter", "renesas,r8a7791" - RSKRZA1 (YR0K77210C000BE) compatible = "renesas,rskrza1", "renesas,r7s72100" + - RZ/A2M Eval Board (RTK7921053S00000BE) + compatible = "renesas,rza2mevb", "renesas,r7s9210" - RZN1D-DB (RZ/N1D Demo Board for the RZ/N1D 400 pins package) compatible = "renesas,rzn1d400-db", "renesas,r9a06g032" - Salvator-X (RTP0RC7795SIPB0010S) diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index b0e966d625b9..fbdd068fd8af 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -824,6 +824,7 @@ dtb-$(CONFIG_ARCH_RENESAS) += \ r7s72100-genmai.dtb \ r7s72100-gr-peach.dtb \ r7s72100-rskrza1.dtb \ + r7s9210-rza2mevb.dtb \ r8a73a4-ape6evm.dtb \ r8a7740-armadillo800eva.dtb \ r8a7743-iwg20d-q7.dtb \ diff --git a/arch/arm/boot/dts/r7s9210-rza2mevb.dts b/arch/arm/boot/dts/r7s9210-rza2mevb.dts new file mode 100644 index 000000000000..9570aeb8f1ce --- /dev/null +++ b/arch/arm/boot/dts/r7s9210-rza2mevb.dts @@ -0,0 +1,133 @@ +/* + * Device Tree Source for the RZA2MEVB board + * + * Copyright (C) 2018 Renesas Electronics + * + * This file is dual-licensed: you can use it either under the terms + * of the GPL or the X11 license, at your option. Note that this dual + * licensing only applies to this file, and not this project as a + * whole. + * + * a) This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Or, alternatively, + * + * b) Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +/dts-v1/; +#include "r7s9210.dtsi" +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/pinctrl/r7s9210-pinctrl.h> + +/ { + model = "RZA2MEVB"; + compatible = "renesas,rza2mevb", "renesas,r7s9210"; + + aliases { + serial0 = &scif4; + }; + + chosen { + bootargs = "ignore_loglevel earlycon rootfstype=cramfs root=mtd:rootfs"; + stdout-path = "serial0:115200n8"; + }; + + memory@80000000 { + device_type = "memory"; + reg = <0x40000000 0x00800000>; /* HyperRAM */ + }; + + lbsc { + #address-cells = <1>; + #size-cells = <1>; + }; + + leds { + status = "okay"; + compatible = "gpio-leds"; + + led0 { + gpios = <&pinctrl RZA2_PIN(PORT6, 0) GPIO_ACTIVE_HIGH>; + }; + }; + + /* Cramfs XIP File System in QSPI */ + qspi@20000000 { + compatible = "mtd-rom"; + probe-type = "direct-mapped"; /* XIP from QSPI */ + reg = <0x20000000 0x4000000>; /* 64 MB*/ + bank-width = <4>; + device-width = <1>; + #address-cells = <1>; + #size-cells = <1>; + partition@800000 { + label ="rootfs"; + reg = <0x0800000 0x1000000>; /* 16MB @ 0x20800000 */ + read-only; + }; + }; +}; + +/* EXTAL */ +&extal_clk { + clock-frequency = <24000000>; /* 24MHz */ +}; + +/* RTC_X1 */ +&rtc_x1_clk { + clock-frequency = <32768>; +}; + +&pinctrl { + + /* Serial Console */ + scif4_pins: serial4 { + pinmux = <RZA2_PINMUX(PORT9, 0, 4)>, /* TxD4 */ + <RZA2_PINMUX(PORT9, 1, 4)>; /* RxD4 */ + }; +}; + +/* High resolution System tick timers */ +&ostm0 { + status = "okay"; +}; + +&ostm1 { + status = "okay"; +}; + +/* Serial Console */ +&scif4 { + pinctrl-names = "default"; + pinctrl-0 = <&scif4_pins>; + + status = "okay"; +};
Add support for Renesas RZ/A2M evaluation board. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- Documentation/devicetree/bindings/arm/shmobile.txt | 2 + arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/r7s9210-rza2mevb.dts | 133 +++++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 arch/arm/boot/dts/r7s9210-rza2mevb.dts