[PATCH v4 04/10] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties

[PATCH v4 04/10] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@exceet.de>

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(-)

diff --git a/arch/arm/boot/dts/imx6sx-sdb-reva.dts b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
index 9cc6ff2..9997156 100644
--- a/arch/arm/boot/dts/imx6sx-sdb-reva.dts
+++ b/arch/arm/boot/dts/imx6sx-sdb-reva.dts
@@ -132,13 +132,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 at 1 {
-		reg = <1>;
+	flash1: s25fl128s at 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 at 1 {
+	flash1: n25q256a at 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 at 0 {
-- 
2.7.4

[PATCH v4 05/10] arm64: dts: Reflect change of FSL QSPI driver and remove unused properties

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@exceet.de>

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 'num-cs' and 'bus-num' were never read by the
driver and can be removed.

The property 'fsl,qspi-has-second-chip' is not needed anymore
and will be removed after the old driver was disabled to avoid
breaking fsl-ls1046a-rdb.dts.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts  | 3 ++-
 arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts  | 4 ++--
 arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts  | 6 ++++--
 arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi | 4 ++++
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
index dff3d64..8a50094 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a-qds.dts
@@ -135,7 +135,6 @@
 };
 
 &qspi {
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fl128s at 0 {
@@ -143,6 +142,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
index e58a8ca..2f220ec 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-qds.dts
@@ -163,8 +163,6 @@
 };
 
 &qspi {
-	num-cs = <2>;
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fl128s at 0 {
@@ -172,6 +170,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
index a59b482..07c665c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
@@ -99,8 +99,6 @@
 };
 
 &qspi {
-	num-cs = <2>;
-	bus-num = <0>;
 	status = "okay";
 
 	qflash0: s25fs512s at 0 {
@@ -108,6 +106,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 
@@ -116,6 +116,8 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <1>;
 	};
 };
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
index c11f52e..10d2fe0 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa-qds.dtsi
@@ -134,6 +134,8 @@
 		#size-cells = <1>;
 		compatible = "st,m25p80";
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <0>;
 	};
 	flash2: s25fl256s1 at 2 {
@@ -141,6 +143,8 @@
 		#size-cells = <1>;
 		compatible = "st,m25p80";
 		spi-max-frequency = <20000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
 		reg = <2>;
 	};
 };
-- 
2.7.4

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@exceet.de>

The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm/configs/imx_v6_v7_defconfig | 2 +-
 arch/arm/configs/multi_v7_defconfig  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 1ad5736..1741d87 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -113,7 +113,6 @@ CONFIG_MTD_NAND_GPMI_NAND=y
 CONFIG_MTD_NAND_VF610_NFC=y
 CONFIG_MTD_NAND_MXC=y
 CONFIG_MTD_SPI_NOR=y
-CONFIG_SPI_FSL_QUADSPI=y
 CONFIG_MTD_UBI=y
 CONFIG_MTD_UBI_FASTMAP=y
 CONFIG_MTD_UBI_BLOCK=y
@@ -204,6 +203,7 @@ CONFIG_I2C_ALGOPCA=m
 CONFIG_I2C_GPIO=y
 CONFIG_I2C_IMX=y
 CONFIG_SPI=y
+CONFIG_SPI_FSL_QSPI=y
 CONFIG_SPI_GPIO=y
 CONFIG_SPI_IMX=y
 CONFIG_SPI_FSL_DSPI=y
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 1c76168..faddf29 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -193,7 +193,6 @@ CONFIG_MTD_NAND_BRCMNAND=y
 CONFIG_MTD_NAND_VF610_NFC=y
 CONFIG_MTD_NAND_DAVINCI=y
 CONFIG_MTD_SPI_NOR=y
-CONFIG_SPI_FSL_QUADSPI=m
 CONFIG_MTD_UBI=y
 CONFIG_BLK_DEV_LOOP=y
 CONFIG_BLK_DEV_RAM=y
@@ -383,6 +382,7 @@ CONFIG_SPI_BCM2835=y
 CONFIG_SPI_BCM2835AUX=y
 CONFIG_SPI_CADENCE=y
 CONFIG_SPI_DAVINCI=y
+CONFIG_SPI_FSL_QSPI=m
 CONFIG_SPI_GPIO=m
 CONFIG_SPI_FSL_DSPI=m
 CONFIG_SPI_OMAP24XX=y
-- 
2.7.4

[PATCH v4 08/10] ARM: dts: ls1021a: Remove fsl, qspi-has-second-chip as it is not used

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@exceet.de>

After switching to the new FSL QSPI driver the property
'fsl,qspi-has-second-chip' is not needed anymore.

The driver now uses the 'reg' property to determine the bus and
the chipselect.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
index 6a83f30..d3a1a73 100644
--- a/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
+++ b/arch/arm/boot/dts/ls1021a-moxa-uc-8410a.dts
@@ -203,7 +203,6 @@
 };
 
 &qspi {
-	fsl,qspi-has-second-chip;
 	status = "okay";
 
 	flash: flash at 0 {
-- 
2.7.4

[PATCH v4 09/10] ARM64: dts: ls1046a: Remove fsl, qspi-has-second-chip as it is not used

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@exceet.de>

After switching to the new FSL QSPI driver the property
'fsl,qspi-has-second-chip' is not needed anymore.

The driver now uses the 'reg' property to determine the bus and
the chipselect.

Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
---
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 51cbd50..9e083f6 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -213,7 +213,6 @@
 			clock-names = "qspi_en", "qspi";
 			clocks = <&clockgen 4 1>, <&clockgen 4 1>;
 			big-endian;
-			fsl,qspi-has-second-chip;
 			status = "disabled";
 		};
 
-- 
2.7.4

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Olof Johansson]

On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> From: Frieder Schrempf <frieder.schrempf@exceet.de>
>
> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>

Hi Frieder,

This patch is part of a series that I didn't see the rest of, but in
general we prefer to merge these through arm-soc even if the driver
goes in through another tree. The way we'd prefer to handle it is that
once the driver lands, we'll take the config option change to turn it
on. To avoid our branches to break until both sides have landed, it
might be a good idea to keep both drivers on for a short while (one
release).

So, I'm not going to ack this since we avoid taking defconfig changes
through driver trees (these two defconfigs tend to churn a lot and we
don't want to create merge conflicts where we don't have to), but
we'll be happy to pick it up when the time comes.


Thanks,

-Olof

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Schrempf Frieder]

