linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm: dts: imx7: add QSPI
@ 2020-07-28 11:28 Matthias Schiffer
  2020-07-28 13:51 ` Marco Felsch
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Schiffer @ 2020-07-28 11:28 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer
  Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, linux-kernel, Matthias Schiffer

In preparation for an update of the TQ-Systems TQMa7x/MBa7x DTS, add the
QSPI controller to imx7s.dtsi.

Based-on-patch-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---
 arch/arm/boot/dts/imx7s.dtsi | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 1cfaf410aa43..e45683e61593 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1162,6 +1162,19 @@
 				status = "disabled";
 			};
 
+			qspi1: spi@30bb0000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx7d-qspi";
+				reg = <0x30bb0000 0x10000>, <0x60000000 0x10000000>;
+				reg-names = "QuadSPI", "QuadSPI-memory";
+				interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&clks IMX7D_QSPI_ROOT_CLK>,
+					<&clks IMX7D_QSPI_ROOT_CLK>;
+				clock-names = "qspi_en", "qspi";
+				status = "disabled";
+			};
+
 			sdma: sdma@30bd0000 {
 				compatible = "fsl,imx7d-sdma", "fsl,imx35-sdma";
 				reg = <0x30bd0000 0x10000>;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] arm: dts: imx7: add QSPI
  2020-07-28 11:28 [PATCH] arm: dts: imx7: add QSPI Matthias Schiffer
@ 2020-07-28 13:51 ` Marco Felsch
  2020-07-28 14:05   ` (EXT) " Matthias Schiffer
  0 siblings, 1 reply; 4+ messages in thread
From: Marco Felsch @ 2020-07-28 13:51 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, linux-kernel, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel

Hi Matthias,

thanks for the patch.

On 20-07-28 13:28, Matthias Schiffer wrote:
> In preparation for an update of the TQ-Systems TQMa7x/MBa7x DTS, add the
> QSPI controller to imx7s.dtsi.
> 
> Based-on-patch-by: Han Xu <han.xu@nxp.com>
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
>  arch/arm/boot/dts/imx7s.dtsi | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 1cfaf410aa43..e45683e61593 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -1162,6 +1162,19 @@
>  				status = "disabled";
>  			};
>  
> +			qspi1: spi@30bb0000 {

Are there more controllers and why not using "qspi@30bb0000" as node
name?

> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx7d-qspi";
> +				reg = <0x30bb0000 0x10000>, <0x60000000 0x10000000>;
> +				reg-names = "QuadSPI", "QuadSPI-memory";

The node should begin with compatible, reg, reg-names properties.
Pls check the current .dtsi file for examples.

> +				interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clks IMX7D_QSPI_ROOT_CLK>,
> +					<&clks IMX7D_QSPI_ROOT_CLK>;
> +				clock-names = "qspi_en", "qspi";
> +				status = "disabled";
> +			};
> +
>  			sdma: sdma@30bd0000 {
>  				compatible = "fsl,imx7d-sdma", "fsl,imx35-sdma";
>  				reg = <0x30bd0000 0x10000>;

Regards,
  Marco

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: (EXT) Re: [PATCH] arm: dts: imx7: add QSPI
  2020-07-28 13:51 ` Marco Felsch
@ 2020-07-28 14:05   ` Matthias Schiffer
  2020-07-28 14:31     ` Marco Felsch
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Schiffer @ 2020-07-28 14:05 UTC (permalink / raw)
  To: Marco Felsch
  Cc: Shawn Guo, Sascha Hauer, linux-kernel, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel

