| Message ID | 1527686082-15142-6-git-send-email-frieder.schrempf@exceet.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | Port the FSL QSPI driver to the SPI framework | expand |
On Wed, 30 May 2018 15:14:34 +0200 Frieder Schrempf <frieder.schrempf@exceet.de> wrote: > The FSL QSPI driver was moved to the SPI framework and it now > acts as a SPI controller. Therefore the subnodes need to set > spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before. We should try to keep the current behavior even when spi-[rx/tx]-bus-width are not defined. How about considering spi-[rx/tx]-bus-width as board constraints and then let the core pick the best mode based on these constraints plus the SPI NOR chip limitations. > > Also the properties 'bus-num', 'fsl,spi-num-chipselects' and > 'fsl,spi-flash-chipselects' were never read by the driver and > can be removed. > > The 'reg' properties are adjusted to reflect the what bus and > chipselect the flash is connected to, as the new driver needs > this information. > > The property 'fsl,qspi-has-second-chip' is not needed anymore > and will be removed after the old driver was disabled to avoid > breaking ls1021a-moxa-uc-8410a.dts. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de> > --- > arch/arm/boot/dts/imx6sx-sdb-reva.dts | 8 ++++++-- > arch/arm/boot/dts/imx6sx-sdb.dts | 8 ++++++-- > arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 2 ++ > arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 5 ++--- > 4 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts > index e3533e7..1a6f680 100644 > --- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts > +++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts > @@ -131,13 +131,17 @@ > #size-cells = <1>; > compatible = "spansion,s25fl128s", "jedec,spi-nor"; > spi-max-frequency = <66000000>; > + spi-rx-bus-width = <4>; > + spi-tx-bus-width = <4>; > }; > > - flash1: s25fl128s@1 { > - reg = <1>; > + flash1: s25fl128s@2 { > + reg = <2>; Hm, you're breaking backward compat here. Can we try to re-use the old numbering scheme instead of patching all DTs?
Hi Boris, On 30.05.2018 17:10, Boris Brezillon wrote: > On Wed, 30 May 2018 15:14:34 +0200 > Frieder Schrempf <frieder.schrempf@exceet.de> wrote: > >> The FSL QSPI driver was moved to the SPI framework and it now >> acts as a SPI controller. Therefore the subnodes need to set >> spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before. > > We should try to keep the current behavior even when > spi-[rx/tx]-bus-width are not defined. How about considering > spi-[rx/tx]-bus-width as board constraints and then let the core pick > the best mode based on these constraints plus the SPI NOR chip > limitations. Ok, I'll try to adjust this, so we can leave spi-[rx/tx]-bus-width undefined and still get quad mode as default if possible. > >> >> Also the properties 'bus-num', 'fsl,spi-num-chipselects' and >> 'fsl,spi-flash-chipselects' were never read by the driver and >> can be removed. >> >> The 'reg' properties are adjusted to reflect the what bus and >> chipselect the flash is connected to, as the new driver needs >> this information. >> >> The property 'fsl,qspi-has-second-chip' is not needed anymore >> and will be removed after the old driver was disabled to avoid >> breaking ls1021a-moxa-uc-8410a.dts. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de> >> --- >> arch/arm/boot/dts/imx6sx-sdb-reva.dts | 8 ++++++-- >> arch/arm/boot/dts/imx6sx-sdb.dts | 8 ++++++-- >> arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 2 ++ >> arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 5 ++--- >> 4 files changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts >> index e3533e7..1a6f680 100644 >> --- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts >> +++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts >> @@ -131,13 +131,17 @@ >> #size-cells = <1>; >> compatible = "spansion,s25fl128s", "jedec,spi-nor"; >> spi-max-frequency = <66000000>; >> + spi-rx-bus-width = <4>; >> + spi-tx-bus-width = <4>; >> }; >> >> - flash1: s25fl128s@1 { >> - reg = <1>; >> + flash1: s25fl128s@2 { >> + reg = <2>; > > Hm, you're breaking backward compat here. Can we try to re-use the > old numbering scheme instead of patching all DTs? Unfortunately in the current setup, the definitions for the reg property are already broken. For example imx6sx-sdb.dts seems to have one chip connected on bus A, CS0 and one on bus B, CS0. It has reg set to 0 for the first and 1 for the second chip. While fsl-ls208xa-qds.dtsi uses the same hw setup, but has reg set to 0 and 2. So either way we need to change the reg property at some place. So the best approach in my opinion is to fix the definitions to use a single scheme and while at it also remove the fsl,qspi-has-second-chip property, that is not needed if a single consistent scheme for the reg properties is used. Regards, Frieder
diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts index e3533e7..1a6f680 100644 --- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts +++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts @@ -131,13 +131,17 @@ #size-cells = <1>; compatible = "spansion,s25fl128s", "jedec,spi-nor"; spi-max-frequency = <66000000>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; }; - flash1: s25fl128s@1 { - reg = <1>; + flash1: s25fl128s@2 { + reg = <2>; #address-cells = <1>; #size-cells = <1>; compatible = "spansion,s25fl128s", "jedec,spi-nor"; spi-max-frequency = <66000000>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; }; }; diff --git a/arch/arm/boot/dts/imx6sx-sdb.dts b/arch/arm/boot/dts/imx6sx-sdb.dts index 6dd9beb..9acfda8 100644 --- a/arch/arm/boot/dts/imx6sx-sdb.dts +++ b/arch/arm/boot/dts/imx6sx-sdb.dts @@ -117,15 +117,19 @@ #size-cells = <1>; compatible = "micron,n25q256a", "jedec,spi-nor"; spi-max-frequency = <29000000>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; reg = <0>; }; - flash1: n25q256a@1 { + flash1: n25q256a@2 { #address-cells = <1>; #size-cells = <1>; compatible = "micron,n25q256a", "jedec,spi-nor"; spi-max-frequency = <29000000>; - reg = <1>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; + reg = <2>; }; }; diff --git a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi index 32a0723..c2c9a2a 100644 --- a/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi +++ b/arch/arm/boot/dts/imx6ul-14x14-evk.dtsi @@ -176,6 +176,8 @@ #size-cells = <1>; compatible = "micron,n25q256a"; spi-max-frequency = <29000000>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; reg = <0>; }; }; diff --git a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts index d01f64b..6a83f30 100644 --- a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts +++ b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts @@ -203,9 +203,6 @@ }; &qspi { - bus-num = <0>; - fsl,spi-num-chipselects = <2>; - fsl,spi-flash-chipselects = <0>; fsl,qspi-has-second-chip; status = "okay"; @@ -214,6 +211,8 @@ #address-cells = <1>; #size-cells = <1>; spi-max-frequency = <20000000>; + spi-rx-bus-width = <4>; + spi-tx-bus-width = <4>; reg = <0>; partitions@0 {
The FSL QSPI driver was moved to the SPI framework and it now acts as a SPI controller. Therefore the subnodes need to set spi-[rx/tx]-bus-width = <4>, so quad mode is used just as before. Also the properties 'bus-num', 'fsl,spi-num-chipselects' and 'fsl,spi-flash-chipselects' were never read by the driver and can be removed. The 'reg' properties are adjusted to reflect the what bus and chipselect the flash is connected to, as the new driver needs this information. The property 'fsl,qspi-has-second-chip' is not needed anymore and will be removed after the old driver was disabled to avoid breaking ls1021a-moxa-uc-8410a.dts. Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de> --- arch/arm/boot/dts/imx6sx-sdb-reva.dts | 8 ++++++-- arch/arm/boot/dts/imx6sx-sdb.dts | 8 ++++++-- arch/arm/boot/dts/imx6ul-14x14-evk.dtsi | 2 ++ arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 5 ++--- 4 files changed, 16 insertions(+), 7 deletions(-)