Re: [PATCH] arm64: dts: imx8mq: Add QuadSPI controller

From: Lucas Stach <l.stach@pengutronix.de>
To: Carlo Caione <ccaione@baylibre.com>,
	robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de,
	festevam@gmail.com, linux-imx@nxp.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: dts: imx8mq: Add QuadSPI controller
Date: Tue, 29 Jan 2019 18:40:52 +0100	[thread overview]
Message-ID: <1548783652.6869.11.camel@pengutronix.de> (raw)
In-Reply-To: <3ff2f0d6-07d2-9ed4-878f-011406f09da3@baylibre.com>

Am Dienstag, den 29.01.2019, 16:54 +0000 schrieb Carlo Caione:
> On 29/01/2019 16:42, Lucas Stach wrote:
> > Hi Carlo,
> 
> Hi Lucas,
> 
> > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > @@ -516,6 +516,19 @@
> > > > > > > >   			};
> > > >   		};
> > > 
> > >   
> > > +		spi0: spi@30bb0000 {
> > 
> > 30bb0000 is part of the AIPS3 bus address space, so please move this to
> > the correct location within this bus node.
> 
> The problem is that the "QuadSPI-memory" region doesn't fall within the 
> memory range of the AIPS3 bus, so the devm_ioremap_resource is failing 
> when moving the node there.

Uh, that's interesting. Normally the DTs are organized along the
control path and I guess the QuadSPI-memory is not really a control
path, but the fast memory access path. Maybe this is something the DT
folks could take a look at.

But then I would still prefer to have the QSPI controller moved into
the correct control bus. I guess we can work around your
devm_ioremap_resource failing issue by adding the QSPI-memory region to
the AIPS3 bus ranges.

> > > +			#address-cells = <1>;
> > > > +			#size-cells = <0>;
> > > 
> > > +			compatible = "fsl,imx7d-qspi";
> > 
> > Please add a "fsl,imx8mq-qspi" compatible here, as was done with all
> > the other nodes in this file, so we can match this in the driver should
> > the need arise.
> 
> This is odd since at least for the AmLogic SoCs we are going exactly in 
> the opposite direction where we avoid to add unnecessary compatibles if 
> that's not strictly required.

It's a safety net, so we don't need to change existing DTBs if it turns
out that a specific SoC integration has a bug that needs a workaround
in the driver.

This is one of things I've proposed in my ELC-E talk "Stable Devicetree
ABI: it's possible!", which was generally well received by the DT
folks.

Regards,
Lucas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel