Hello Krzysztof
On Mon, May 30, 2022 at 11:56 AM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 30/05/2022 15:08, Paweł Anikiel wrote:
> > Add devicetree for the Google Chameleon v3 board.
> >
> > Signed-off-by: Paweł Anikiel <pan@xxxxxxxxxxxx>
> > Signed-off-by: Alexandru M Stan <amstan@xxxxxxxxxxxx>
>
> 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)
