| Message ID | 20200728112814.14158-1-matthias.schiffer@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm: dts: imx7: add QSPI | expand |
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
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
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 > >
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>;
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(+)