Re: [RFC PATCH v3 30/35] arch/sh/boot/dts: RTS7751R2D Plus DeviceTree.

Hi Sato-san,

On Sat, Oct 14, 2023 at 4:54 PM Yoshinori Sato
<[email protected]> wrote:
> Renesas RTS7751R2D Plus devicetree.
>
> Signed-off-by: Yoshinori Sato <[email protected]>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/sh/boot/dts/rts7751r2dplus.dts

> +       oscillator {
> +               clock-frequency = <22222222>;
> +       };

When adding or overriding properties to/in existing device nodes,
please refer to the nodes using symbolic labels.  I.e. please add

    &xtal {
                clock-frequency = <22222222>;
     };

at the bottom of this file instead.

> +       display@1,0 {
> +               compatible = "smi,sm501";
> +               reg = <0x10000000 0x03e00000
> +                      0x13e00000 0x00200000>;
> +               interrupt-parent = <&r2dintc>;
> +               interrupts = <4>;
> +               mode = "640x480-16@60";
> +               little-endian;
> +               smi,devices = "usb-host","uart0";
> +               interrupt-name = "sm501";
> +               route = "own";
> +               swap-fb-endian;
> +
> +               crt {
> +                       flags = "use_init_mode",
> +                               "use_hwcursor",
> +                               "use_hwaccel",
> +                               "disable_at_exit";

"make dtbs_check" does not like flags being non-integer.
Might be an artefact of having only plain text bindings in
Documentation/devicetree/bindings/display/sm501fb.txt

> +               };
> +
> +               panel {
> +                       bpp = <16>;
> +                       edid = [00 ff ff ff ff ff ff 00 00 00 00 00 00 00 00 00
> +                               00 00 01 04 00 00 00 00 00 00 00 00 00 00 00 00
> +                               00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                               00 00 00 00 00 00 f0 0a 80 fb 20 e0 25 10 32 60
> +                               02 00 00 00 00 00 00 06 00 00 00 00 00 00 00 00
> +                               00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                               00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +                               00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bd];
> +                       flags = "use_init_mode",
> +                               "use_hwcursor",
> +                               "use_hwaccel",
> +                               "disable_at_exit";

Likewise.

> +               };
> +       };
> +
> +       compact-flash@b4001000 {
> +               compatible = "renesas,rts7751r2d-ata", "ata-generic";
> +               reg = <0xb4001000 0x0e>, <0xb400080c 2>;
> +               reg-shift = <1>;
> +               interrupt-parent = <&r2dintc>;
> +               interrupts = <1>;
> +       };
> +
> +       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>;
> +               };
> +
> +               partition@2 {
> +                       label = "Kernel";
> +                       reg = <0x00080000 0x001c0000>;
> +               };
> +
> +               partition@3 {
> +                       label = "Flash_FS";
> +                       reg = <0x00240000 0x00dc0000>;
> +               };
> +       };
> +
> +       soc {
> +               clock-controller@ffc00000 {
> +                       renesas,mode = <5>;
> +               };

    &cpg {
                renesas,mode = <5>;
    };

At the bottom of this file.

> +
> +               pci@fe200000 {

Likewise, "&pcic { ... };" at the bottom of this file.

> +                       compatible = "renesas,pci-sh7751";

No need to override the compatible property, it is already set in
sh7751.dtsi.

> +                       interrupt-parent = <&r2dintc>;
> +                       renesas,bcr1 = <0x40080000>;
> +                       renesas,intm = <0x0000c3ff>;
> +                       renesas,aintm = <0x0000380f>;
> +                       renesas,config = <1 0xfb900047>, <4 0xab000001>;
> +                       renesas,mcrmask = <0x40000004>;
> +
> +                       interrupt-map = <0x0000 0 0 1 &r2dintc 9>,
> +                                       <0x0000 0 0 2 &r2dintc 10>,
> +                                       <0x0000 0 0 3 &r2dintc 3>,
> +                                       <0x0000 0 0 4 &r2dintc 0>,
> +                                       <0x0800 0 0 1 &r2dintc 10>,
> +                                       <0x0800 0 0 2 &r2dintc 3>,
> +                                       <0x0800 0 0 3 &r2dintc 0>,
> +                                       <0x0800 0 0 4 &r2dintc 9>,
> +                                       <0x1000 0 0 1 &r2dintc 3>,
> +                                       <0x1000 0 0 2 &r2dintc 0>,
> +                                       <0x1000 0 0 3 &r2dintc 9>,
> +                                       <0x1000 0 0 4 &r2dintc 10>;
> +                       interrupt-map-mask = <0x1800 0 0 7>;
> +               };
> +       };
> +};

Some of my comments apply to landisk.dts and usl-5p.dts, too.
And please run "make dtbs_check" ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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