Hi Olof,

On 07.11.18 17:20, Olof Johansson wrote:
> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>>
>> From: Frieder Schrempf <frieder.schrempf@exceet.de>
>>
>> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
>> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
>>
>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> 
> Hi Frieder,
> 
> This patch is part of a series that I didn't see the rest of, but in
> general we prefer to merge these through arm-soc even if the driver
> goes in through another tree. The way we'd prefer to handle it is that
> once the driver lands, we'll take the config option change to turn it
> on. To avoid our branches to break until both sides have landed, it
> might be a good idea to keep both drivers on for a short while (one
> release).
> 
> So, I'm not going to ack this since we avoid taking defconfig changes
> through driver trees (these two defconfigs tend to churn a lot and we
> don't want to create merge conflicts where we don't have to), but
> we'll be happy to pick it up when the time comes.

Ok, thank you for explaining the common practice. I will drop the config 
changes for the next version and send it separately when the time is ready.

Both the old driver and the new one use the same compatible strings for 
probing. Wouldn't that cause problems if both drivers are enabled for a 
while, or am I missing something?

Thanks,
Frieder

> 
> 
> Thanks,
> 
> -Olof
> 

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Olof Johansson]

On Wed, Nov 7, 2018 at 8:36 AM Schrempf Frieder
<frieder.schrempf@kontron.de> wrote:
>
> Hi Olof,
>
> On 07.11.18 17:20, Olof Johansson wrote:
> > On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
> > <frieder.schrempf@kontron.de> wrote:
> >>
> >> From: Frieder Schrempf <frieder.schrempf@exceet.de>
> >>
> >> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
> >> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
> >
> > Hi Frieder,
> >
> > This patch is part of a series that I didn't see the rest of, but in
> > general we prefer to merge these through arm-soc even if the driver
> > goes in through another tree. The way we'd prefer to handle it is that
> > once the driver lands, we'll take the config option change to turn it
> > on. To avoid our branches to break until both sides have landed, it
> > might be a good idea to keep both drivers on for a short while (one
> > release).
> >
> > So, I'm not going to ack this since we avoid taking defconfig changes
> > through driver trees (these two defconfigs tend to churn a lot and we
> > don't want to create merge conflicts where we don't have to), but
> > we'll be happy to pick it up when the time comes.
>
> Ok, thank you for explaining the common practice. I will drop the config
> changes for the next version and send it separately when the time is ready.
>
> Both the old driver and the new one use the same compatible strings for
> probing. Wouldn't that cause problems if both drivers are enabled for a
> while, or am I missing something?

