diff mbox series

[v5,12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS

Message ID 20200318001603.9650-13-olteanv@gmail.com (mailing list archive)
State New, archived
Headers show
Series NXP DSPI bugfixes and support for LS1028A | expand

Commit Message

Vladimir Oltean March 18, 2020, 12:16 a.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

For debugging, it is useful to have access to the DSPI controller
signals. On the reference design board, these are exported to either the
mikroBUS1 or mikroBUS2 connector (according to the CPLD register
BRDCFG3[SPI3]).

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v5:
None.

Changes in v4:
None.

Changes in v3:
None.

Changes in v2:
Change compatible string for spidev node.

 arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Shawn Guo April 20, 2020, 2:38 p.m. UTC | #1 
On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> For debugging, it is useful to have access to the DSPI controller
> signals. On the reference design board, these are exported to either the
> mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> BRDCFG3[SPI3]).
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> Changes in v5:
> None.
> 
> Changes in v4:
> None.
> 
> Changes in v3:
> None.
> 
> Changes in v2:
> Change compatible string for spidev node.
> 
>  arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> index 6d05b76c2c7a..0d27b5667b8c 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -83,6 +83,20 @@
>  	};
>  };
>  
> +&dspi2 {
> +	bus-num = <2>;
> +	status = "okay";
> +
> +	/* mikroBUS1 */
> +	spidev@0 {
> +		compatible = "rohm,dh2228fv";

Is the compatible documented?

Shawn

> +		spi-max-frequency = <20000000>;
> +		fsl,spi-cs-sck-delay = <100>;
> +		fsl,spi-sck-cs-delay = <100>;
> +		reg = <0>;
> +	};
> +};
> +
>  &esdhc {
>  	sd-uhs-sdr104;
>  	sd-uhs-sdr50;
> -- 
> 2.17.1
>
Vladimir Oltean April 20, 2020, 3:10 p.m. UTC | #2 
On Mon, 20 Apr 2020 at 17:38, Shawn Guo <shawnguo@kernel.org> wrote:
>
> On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > For debugging, it is useful to have access to the DSPI controller
> > signals. On the reference design board, these are exported to either the
> > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > BRDCFG3[SPI3]).
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > Changes in v5:
> > None.
> >
> > Changes in v4:
> > None.
> >
> > Changes in v3:
> > None.
> >
> > Changes in v2:
> > Change compatible string for spidev node.
> >
> >  arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > index 6d05b76c2c7a..0d27b5667b8c 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > @@ -83,6 +83,20 @@
> >       };
> >  };
> >
> > +&dspi2 {
> > +     bus-num = <2>;
> > +     status = "okay";
> > +
> > +     /* mikroBUS1 */
> > +     spidev@0 {
> > +             compatible = "rohm,dh2228fv";
>
> Is the compatible documented?
>
> Shawn
>
> > +             spi-max-frequency = <20000000>;
> > +             fsl,spi-cs-sck-delay = <100>;
> > +             fsl,spi-sck-cs-delay = <100>;
> > +             reg = <0>;
> > +     };
> > +};
> > +
> >  &esdhc {
> >       sd-uhs-sdr104;
> >       sd-uhs-sdr50;
> > --
> > 2.17.1
> >

I don't really know what's the status with spidev compatibles. I do
see other device trees are using this one, I thought it should be
fine.

Thanks,
-Vladimir
Geert Uytterhoeven April 20, 2020, 6:06 p.m. UTC | #3 
Hi Vladimir,

On Wed, Mar 18, 2020 at 1:17 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> For debugging, it is useful to have access to the DSPI controller
> signals. On the reference design board, these are exported to either the
> mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> BRDCFG3[SPI3]).
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -83,6 +83,20 @@
>         };
>  };
>
> +&dspi2 {
> +       bus-num = <2>;
> +       status = "okay";
> +
> +       /* mikroBUS1 */
> +       spidev@0 {

Please use generic node names, e.g. "dac@0".

> +               compatible = "rohm,dh2228fv";
> +               spi-max-frequency = <20000000>;
> +               fsl,spi-cs-sck-delay = <100>;
> +               fsl,spi-sck-cs-delay = <100>;
> +               reg = <0>;
> +       };
> +};
> +
>  &esdhc {
>         sd-uhs-sdr104;
>         sd-uhs-sdr50;

Gr{oetje,eeting}s,

                        Geert
Vladimir Oltean April 20, 2020, 6:10 p.m. UTC | #4 
Hi Geert,

On Mon, 20 Apr 2020 at 21:07, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Vladimir,
>
> On Wed, Mar 18, 2020 at 1:17 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > For debugging, it is useful to have access to the DSPI controller
> > signals. On the reference design board, these are exported to either the
> > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > BRDCFG3[SPI3]).
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > @@ -83,6 +83,20 @@
> >         };
> >  };
> >
> > +&dspi2 {
> > +       bus-num = <2>;
> > +       status = "okay";
> > +
> > +       /* mikroBUS1 */
> > +       spidev@0 {
>
> Please use generic node names, e.g. "dac@0".
>

It's not a DAC. It's really an unpopulated pin header. I would have
really liked to have access to that as a char device with the default
board DTS, via spidev. That being said, there are warnings to not use
the "spidev" compatible in device trees. So if what I want is not
possible, I'd rather drop the patch altogether.

> > +               compatible = "rohm,dh2228fv";
> > +               spi-max-frequency = <20000000>;
> > +               fsl,spi-cs-sck-delay = <100>;
> > +               fsl,spi-sck-cs-delay = <100>;
> > +               reg = <0>;
> > +       };
> > +};
> > +
> >  &esdhc {
> >         sd-uhs-sdr104;
> >         sd-uhs-sdr50;
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

Thanks,
-Vladimir
Shawn Guo April 21, 2020, 3 a.m. UTC | #5 
On Mon, Apr 20, 2020 at 06:10:35PM +0300, Vladimir Oltean wrote:
> On Mon, 20 Apr 2020 at 17:38, Shawn Guo <shawnguo@kernel.org> wrote:
> >
> > On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > For debugging, it is useful to have access to the DSPI controller
> > > signals. On the reference design board, these are exported to either the
> > > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > > BRDCFG3[SPI3]).
> > >
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > Changes in v5:
> > > None.
> > >
> > > Changes in v4:
> > > None.
> > >
> > > Changes in v3:
> > > None.
> > >
> > > Changes in v2:
> > > Change compatible string for spidev node.
> > >
> > >  arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > index 6d05b76c2c7a..0d27b5667b8c 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > @@ -83,6 +83,20 @@
> > >       };
> > >  };
> > >
> > > +&dspi2 {
> > > +     bus-num = <2>;
> > > +     status = "okay";
> > > +
> > > +     /* mikroBUS1 */
> > > +     spidev@0 {
> > > +             compatible = "rohm,dh2228fv";
> >
> > Is the compatible documented?
> >
> > Shawn
> >
> > > +             spi-max-frequency = <20000000>;
> > > +             fsl,spi-cs-sck-delay = <100>;
> > > +             fsl,spi-sck-cs-delay = <100>;
> > > +             reg = <0>;
> > > +     };
> > > +};
> > > +
> > >  &esdhc {
> > >       sd-uhs-sdr104;
> > >       sd-uhs-sdr50;
> > > --
> > > 2.17.1
> > >
> 
> I don't really know what's the status with spidev compatibles. I do
> see other device trees are using this one, I thought it should be
> fine.

My understanding is that every compatible needs to be documented.

Shawn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 6d05b76c2c7a..0d27b5667b8c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -83,6 +83,20 @@ 
 	};
 };
 
+&dspi2 {
+	bus-num = <2>;
+	status = "okay";
+
+	/* mikroBUS1 */
+	spidev@0 {
+		compatible = "rohm,dh2228fv";
+		spi-max-frequency = <20000000>;
+		fsl,spi-cs-sck-delay = <100>;
+		fsl,spi-sck-cs-delay = <100>;
+		reg = <0>;
+	};
+};
+
 &esdhc {
 	sd-uhs-sdr104;
 	sd-uhs-sdr50;