On Tue, 2020-07-28 at 15:51 +0200, Marco Felsch wrote:
> Hi Matthias,
> 
> thanks for the patch.
> 
> On 20-07-28 13:28, Matthias Schiffer wrote:
> > In preparation for an update of the TQ-Systems TQMa7x/MBa7x DTS,
> > add the
> > QSPI controller to imx7s.dtsi.
> > 
> > Based-on-patch-by: Han Xu <han.xu@nxp.com>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
> > >
> > ---
> >  arch/arm/boot/dts/imx7s.dtsi | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx7s.dtsi
> > b/arch/arm/boot/dts/imx7s.dtsi
> > index 1cfaf410aa43..e45683e61593 100644
> > --- a/arch/arm/boot/dts/imx7s.dtsi
> > +++ b/arch/arm/boot/dts/imx7s.dtsi
> > @@ -1162,6 +1162,19 @@
> >  				status = "disabled";
> >  			};
> >  
> > +			qspi1: spi@30bb0000 {
> 
> Are there more controllers and why not using "qspi@30bb0000" as node
> name?

The vast majority of QSPI controllers use spi@ node names, qspi@ only
appears in a single example in Documentation/devicetree/bindings/, and
in no actual DTS(I) files.

There is only one controller. The label "qspi1" is chosen as this has
been in use in the linux-imx vendor kernels for years; IMHO, switching
to "qspi" would just cause unnecessary churn for dependent device
trees. I have no strong opinions on this though.

> 
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +				compatible = "fsl,imx7d-qspi";
> > +				reg = <0x30bb0000 0x10000>, <0x60000000
> > 0x10000000>;
> > +				reg-names = "QuadSPI", "QuadSPI-
> > memory";
> 
> The node should begin with compatible, reg, reg-names properties.
> Pls check the current .dtsi file for examples.

Thanks, will fix.

> 
> > +				interrupts = <GIC_SPI 107
> > IRQ_TYPE_LEVEL_HIGH>;
> > +				clocks = <&clks IMX7D_QSPI_ROOT_CLK>,
> > +					<&clks IMX7D_QSPI_ROOT_CLK>;
> > +				clock-names = "qspi_en", "qspi";
> > +				status = "disabled";
> > +			};
> > +
> >  			sdma: sdma@30bd0000 {
> >  				compatible = "fsl,imx7d-sdma",
> > "fsl,imx35-sdma";
> >  				reg = <0x30bd0000 0x10000>;
> 
> Regards,
>   Marco


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: (EXT) Re: [PATCH] arm: dts: imx7: add QSPI
  2020-07-28 14:05   ` (EXT) " Matthias Schiffer
@ 2020-07-28 14:31     ` Marco Felsch
  0 siblings, 0 replies; 4+ messages in thread
From: Marco Felsch @ 2020-07-28 14:31 UTC (permalink / raw)
  To: Matthias Schiffer
  Cc: Shawn Guo, Sascha Hauer, linux-kernel, NXP Linux Team,
	Pengutronix Kernel Team, Fabio Estevam, linux-arm-kernel

On 20-07-28 16:05, Matthias Schiffer wrote:
> On Tue, 2020-07-28 at 15:51 +0200, Marco Felsch wrote:
> > Hi Matthias,
> > 
> > thanks for the patch.
> > 
> > On 20-07-28 13:28, Matthias Schiffer wrote:
> > > In preparation for an update of the TQ-Systems TQMa7x/MBa7x DTS,
> > > add the
> > > QSPI controller to imx7s.dtsi.
> > > 
> > > Based-on-patch-by: Han Xu <han.xu@nxp.com>
> > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com
> > > >
> > > ---
> > >  arch/arm/boot/dts/imx7s.dtsi | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/imx7s.dtsi
> > > b/arch/arm/boot/dts/imx7s.dtsi
> > > index 1cfaf410aa43..e45683e61593 100644
> > > --- a/arch/arm/boot/dts/imx7s.dtsi
> > > +++ b/arch/arm/boot/dts/imx7s.dtsi
> > > @@ -1162,6 +1162,19 @@
> > >  				status = "disabled";
> > >  			};
> > >  
> > > +			qspi1: spi@30bb0000 {
> > 
> > Are there more controllers and why not using "qspi@30bb0000" as node
> > name?
> 
> The vast majority of QSPI controllers use spi@ node names, qspi@ only
> appears in a single example in Documentation/devicetree/bindings/, and
> in no actual DTS(I) files.

IMHO using spi as node name is incorrect because this inherits the
assumption to connect 'normal' spi devices to these controllers. But
this is absolutly not the case since in most cases the qspi controllers
are state-machines optimized for qspi memory devices. So all dts(i)
files using spi as node name are incorrect IMHO. But we can't change
that due to the backward compability.

> There is only one controller. The label "qspi1" is chosen as this has
> been in use in the linux-imx vendor kernels for years; IMHO, switching
> to "qspi" would just cause unnecessary churn for dependent device
> trees. I have no strong opinions on this though.

Why? There is no _mainline_ imx7s devicetree using this node currently.
I would drop the number since we have only one instance.

Regards,
  Marco


> > > +				#address-cells = <1>;
> > > +				#size-cells = <0>;
> > > +				compatible = "fsl,imx7d-qspi";
> > > +				reg = <0x30bb0000 0x10000>, <0x60000000
> > > 0x10000000>;
> > > +				reg-names = "QuadSPI", "QuadSPI-
> > > memory";
> > 
> > The node should begin with compatible, reg, reg-names properties.
> > Pls check the current .dtsi file for examples.
> 
> Thanks, will fix.
> 
> > 
> > > +				interrupts = <GIC_SPI 107
> > > IRQ_TYPE_LEVEL_HIGH>;
> > > +				clocks = <&clks IMX7D_QSPI_ROOT_CLK>,
> > > +					<&clks IMX7D_QSPI_ROOT_CLK>;
> > > +				clock-names = "qspi_en", "qspi";
> > > +				status = "disabled";
> > > +			};
> > > +
> > >  			sdma: sdma@30bd0000 {
> > >  				compatible = "fsl,imx7d-sdma",
> > > "fsl,imx35-sdma";
> > >  				reg = <0x30bd0000 0x10000>;
> > 
> > Regards,
> >   Marco
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-28 14:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 11:28 [PATCH] arm: dts: imx7: add QSPI Matthias Schiffer
2020-07-28 13:51 ` Marco Felsch
2020-07-28 14:05   ` (EXT) " Matthias Schiffer
2020-07-28 14:31     ` Marco Felsch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).