Only one of them would be allowed to bind to a device, but it might
not be predictable which one does (especially in the case of modules).

So, it's far from ideal, but breaking the platform is possibly worse.
It might just be a good idea to merge the driver and not turn it on
until it's in for that case (i.e. we take the config change between
-rc1 and -rc2).


-Olof

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Boris Brezillon]

On Wed, 7 Nov 2018 16:36:13 +0000
Schrempf Frieder <frieder.schrempf@kontron.De> wrote:

> Hi Olof,
> 
> On 07.11.18 17:20, Olof Johansson wrote:
> > On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
> > <frieder.schrempf@kontron.de> wrote:  
> >>
> >> From: Frieder Schrempf <frieder.schrempf@exceet.de>
> >>
> >> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
> >> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>  
> > 
> > Hi Frieder,
> > 
> > This patch is part of a series that I didn't see the rest of, but in
> > general we prefer to merge these through arm-soc even if the driver
> > goes in through another tree. The way we'd prefer to handle it is that
> > once the driver lands, we'll take the config option change to turn it
> > on. To avoid our branches to break until both sides have landed, it
> > might be a good idea to keep both drivers on for a short while (one
> > release).
> > 
> > So, I'm not going to ack this since we avoid taking defconfig changes
> > through driver trees (these two defconfigs tend to churn a lot and we
> > don't want to create merge conflicts where we don't have to), but
> > we'll be happy to pick it up when the time comes.  
> 
> Ok, thank you for explaining the common practice. I will drop the config 
> changes for the next version and send it separately when the time is ready.
> 
> Both the old driver and the new one use the same compatible strings for 
> probing. Wouldn't that cause problems if both drivers are enabled for a 
> while, or am I missing something?

Or maybe we should not introduce a new Kconfig option and just reuse
the old one. It probably requires re-ordering patches a bit (patch 1
should be moved after patch 5). Then you have 2 choices:

1/ merge patch 1 and 6 so that the new driver effectively replaces the
   old one but uses the same Kconfig option
2/ remove the ability to compile the old driver when the new one is
   introduced: remove the line from drivers/mtd/spi-nor Makefile and
   move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to
   drivers/spi/Kconfig. And remove the old code in a separate patch

I'm fine either way, but option #2 will probably make the patch
introducing the new driver bigger and hurt readability.

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Schrempf Frieder]

On 08.11.18 09:34, Boris Brezillon wrote:
> On Wed, 7 Nov 2018 16:36:13 +0000
> Schrempf Frieder <frieder.schrempf@kontron.De> wrote:
> 
>> Hi Olof,
>>
>> On 07.11.18 17:20, Olof Johansson wrote:
>>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
>>> <frieder.schrempf@kontron.de> wrote:
>>>>
>>>> From: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>>
>>>> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
>>>> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
>>>>
>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>
>>> Hi Frieder,
>>>
>>> This patch is part of a series that I didn't see the rest of, but in
>>> general we prefer to merge these through arm-soc even if the driver
>>> goes in through another tree. The way we'd prefer to handle it is that
>>> once the driver lands, we'll take the config option change to turn it
>>> on. To avoid our branches to break until both sides have landed, it
>>> might be a good idea to keep both drivers on for a short while (one
>>> release).
>>>
>>> So, I'm not going to ack this since we avoid taking defconfig changes
>>> through driver trees (these two defconfigs tend to churn a lot and we
>>> don't want to create merge conflicts where we don't have to), but
>>> we'll be happy to pick it up when the time comes.
>>
>> Ok, thank you for explaining the common practice. I will drop the config
>> changes for the next version and send it separately when the time is ready.
>>
>> Both the old driver and the new one use the same compatible strings for
>> probing. Wouldn't that cause problems if both drivers are enabled for a
>> while, or am I missing something?
> 
> Or maybe we should not introduce a new Kconfig option and just reuse
> the old one. It probably requires re-ordering patches a bit (patch 1
> should be moved after patch 5). Then you have 2 choices:
> 
> 1/ merge patch 1 and 6 so that the new driver effectively replaces the
>     old one but uses the same Kconfig option
> 2/ remove the ability to compile the old driver when the new one is
>     introduced: remove the line from drivers/mtd/spi-nor Makefile and
>     move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to
>     drivers/spi/Kconfig. And remove the old code in a separate patch
> 
> I'm fine either way, but option #2 will probably make the patch
> introducing the new driver bigger and hurt readability.

