| Message ID | d63793503fbbc7d5ca7b40d6b31678d371b69c29.1694596125.git.ysato@users.sourceforge.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Device Tree support for SH7751 based board | expand |
On 13/09/2023 11:23, Yoshinori Sato wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> No improvements in commit msg. No improvements in subject. > --- > arch/sh/boot/dts/rts7751r2dplus.dts | 124 ++++++++++++++++++++++++++++ > 1 file changed, 124 insertions(+) > create mode 100644 arch/sh/boot/dts/rts7751r2dplus.dts > > diff --git a/arch/sh/boot/dts/rts7751r2dplus.dts b/arch/sh/boot/dts/rts7751r2dplus.dts > new file mode 100644 > index 000000000000..a08061133841 > --- /dev/null > +++ b/arch/sh/boot/dts/rts7751r2dplus.dts > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the Renesas RTS7751R2D Plus > + */ > + > +/dts-v1/; > + > +#include "sh7751.dtsi" > + > +/ { > + model = "Renesas RTS7715R2D Plus"; > + compatible = "renesas,r2dplus"; Missing bindings documentation. Incomplete compatible - missing SoC. Sorry, but this is nowhere ready for review. There are so many trivial issues to fix. Best regards, Krzysztof
Hi Sato-san, On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- /dev/null > +++ b/arch/sh/boot/dts/rts7751r2dplus.dts > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the Renesas RTS7751R2D Plus > + */ > + > +/dts-v1/; > + > +#include "sh7751.dtsi" > + > +/ { > + model = "Renesas RTS7715R2D Plus"; > + compatible = "renesas,r2dplus"; > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <&shintc>; > + > + aliases { > + serial0 = &scif1; > + }; > + > + chosen { I had to add stdout-path = "serial0:115200n8"; to get serial console output on qemu/r2d. > + }; Gr{oetje,eeting}s, Geert
Hi Sato-san, On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- /dev/null > +++ b/arch/sh/boot/dts/rts7751r2dplus.dts > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Device Tree Source for the Renesas RTS7751R2D Plus > + */ > + > +/dts-v1/; > + > +#include "sh7751.dtsi" #include "sh7751r.dtsi"? To make that work, you can create "sh7751.dtsi" that includes "sh7751.dtsi" and overrides the parts that are different. > + > +/ { > + model = "Renesas RTS7715R2D Plus"; > + compatible = "renesas,r2dplus"; compatible = "renesas,r2dplus", "renesas,sh7751r", "renesas,sh7751". And all these compatible values must be documented in the DT binding documentation. > + #address-cells = <1>; > + #size-cells = <1>; > + interrupt-parent = <&shintc>; > + > + aliases { > + serial0 = &scif1; > + }; > + > + chosen { > + }; > + > + clocks { > + xtal: oscillator { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <22222222>; > + }; Please > + > + cpg: cpg@ffc00000 { "&cpg"? > + compatible = "renesas,sh7750r-cpg", > + "renesas,sh7750-cpg"; There should be no need to override the cpg's compatible value. > + renesas,mode = <5>; > + }; > + }; > + > + cpus { > + cpu@0 { > + clock-frequency = <266666666>; As the CPU clock is programmable, the "clock-frequency" property should probably be replaced by a "clocks" property in the base sh7751.dtsi. > + }; > + }; > + > + memory@c000000 { > + device_type = "memory"; > + reg = <0x0c000000 0x4000000>; > + }; > + > + r2dintc: sh7751irl_encoder@a4000000 { > + compatible = "renesas,sh7751-irl-ext"; > + reg = <0xa4000000 0x02>; > + interrupt-controller; > + #address-cells = <1>; > + #interrupt-cells = <2>; > + sh7751irl,width = <16>; > + sh7751irl,polarity = <0>; > + sh7751irl,irqbit =<11>, /* PCI INTD */ > + <9>, /* CF IDE */ > + <8>, /* CF CD */ > + <12>, /* PCI INTC */ > + <10>, /* SM501 */ > + <6>, /* KEY */ > + <5>, /* RTC ALARM */ > + <4>, /* RTC T */ > + <7>, /* SDCARD */ > + <14>, /* PCI INTA */ > + <13>, /* PCI INTB */ > + <0>, /* EXT */ > + <15>; /* TP */ > + }; > + > + display@1,0 { > + compatible = "smi,sm501"; > + reg = <0x10000000 0x03e00000 > + 0x13e00000 0x00200000>; > + interrupt-parent = <&r2dintc>; > + interrupts = <4 0>; > + mode = "640x480-16@60"; > + little-endian; > + sm501,devices = "usb-host","uart0"; > + }; > + > + compact-flash@b4001000 { > + compatible = "ata-generic"; compact-flash@b4001000: compatible:0: 'ata-generic' is not one of ['arm,vexpress-cf', 'fsl,mpc8349emitx-pata'] from schema $id: http://devicetree.org/schemas/ata/ata-generic.yaml# > + reg = <0xb4001000 0x0e>, <0xb400080c 2>; > + reg-shift = <1>; > + interrupt-parent = <&r2dintc>; > + interrupts = <1 0>; > + }; > + > + flash@0 { > + compatible = "cfi-flash"; > + reg = <0x00000000 0x02000000>; > + device-width = <2>; > + #address-cells = <1>; > + #size-cells = <1>; > + partition@0 { > + label = "U-Boot"; > + reg = <0x00000000 0x00040000>; > + }; > + partition@1 { > + label = "Environemt"; Environment > + reg = <0x00040000 0x00040000>; > + }; Several of the above comments apply to "[RFC PATCH v2 27/30] arch/sh: LANDISK DeviceTree." and "[RFC PATCH v2 28/30] arch/sh: USL-5P DeviceTree.", too. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, 19 Sep 2023 22:25:19 +0900, Geert Uytterhoeven wrote: > > Hi Sato-san, > > On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato > <ysato@users.sourceforge.jp> wrote: > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> > > Thanks for your patch! > > > --- /dev/null > > +++ b/arch/sh/boot/dts/rts7751r2dplus.dts > > @@ -0,0 +1,124 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Device Tree Source for the Renesas RTS7751R2D Plus > > + */ > > + > > +/dts-v1/; > > + > > +#include "sh7751.dtsi" > > #include "sh7751r.dtsi"? > > To make that work, you can create "sh7751.dtsi" that includes > "sh7751.dtsi" and overrides the parts that are different. The only difference between 7751 and 7751R is CPG, so I don't differentiate between them. Shall we write the CPG differences in sh7751r.dtsi? > > + > > +/ { > > + model = "Renesas RTS7715R2D Plus"; > > + compatible = "renesas,r2dplus"; > > compatible = "renesas,r2dplus", "renesas,sh7751r", "renesas,sh7751". > > And all these compatible values must be documented in the DT > binding documentation. > > > + #address-cells = <1>; > > + #size-cells = <1>; > > + interrupt-parent = <&shintc>; > > + > > + aliases { > > + serial0 = &scif1; > > + }; > > + > > + chosen { > > + }; > > + > > + clocks { > > + xtal: oscillator { > > + #clock-cells = <0>; > > + compatible = "fixed-clock"; > > + clock-frequency = <22222222>; > > + }; > > Please > > + > > + cpg: cpg@ffc00000 { > > "&cpg"? > > > + compatible = "renesas,sh7750r-cpg", > > + "renesas,sh7750-cpg"; > > There should be no need to override the cpg's compatible value. > > > + renesas,mode = <5>; > > + }; > > + }; > > + > > + cpus { > > + cpu@0 { > > + clock-frequency = <266666666>; > > As the CPU clock is programmable, the "clock-frequency" > property should probably be replaced by a "clocks" property in the > base sh7751.dtsi. > > > + }; > > + }; > > + > > + memory@c000000 { > > + device_type = "memory"; > > + reg = <0x0c000000 0x4000000>; > > + }; > > + > > + r2dintc: sh7751irl_encoder@a4000000 { > > + compatible = "renesas,sh7751-irl-ext"; > > + reg = <0xa4000000 0x02>; > > + interrupt-controller; > > + #address-cells = <1>; > > + #interrupt-cells = <2>; > > + sh7751irl,width = <16>; > > + sh7751irl,polarity = <0>; > > + sh7751irl,irqbit =<11>, /* PCI INTD */ > > + <9>, /* CF IDE */ > > + <8>, /* CF CD */ > > + <12>, /* PCI INTC */ > > + <10>, /* SM501 */ > > + <6>, /* KEY */ > > + <5>, /* RTC ALARM */ > > + <4>, /* RTC T */ > > + <7>, /* SDCARD */ > > + <14>, /* PCI INTA */ > > + <13>, /* PCI INTB */ > > + <0>, /* EXT */ > > + <15>; /* TP */ > > + }; > > + > > + display@1,0 { > > + compatible = "smi,sm501"; > > + reg = <0x10000000 0x03e00000 > > + 0x13e00000 0x00200000>; > > + interrupt-parent = <&r2dintc>; > > + interrupts = <4 0>; > > + mode = "640x480-16@60"; > > + little-endian; > > + sm501,devices = "usb-host","uart0"; > > + }; > > + > > + compact-flash@b4001000 { > > + compatible = "ata-generic"; > > compact-flash@b4001000: compatible:0: 'ata-generic' is not one of > ['arm,vexpress-cf', 'fsl,mpc8349emitx-pata'] > from schema $id: http://devicetree.org/schemas/ata/ata-generic.yaml# > > > + reg = <0xb4001000 0x0e>, <0xb400080c 2>; > > + reg-shift = <1>; > > + interrupt-parent = <&r2dintc>; > > + interrupts = <1 0>; > > + }; > > + > > + flash@0 { > > + compatible = "cfi-flash"; > > + reg = <0x00000000 0x02000000>; > > + device-width = <2>; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + partition@0 { > > + label = "U-Boot"; > > + reg = <0x00000000 0x00040000>; > > + }; > > + partition@1 { > > + label = "Environemt"; > > Environment > > > + reg = <0x00040000 0x00040000>; > > + }; > > Several of the above comments apply to "[RFC PATCH v2 27/30] > arch/sh: LANDISK DeviceTree." and "[RFC PATCH v2 28/30] arch/sh: > USL-5P DeviceTree.", too. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Sato-san, On Mon, Oct 2, 2023 at 3:22 PM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > On Tue, 19 Sep 2023 22:25:19 +0900, > Geert Uytterhoeven wrote: > > On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato > > <ysato@users.sourceforge.jp> wrote: > > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> > > > > Thanks for your patch! > > > > > --- /dev/null > > > +++ b/arch/sh/boot/dts/rts7751r2dplus.dts > > > @@ -0,0 +1,124 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Device Tree Source for the Renesas RTS7751R2D Plus > > > + */ > > > + > > > +/dts-v1/; > > > + > > > +#include "sh7751.dtsi" > > > > #include "sh7751r.dtsi"? > > > > To make that work, you can create "sh7751.dtsi" that includes > > "sh7751.dtsi" and overrides the parts that are different. > > The only difference between 7751 and 7751R is CPG, > so I don't differentiate between them. > > Shall we write the CPG differences in sh7751r.dtsi? Yes please. That way the .dts board file for a board with SH7751 can include sh7751.dtsi, and the .dts board file for a board with SH7751R can include sh7751r.dtsi. All common parts should be put in sh7751.dtsi, to avoid duplication. Gr{oetje,eeting}s, Geert
diff --git a/arch/sh/boot/dts/rts7751r2dplus.dts b/arch/sh/boot/dts/rts7751r2dplus.dts new file mode 100644 index 000000000000..a08061133841 --- /dev/null +++ b/arch/sh/boot/dts/rts7751r2dplus.dts @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree Source for the Renesas RTS7751R2D Plus + */ + +/dts-v1/; + +#include "sh7751.dtsi" + +/ { + model = "Renesas RTS7715R2D Plus"; + compatible = "renesas,r2dplus"; + #address-cells = <1>; + #size-cells = <1>; + interrupt-parent = <&shintc>; + + aliases { + serial0 = &scif1; + }; + + chosen { + }; + + clocks { + xtal: oscillator { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <22222222>; + }; + + cpg: cpg@ffc00000 { + compatible = "renesas,sh7750r-cpg", + "renesas,sh7750-cpg"; + renesas,mode = <5>; + }; + }; + + cpus { + cpu@0 { + clock-frequency = <266666666>; + }; + }; + + memory@c000000 { + device_type = "memory"; + reg = <0x0c000000 0x4000000>; + }; + + r2dintc: sh7751irl_encoder@a4000000 { + compatible = "renesas,sh7751-irl-ext"; + reg = <0xa4000000 0x02>; + interrupt-controller; + #address-cells = <1>; + #interrupt-cells = <2>; + sh7751irl,width = <16>; + sh7751irl,polarity = <0>; + sh7751irl,irqbit =<11>, /* PCI INTD */ + <9>, /* CF IDE */ + <8>, /* CF CD */ + <12>, /* PCI INTC */ + <10>, /* SM501 */ + <6>, /* KEY */ + <5>, /* RTC ALARM */ + <4>, /* RTC T */ + <7>, /* SDCARD */ + <14>, /* PCI INTA */ + <13>, /* PCI INTB */ + <0>, /* EXT */ + <15>; /* TP */ + }; + + display@1,0 { + compatible = "smi,sm501"; + reg = <0x10000000 0x03e00000 + 0x13e00000 0x00200000>; + interrupt-parent = <&r2dintc>; + interrupts = <4 0>; + mode = "640x480-16@60"; + little-endian; + sm501,devices = "usb-host","uart0"; + }; + + compact-flash@b4001000 { + compatible = "ata-generic"; + reg = <0xb4001000 0x0e>, <0xb400080c 2>; + reg-shift = <1>; + interrupt-parent = <&r2dintc>; + interrupts = <1 0>; + }; + + flash@0 { + compatible = "cfi-flash"; + reg = <0x00000000 0x02000000>; + device-width = <2>; + #address-cells = <1>; + #size-cells = <1>; + partition@0 { + label = "U-Boot"; + reg = <0x00000000 0x00040000>; + }; + partition@1 { + label = "Environemt"; + reg = <0x00040000 0x00040000>; + }; + partition@2 { + label = "Kernel"; + reg = <0x00080000 0x001c0000>; + }; + partition@3 { + label = "Flash_FS"; + reg = <0x00240000 0x00dc0000>; + }; + }; + + pci@fe200000 { + compatible = "renesas,r2d-pci", "renesas,sh7751-pci"; + #interrupt-cells = <1>; + interrupt-parent = <&r2dintc>; + eth@2,0 { + reg = <0x1000 0 0 0 0>; + interrupts = <3 0>; + }; + }; +};
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> --- arch/sh/boot/dts/rts7751r2dplus.dts | 124 ++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 arch/sh/boot/dts/rts7751r2dplus.dts