All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frieder Schrempf <frieder.schrempf@exceet.de>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org,
	dwmw2@infradead.org, computersforpeace@gmail.com,
	marek.vasut@gmail.com, richard@nod.at, miquel.raynal@bootlin.com,
	broonie@kernel.org, david.wolfe@nxp.com, fabio.estevam@nxp.com,
	prabhakar.kushwaha@nxp.com, yogeshnarayan.gaur@nxp.com,
	han.xu@nxp.com, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/11] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties
Date: Fri, 1 Jun 2018 11:27:13 +0200	[thread overview]
Message-ID: <d6512adf-2d48-5bdc-d793-a7c1fcabc177@exceet.de> (raw)
In-Reply-To: <20180530171057.39f1a2be@bbrezillon>

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

WARNING: multiple messages have this Message-ID (diff)
From: Frieder Schrempf <frieder.schrempf@exceet.de>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, yogeshnarayan.gaur@nxp.com,
	Rob Herring <robh+dt@kernel.org>,
	richard@nod.at, Sascha Hauer <s.hauer@pengutronix.de>,
	prabhakar.kushwaha@nxp.com, linux-kernel@vger.kernel.org,
	Shawn Guo <shawnguo@kernel.org>,
	linux-spi@vger.kernel.org, marek.vasut@gmail.com, han.xu@nxp.com,
	broonie@kernel.org, linux-mtd@lists.infradead.org,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	miquel.raynal@bootlin.com, fabio.estevam@nxp.com,
	david.wolfe@nxp.com, computersforpeace@gmail.com,
	dwmw2@infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 05/11] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties
Date: Fri, 1 Jun 2018 11:27:13 +0200	[thread overview]
Message-ID: <d6512adf-2d48-5bdc-d793-a7c1fcabc177@exceet.de> (raw)
In-Reply-To: <20180530171057.39f1a2be@bbrezillon>

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

WARNING: multiple messages have this Message-ID (diff)
From: frieder.schrempf@exceet.de (Frieder Schrempf)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 05/11] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties
Date: Fri, 1 Jun 2018 11:27:13 +0200	[thread overview]
Message-ID: <d6512adf-2d48-5bdc-d793-a7c1fcabc177@exceet.de> (raw)
In-Reply-To: <20180530171057.39f1a2be@bbrezillon>

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 at 1 {
>> -		reg = <1>;
>> +	flash1: s25fl128s at 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

  reply	other threads:[~2018-06-01  9:28 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30 13:14 [PATCH 00/11] Port the FSL QSPI driver to the SPI framework Frieder Schrempf
2018-05-30 13:14 ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 01/11] spi: spi-mem: Extend the SPI mem interface to set a custom memory name Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 14:32   ` Boris Brezillon
2018-05-30 15:12     ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 02/11] mtd: m25p80: Call spi_mem_get_name() to let controller set a custom name Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:50   ` Yogesh Narayan Gaur
2018-05-30 14:24     ` Boris Brezillon
2018-06-01  9:14       ` Frieder Schrempf
2018-06-01  9:14         ` Frieder Schrempf
2018-05-30 14:58   ` Boris Brezillon
2018-05-30 15:13     ` Frieder Schrempf
2018-05-30 15:13       ` Frieder Schrempf
2018-06-05 15:00   ` Boris Brezillon
2018-06-08 11:54   ` Yogesh Narayan Gaur
2018-06-08 12:51     ` Boris Brezillon
2018-06-11  6:31       ` Yogesh Narayan Gaur
2018-06-11  7:46         ` Boris Brezillon
2018-06-11  9:38           ` Yogesh Narayan Gaur
2018-06-11 10:16             ` Boris Brezillon
2018-06-11 10:21               ` Yogesh Narayan Gaur
2018-06-12  6:42                 ` Yogesh Narayan Gaur
2018-06-12  7:13                   ` Boris Brezillon
2018-06-12  8:51                     ` Yogesh Narayan Gaur
2018-06-15 12:50                       ` Boris Brezillon
2018-06-15 13:42                         ` Yogesh Narayan Gaur
2018-06-15 13:55                           ` Boris Brezillon
2018-06-15 13:58                             ` Boris Brezillon
2018-06-18 13:32                             ` Yogesh Narayan Gaur
2018-06-18 19:15                               ` Boris Brezillon
2018-06-19  7:10                                 ` Yogesh Narayan Gaur
2018-06-19  7:28                                   ` Boris Brezillon
2018-06-19  8:31                                     ` Yogesh Narayan Gaur
2018-06-19  8:46                                       ` Boris Brezillon
2018-06-26  8:58                                         ` Frieder Schrempf
2018-06-08 20:27     ` Andy Shevchenko
2018-06-26 12:26       ` Frieder Schrempf
2018-06-26 12:26         ` Frieder Schrempf
2018-06-26 13:18         ` Andy Shevchenko
2018-06-26 13:47           ` Boris Brezillon
2018-06-26 15:42             ` Andy Shevchenko
2018-06-18 19:27   ` Boris Brezillon
2018-05-30 13:14 ` [PATCH 04/11] dt-bindings: spi: Move and adjust the bindings for the fsl-qspi driver Frieder Schrempf
2018-05-30 15:06   ` Boris Brezillon
2018-05-30 15:14     ` Frieder Schrempf
2018-05-30 15:14       ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 05/11] ARM: dts: Reflect change of FSL QSPI driver and remove unused properties Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 15:10   ` Boris Brezillon
2018-05-30 15:10     ` Boris Brezillon
2018-06-01  9:27     ` Frieder Schrempf [this message]
2018-06-01  9:27       ` Frieder Schrempf
2018-06-01  9:27       ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 06/11] arm64: " Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 07/11] ARM: defconfig: Use the new FSL QSPI driver under the SPI framework Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 08/11] mtd: fsl-quadspi: Remove the driver as it was replaced by spi-fsl-qspi.c Frieder Schrempf
2018-05-30 13:14 ` [PATCH 09/11] ARM: dts: ls1021a: Remove fsl,qspi-has-second-chip as it is not used Frieder Schrempf
2018-05-30 13:14   ` [PATCH 09/11] ARM: dts: ls1021a: Remove fsl, qspi-has-second-chip " Frieder Schrempf
2018-05-30 13:14 ` [PATCH 10/11] ARM64: dts: ls1046a: Remove fsl,qspi-has-second-chip " Frieder Schrempf
2018-05-30 13:14   ` [PATCH 10/11] ARM64: dts: ls1046a: Remove fsl, qspi-has-second-chip " Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf
2018-05-30 13:14 ` [PATCH 11/11] MAINTAINERS: Move the Freescale QSPI driver to the SPI framework Frieder Schrempf
2018-05-30 13:14   ` Frieder Schrempf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d6512adf-2d48-5bdc-d793-a7c1fcabc177@exceet.de \
    --to=frieder.schrempf@exceet.de \
    --cc=boris.brezillon@bootlin.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=david.wolfe@nxp.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=fabio.estevam@nxp.com \
    --cc=han.xu@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=prabhakar.kushwaha@nxp.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=yogeshnarayan.gaur@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.