I think having both drivers in the tree for a while wouldn't be so bad. 
So if any compatibility issues come up with the new driver, people can 
still use the old one.

Therefore I think I will drop the patches that change the defconfig and 
remove the old driver code and keep the different Kconfig options. And 
maybe add an exclusive dependency in Kconfig, so both drivers can not be 
enabled at the same time.

Does this make sense?

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Boris Brezillon]

On Mon, 12 Nov 2018 10:46:45 +0000
Schrempf Frieder <frieder.schrempf@kontron.De> wrote:

> On 08.11.18 09:34, Boris Brezillon wrote:
> > On Wed, 7 Nov 2018 16:36:13 +0000
> > Schrempf Frieder <frieder.schrempf@kontron.De> wrote:
> >   
> >> Hi Olof,
> >>
> >> On 07.11.18 17:20, Olof Johansson wrote:  
> >>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
> >>> <frieder.schrempf@kontron.de> wrote:  
> >>>>
> >>>> From: Frieder Schrempf <frieder.schrempf@exceet.de>
> >>>>
> >>>> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
> >>>> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
> >>>>
> >>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>  
> >>>
> >>> Hi Frieder,
> >>>
> >>> This patch is part of a series that I didn't see the rest of, but in
> >>> general we prefer to merge these through arm-soc even if the driver
> >>> goes in through another tree. The way we'd prefer to handle it is that
> >>> once the driver lands, we'll take the config option change to turn it
> >>> on. To avoid our branches to break until both sides have landed, it
> >>> might be a good idea to keep both drivers on for a short while (one
> >>> release).
> >>>
> >>> So, I'm not going to ack this since we avoid taking defconfig changes
> >>> through driver trees (these two defconfigs tend to churn a lot and we
> >>> don't want to create merge conflicts where we don't have to), but
> >>> we'll be happy to pick it up when the time comes.  
> >>
> >> Ok, thank you for explaining the common practice. I will drop the config
> >> changes for the next version and send it separately when the time is ready.
> >>
> >> Both the old driver and the new one use the same compatible strings for
> >> probing. Wouldn't that cause problems if both drivers are enabled for a
> >> while, or am I missing something?  
> > 
> > Or maybe we should not introduce a new Kconfig option and just reuse
> > the old one. It probably requires re-ordering patches a bit (patch 1
> > should be moved after patch 5). Then you have 2 choices:
> > 
> > 1/ merge patch 1 and 6 so that the new driver effectively replaces the
> >     old one but uses the same Kconfig option
> > 2/ remove the ability to compile the old driver when the new one is
> >     introduced: remove the line from drivers/mtd/spi-nor Makefile and
> >     move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to
> >     drivers/spi/Kconfig. And remove the old code in a separate patch
> > 
> > I'm fine either way, but option #2 will probably make the patch
> > introducing the new driver bigger and hurt readability.  
> 
> I think having both drivers in the tree for a while wouldn't be so bad. 
> So if any compatibility issues come up with the new driver, people can 
> still use the old one.

Except that's not what happens in practice. Believe me, I tried this
approach several times, and people keep using the old driver until
they're forced to switch to the new one. So you actually don't address
the problem, you just delay it a bit, and you'll have to fix
regressions anyway.

> 
> Therefore I think I will drop the patches that change the defconfig and 
> remove the old driver code and keep the different Kconfig options. And 
> maybe add an exclusive dependency in Kconfig, so both drivers can not be 
> enabled at the same time.
> 
> Does this make sense?

