linux-kernel - Re: [PATCH 2/3] dts: socfpga: Add Google Chameleon v3 devicetree

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHNYxRw00QraVW0085xO-qzgGJdZ2joukuSYzBQo+yjLnkD=Tw@mail.gmail.com>
Date:   Mon, 30 May 2022 18:20:43 -0700
From:   Alexandru M Stan <amstan@...omium.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Paweł Anikiel <pan@...ihalf.com>, soc@...nel.org,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>, arnd@...db.de,
        Olof Johansson <olof@...om.net>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, dinguyen@...nel.org
Subject: Re: [PATCH 2/3] dts: socfpga: Add Google Chameleon v3 devicetree

Hello Krzysztof

On Mon, May 30, 2022 at 11:56 AM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 30/05/2022 15:08, Paweł Anikiel wrote:
> > Add devicetree for the Google Chameleon v3 board.
> >
> > Signed-off-by: Paweł Anikiel <pan@...ihalf.com>
> > Signed-off-by: Alexandru M Stan <amstan@...omium.org>
>
> Your SoB chain looks odd. Who did what here?

Sorry about this.

It was mainly Pawel but I did some small changes at some point before
it landed in our tree (particularly the GPIOs).

>
> > ---
> >  arch/arm/boot/dts/Makefile                    |  1 +
> >  .../boot/dts/socfpga_arria10_chameleonv3.dts  | 90 +++++++++++++++++++
> >  2 files changed, 91 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/socfpga_arria10_chameleonv3.dts
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 023c8b4ba45c..9417106d3289 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1146,6 +1146,7 @@ dtb-$(CONFIG_ARCH_S5PV210) += \
> >       s5pv210-torbreck.dtb
> >  dtb-$(CONFIG_ARCH_INTEL_SOCFPGA) += \
> >       socfpga_arria5_socdk.dtb \
> > +     socfpga_arria10_chameleonv3.dtb \
> >       socfpga_arria10_socdk_nand.dtb \
> >       socfpga_arria10_socdk_qspi.dtb \
> >       socfpga_arria10_socdk_sdmmc.dtb \
> > diff --git a/arch/arm/boot/dts/socfpga_arria10_chameleonv3.dts b/arch/arm/boot/dts/socfpga_arria10_chameleonv3.dts
> > new file mode 100644
> > index 000000000000..988cc445438e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/socfpga_arria10_chameleonv3.dts
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2022 Google LLC
> > + */
> > +/dts-v1/;
> > +#include "socfpga_arria10_mercury_aa1.dtsi"
> > +
> > +/ {
> > +     model = "Google Chameleon V3";
> > +     compatible = "google,chameleon-v3",
>
> You miss here enclustra compatible.

Does this make sense? I don't expect this device tree to boot/work on
an enclustra motherboard. It's only really compatible with a
"chameleon-v3".

>
> > +                  "altr,socfpga-arria10", "altr,socfpga";
> > +
> > +     aliases {
> > +             serial0 = &uart0;
> > +             i2c0 = &i2c0;
> > +             i2c1 = &i2c1;
> > +     };
> > +};
> > +
> > +&gmac0 {
> > +     status = "okay";
> > +};
> > +
> > +&gpio0 {
> > +     status = "okay";
> > +};
> > +
> > +&gpio1 {
> > +     status = "okay";
> > +};
> > +
> > +&gpio2 {
> > +     status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +     status = "okay";
> > +
> > +     ssm2603: ssm2603@1a {
>
> Generic node names.

Dumb question: what does this mean?

Are you saying the name is too generic? As someone reading the
schematics this would be immediately clear what chip it's talking
about.

>
> > +             compatible = "adi,ssm2603";
> > +             reg = <0x1a>;
> > +     };
> > +};
> > +
> > +&i2c1 {
> > +     status = "okay";
> > +
> > +     u80: u80@21 {
> > +             compatible = "nxp,pca9535";
>
> Generic node names.

FWIW: Schematic is full of these pca9535 io expanders, only one (U80)
is visible to linux on an I2C bus.

>
>
>
> Best regards,
> Krzysztof

Thanks,
Alexandru Stan (amstan)