Hi Rob,
On Wed, Oct 27, 2021 at 2:37 PM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> > +always-y := $(dtb-y)
> > +subdir-y := $(dts-dirs)
> > +clean-files := *.dtb
>
> None of these lines should be needed.
Yes, these will be removed.
> > diff --git a/arch/arm64/boot/dts/pensando/elba-16core.dtsi b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> > new file mode 100644
> > index 000000000000..acf5941afbc1
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0
>
> Do you care about using with non-GPL OS? Dual license is preferred.
>
Dual is fine, changing to this:
SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > + psci {
>
> This goes at the root level.
>
Moving to the root level
> > + compatible = "arm,psci-0.2";
> > + method = "smc";
> > + };
> > +
> > + };
> > +};
> > diff --git a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> > +
> > +&spi0 {
> > + num-cs = <4>;
> > + cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
> > + <&porta 7 GPIO_ACTIVE_LOW>;
> > + status = "okay";
> > + spi0_cs0@0 {
>
> Node names should reflect the class of device and use standard name
> defined in the DT spec. This probably doesn't have one. 'lora' perhaps?
>
Right, I didn't see a standard name and found many approaches in other files
so I likely based off of one of these below. I searched the dts
files for 'lora' and
didn't find it. Is that an acronym? I can change it to what the preference is.
./microchip/sparx5_pcb134_board.dtsi:
&spi0 {
status = "okay";
spi@0 {
compatible = "spi-mux";
...
};
./rockchip/rk3399.dtsi:
spi5 {
spi5_clk: spi5-clk {
rockchip,pins =
<2 RK_PC6 2 &pcfg_pull_up>;
};
spi5_cs0: spi5-cs0 {
rockchip,pins =
<2 RK_PC7 2 &pcfg_pull_up>;
};
spi5_rx: spi5-rx {
rockchip,pins =
<2 RK_PC4 2 &pcfg_pull_up>;
};
spi5_tx: spi5-tx {
rockchip,pins =
<2 RK_PC5 2 &pcfg_pull_up>;
};
};
>
> > + compatible = "semtech,sx1301"; /* Enable spidev */
>
> What's spidev?
>
It's module drivers/spi/spidev.c which won't populate /dev/spidevB.C unless
there is a match which we need for the system to boot. An earlier patch added
to the compatible list below and the feedback on that was to remove it. Later I
noticed the compatible list expanded...
static const struct of_device_id spidev_dt_ids[] = {
{ .compatible = "rohm,dh2228fv" },
{ .compatible = "lineartechnology,ltc2488" },
{ .compatible = "semtech,sx1301" },
{ .compatible = "lwn,bk4" },
{ .compatible = "dh,dhcom-board" },
{ .compatible = "menlo,m53cpld" },
{ .compatible = "cisco,spi-petra" },
{ .compatible = "micron,spi-authenta" },
{},
};
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + spi-max-frequency = <12000000>;
> > + reg = <0>;
> > + };
> > +
> > + spi0_cs1@1 {
> > + compatible = "semtech,sx1301";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + spi-max-frequency = <12000000>;
> > + reg = <1>;
> > + };
> > +
> > + spi0_cs2@2 {
> > + compatible = "semtech,sx1301";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + spi-max-frequency = <12000000>;
> > + reg = <2>;
> > + interrupt-parent = <&porta>;
> > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > + };
> > +
> > + spi0_cs3@3 {
> > + compatible = "semtech,sx1301";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + spi-max-frequency = <12000000>;
> > + reg = <3>;
> > + };
> > +};
> > diff --git a/arch/arm64/boot/dts/pensando/elba-asic.dts b/arch/arm64/boot/dts/pensando/elba-asic.dts
> > new file mode 100644
> > index 000000000000..131931dc643f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > + model = "Elba ASIC Board";
> > + compatible = "pensando,elba";
>
> Normally we have a compatible for the board plus the soc compatible.
In this case there are currently five different boards/products that have no
variations needing a board level description.
Thanks,
Brad