I'd really prefer to have the removal of the old driver in the same
release the new driver is introduced but if that's not possible, let's
have a clear plan, like "introduce new driver in release X, remove the
old one in release X+1".

[PATCH v4 06/10] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework

[Schrempf Frieder]

On 12.11.18 11:56, Boris Brezillon wrote:
> On Mon, 12 Nov 2018 10:46:45 +0000
> Schrempf Frieder <frieder.schrempf@kontron.De> wrote:
> 
>> On 08.11.18 09:34, Boris Brezillon wrote:
>>> On Wed, 7 Nov 2018 16:36:13 +0000
>>> Schrempf Frieder <frieder.schrempf@kontron.De> wrote:
>>>    
>>>> Hi Olof,
>>>>
>>>> On 07.11.18 17:20, Olof Johansson wrote:
>>>>> On Wed, Nov 7, 2018 at 6:44 AM Frieder Schrempf
>>>>> <frieder.schrempf@kontron.de> wrote:
>>>>>>
>>>>>> From: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>>>>
>>>>>> The new driver at spi/spi-fsl-qspi.c replaces the old SPI NOR driver
>>>>>> at mtd/fsl-quadspi.c. Switch to the new driver in the defconfigs.
>>>>>>
>>>>>> Signed-off-by: Frieder Schrempf <frieder.schrempf@exceet.de>
>>>>>
>>>>> Hi Frieder,
>>>>>
>>>>> This patch is part of a series that I didn't see the rest of, but in
>>>>> general we prefer to merge these through arm-soc even if the driver
>>>>> goes in through another tree. The way we'd prefer to handle it is that
>>>>> once the driver lands, we'll take the config option change to turn it
>>>>> on. To avoid our branches to break until both sides have landed, it
>>>>> might be a good idea to keep both drivers on for a short while (one
>>>>> release).
>>>>>
>>>>> So, I'm not going to ack this since we avoid taking defconfig changes
>>>>> through driver trees (these two defconfigs tend to churn a lot and we
>>>>> don't want to create merge conflicts where we don't have to), but
>>>>> we'll be happy to pick it up when the time comes.
>>>>
>>>> Ok, thank you for explaining the common practice. I will drop the config
>>>> changes for the next version and send it separately when the time is ready.
>>>>
>>>> Both the old driver and the new one use the same compatible strings for
>>>> probing. Wouldn't that cause problems if both drivers are enabled for a
>>>> while, or am I missing something?
>>>
>>> Or maybe we should not introduce a new Kconfig option and just reuse
>>> the old one. It probably requires re-ordering patches a bit (patch 1
>>> should be moved after patch 5). Then you have 2 choices:
>>>
>>> 1/ merge patch 1 and 6 so that the new driver effectively replaces the
>>>      old one but uses the same Kconfig option
>>> 2/ remove the ability to compile the old driver when the new one is
>>>      introduced: remove the line from drivers/mtd/spi-nor Makefile and
>>>      move the Kconfig entry from drivers/mtd/spi-nor/Kconfig to
>>>      drivers/spi/Kconfig. And remove the old code in a separate patch
>>>
>>> I'm fine either way, but option #2 will probably make the patch
>>> introducing the new driver bigger and hurt readability.
>>
>> I think having both drivers in the tree for a while wouldn't be so bad.
>> So if any compatibility issues come up with the new driver, people can
>> still use the old one.
> 
> Except that's not what happens in practice. Believe me, I tried this
> approach several times, and people keep using the old driver until
> they're forced to switch to the new one. So you actually don't address
> the problem, you just delay it a bit, and you'll have to fix
> regressions anyway.

Ok, I see.

>> Therefore I think I will drop the patches that change the defconfig and
>> remove the old driver code and keep the different Kconfig options. And
>> maybe add an exclusive dependency in Kconfig, so both drivers can not be
>> enabled at the same time.
>>
>> Does this make sense?
> 
> I'd really prefer to have the removal of the old driver in the same
> release the new driver is introduced but if that's not possible, let's
> have a clear plan, like "introduce new driver in release X, remove the
> old one in release X+1".

We can do it as you suggested. I will think about whether to use option 
#1 or #2.
With #1 we will have the removal of the old driver and adding the new 
driver in one single patch.
With #2 we will have the disabling of the old driver via Makefile in the 
same patch thats adding the new driver.