lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdXfc4EKKOLq1yhwiFaKE1=HT+_puuj6rJZ+OgnHv2cS8g@mail.gmail.com>
Date:   Wed, 15 Jul 2020 10:53:21 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Adam Ford <aford173@...il.com>
Cc:     Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Adam Ford-BE <aford@...conembedded.com>,
        Magnus Damm <magnus.damm@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 1/2] arm64: dts: Introduce r8a774a1-beacon-rzg2m-kit

Hi Adam,

On Tue, Jul 14, 2020 at 2:34 PM Adam Ford <aford173@...il.com> wrote:
> Beacon EmebeddedWorks, formerly Logic PD is introducing a new
> SOM and development kit based on the RZ/G2M SoC from Renesas.
>
> The SOM supports eMMC, WiFi and Bluetooth, along with a Cat-M1
> cellular radio.
>
> The Baseboard has Ethernet, USB, HDMI, stereo audio in and out,
> along with a variety of push buttons and LED's, and support for
> a parallel RGB and an LVDS display.
>
> Signed-off-by: Adam Ford <aford173@...il.com>
> ---
> V2:  Add support for RGB display, second backlight, fix clock references,
>      fix Makefile, remove unsupported versaclock features, and fix typos.

Thanks for the update!
Looks mostly OK to me, "make dtbs_check" found a few issues, though.

> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -5,6 +5,7 @@ dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-ex-idk-1110wr.dtb
>  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2.dtb
>  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2-ex.dtb
>  dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-hihope-rzg2m-rev2-ex-idk-1110wr.dtb
> +dtb-$(CONFIG_ARCH_R8A774A1) += r8a774a1-beacon-rzg2m-kit.dtb

Please preserve alphabetical sort order.

>
>  dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-hihope-rzg2n.dtb
>  dtb-$(CONFIG_ARCH_R8A774B1) += r8a774b1-hihope-rzg2n-ex.dtb

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi

> +       lvds {
> +               compatible = "panel-lvds";
> +               power-supply = <&reg_lcd_reset>;
> +               width-mm = <223>;
> +               height-mm = <125>;
> +               backlight = <&backlight_lvds>;
> +               data-mapping = "vesa-24";
> +
> +               panel-timing {
> +                       /* 800x480@...z */
> +                       clock-frequency = <30000000>;
> +                       hactive = <800>;
> +                       vactive = <480>;
> +                       hsync-len = <48>;
> +                       hfront-porch = <40>;
> +                       hback-porch = <40>;
> +                       vfront-porch = <13>;
> +                       vback-porch = <29>;
> +                       vsync-len = <3>;
> +                       hsync-active = <1>;
> +                       vsync-active = <3>;

"make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/panel-timing.yaml":
lvds: panel-timing:vsync-active:0:0: 3 is not one of [0, 1]

> +                       de-active = <1>;
> +                       pixelclk-active = <0>;
> +               };
> +
> +               port {
> +                       panel_in: endpoint {
> +                               remote-endpoint = <&lvds0_out>;
> +                       };
> +               };
> +       };
> +
> +       rgb {
> +               /* Different LCD with compatible timings */
> +               compatible = "rocktech,rk070er9427";
> +               backlight = <&backlight_rgb>;
> +               enable-gpios = <&gpio1 21 GPIO_ACTIVE_HIGH>;
> +               port {
> +                       rgb_panel: endpoint {
> +                               remote-endpoint = <&du_out_rgb>;
> +                       };
> +               };

Documentation/devicetree/bindings/display/panel/panel-simple.yaml
rgb: 'power-supply' is a required property

> +       };

> +       versaclock6_bb: versaclock@6a {

As per DT generic node name rules, this should be called
"clock-controller@6a".  I'm aware that would cause issues with the
current driver implementation, so I'll accept this for now.  But it
would be good to resolve this later.

> +               compatible = "idt,5p49v6965";
> +               reg = <0x6a>;
> +               #clock-cells = <1>;
> +               clocks = <&x304_clk>;
> +               clock-names = "xin";
> +               /* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */
> +               assigned-clocks = <&versaclock6_bb 1>,
> +                                  <&versaclock6_bb 2>,
> +                                  <&versaclock6_bb 3>,
> +                                  <&versaclock6_bb 4>;
> +               assigned-clock-rates =  <24000000>, <24000000>, <24000000>, <24576000>;
> +       };
> +};

> +       hd3ss3220@47 {
> +               compatible = "ti,hd3ss3220";
> +               reg = <0x47>;
> +               interrupt-parent = <&gpio6>;
> +               interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> +
> +               connector {
> +                       compatible = "usb-c-connector";
> +                       label = "USB-C";
> +                       data-role = "dual";
> +
> +                       ports {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               port@1 {
> +                                       reg = <1>;
> +                                       hd3ss3220_ep: endpoint {
> +                                               remote-endpoint = <&usb3_role_switch>;
> +                                       };
> +                               };

Documentation/devicetree/bindings/connector/usb-connector.yaml
connector: ports: 'port@0' is a required property

For now, I'd ignore that, as it's a contradiction between the usb-connector
and the hd3ss3220 bindings.

> +                       };
> +               };
> +       };
> +};

> index 000000000000..97272f5fa0ab
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-som.dtsi

> +       versaclock5: versaclock_som@6a {

Same comment: "clock-controller@6a" (ignored for now).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