[PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Uwe Kleine-König]

From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

The SPI controllers on Armada 370 and XP differ from the original Orion
SPI controllers (at least) in the configuration of the baud rate. So
it's wrong to claim compatibility which results in bogus baud rates.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/armada-370.dtsi | 4 ++--
 arch/arm/boot/dts/armada-xp.dtsi  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index b4258105e91f..b9377c11b379 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -435,13 +435,13 @@
  * board level if a different configuration is used.
  */
 &spi0 {
-	compatible = "marvell,armada-370-spi", "marvell,orion-spi";
+	compatible = "marvell,armada-370-spi";
 	pinctrl-0 = <&spi0_pins1>;
 	pinctrl-names = "default";
 };
 
 &spi1 {
-	compatible = "marvell,armada-370-spi", "marvell,orion-spi";
+	compatible = "marvell,armada-370-spi";
 	pinctrl-0 = <&spi1_pins>;
 	pinctrl-names = "default";
 };
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 4a5f99e65b51..3a416834d80b 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -367,13 +367,13 @@
 };
 
 &spi0 {
-	compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
+	compatible = "marvell,armada-xp-spi";
 	pinctrl-0 = <&spi0_pins>;
 	pinctrl-names = "default";
 };
 
 &spi1 {
-	compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
+	compatible = "marvell,armada-xp-spi";
 	pinctrl-0 = <&spi1_pins>;
 	pinctrl-names = "default";
 };
-- 
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Gregory CLEMENT]

Hi Uwe,
 
 On mer., déc. 07 2016, Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org> wrote:

> From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> The SPI controllers on Armada 370 and XP differ from the original Orion
> SPI controllers (at least) in the configuration of the baud rate. So
> it's wrong to claim compatibility which results in bogus baud rates.

Until two years ago with the commits
df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
4dacccfac69494ba70248b134352f299171c41b7
we used "marvell,orion-spi" compatible on Armada XP and Armada 370
without any problem.

The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
allows to have more choice for the baudrate for a given clock but it is
not true that Armada 370 and Armada XP are not compatible with
"marvell,orion-spi".

Gregory

>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/armada-370.dtsi | 4 ++--
>  arch/arm/boot/dts/armada-xp.dtsi  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
> index b4258105e91f..b9377c11b379 100644
> --- a/arch/arm/boot/dts/armada-370.dtsi
> +++ b/arch/arm/boot/dts/armada-370.dtsi
> @@ -435,13 +435,13 @@
>   * board level if a different configuration is used.
>   */
>  &spi0 {
> -	compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> +	compatible = "marvell,armada-370-spi";
>  	pinctrl-0 = <&spi0_pins1>;
>  	pinctrl-names = "default";
>  };
>  
>  &spi1 {
> -	compatible = "marvell,armada-370-spi", "marvell,orion-spi";
> +	compatible = "marvell,armada-370-spi";
>  	pinctrl-0 = <&spi1_pins>;
>  	pinctrl-names = "default";
>  };
> diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
> index 4a5f99e65b51..3a416834d80b 100644
> --- a/arch/arm/boot/dts/armada-xp.dtsi
> +++ b/arch/arm/boot/dts/armada-xp.dtsi
> @@ -367,13 +367,13 @@
>  };
>  
>  &spi0 {
> -	compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> +	compatible = "marvell,armada-xp-spi";
>  	pinctrl-0 = <&spi0_pins>;
>  	pinctrl-names = "default";
>  };
>  
>  &spi1 {
> -	compatible = "marvell,armada-xp-spi", "marvell,orion-spi";
> +	compatible = "marvell,armada-xp-spi";
>  	pinctrl-0 = <&spi1_pins>;
>  	pinctrl-names = "default";
>  };
> -- 
> 2.10.2
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Uwe Kleine-König]

Hello Gregory,

On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
>  On mer., déc. 07 2016, Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org> wrote:
> 
> > From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >
> > The SPI controllers on Armada 370 and XP differ from the original Orion
> > SPI controllers (at least) in the configuration of the baud rate. So
> > it's wrong to claim compatibility which results in bogus baud rates.
> 
> Until two years ago with the commits
> df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> 4dacccfac69494ba70248b134352f299171c41b7
> we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> without any problem.
> 
> The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> allows to have more choice for the baudrate for a given clock but it is
> not true that Armada 370 and Armada XP are not compatible with
> "marvell,orion-spi".

The issue I was faced with that made me create this patch is that in
barebox no special case for 370/XP was active. The result was that a
device that could be operated at 60 MHz only got a clock of 11 MHz and
the driver assumed it configured 41.666 MHz. I didn't check if the same
can happen in the opposite direction (or if there are other important
incompatibilities) but still I'd say this is a bug with my patch being
the obvious fix.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

Hello,

On Wed, Dec 07, 2016 at 04:41:45PM +0100, Uwe Kleine-König wrote:
> Hello Gregory,
> 
> On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
> >  On mer., déc. 07 2016, Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> > 
> > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > >
> > > The SPI controllers on Armada 370 and XP differ from the original Orion
> > > SPI controllers (at least) in the configuration of the baud rate. So
> > > it's wrong to claim compatibility which results in bogus baud rates.
> > 
> > Until two years ago with the commits
> > df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> > 4dacccfac69494ba70248b134352f299171c41b7
> > we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> > without any problem.
> > 
> > The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> > allows to have more choice for the baudrate for a given clock but it is
> > not true that Armada 370 and Armada XP are not compatible with
> > "marvell,orion-spi".
> 
> The issue I was faced with that made me create this patch is that in
> barebox no special case for 370/XP was active. The result was that a
> device that could be operated at 60 MHz only got a clock of 11 MHz and
> the driver assumed it configured 41.666 MHz. I didn't check if the same
> can happen in the opposite direction (or if there are other important
> incompatibilities) but still I'd say this is a bug with my patch being
> the obvious fix.

I just found this patch in an old branch and wonder what do to with it.
It still applies fine at least.

(If the original patch already disappeared from your inbox, it can be
found at https://lore.kernel.org/r/20161207152109.17545-1-uwe@kleine-koenig.org/ )

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Andrew Lunn]

On Thu, Sep 09, 2021 at 09:50:05AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Wed, Dec 07, 2016 at 04:41:45PM +0100, Uwe Kleine-König wrote:
> > Hello Gregory,
> > 
> > On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
> > >  On mer., déc. 07 2016, Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> > > 
> > > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > >
> > > > The SPI controllers on Armada 370 and XP differ from the original Orion
> > > > SPI controllers (at least) in the configuration of the baud rate. So
> > > > it's wrong to claim compatibility which results in bogus baud rates.
> > > 
> > > Until two years ago with the commits
> > > df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> > > 4dacccfac69494ba70248b134352f299171c41b7
> > > we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> > > without any problem.
> > > 
> > > The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> > > allows to have more choice for the baudrate for a given clock but it is
> > > not true that Armada 370 and Armada XP are not compatible with
> > > "marvell,orion-spi".
> > 
> > The issue I was faced with that made me create this patch is that in
> > barebox no special case for 370/XP was active. The result was that a
> > device that could be operated at 60 MHz only got a clock of 11 MHz and
> > the driver assumed it configured 41.666 MHz. I didn't check if the same
> > can happen in the opposite direction (or if there are other important
> > incompatibilities) but still I'd say this is a bug with my patch being
> > the obvious fix.
> 
> I just found this patch in an old branch and wonder what do to with it.
> It still applies fine at least.
> 
> (If the original patch already disappeared from your inbox, it can be
> found at https://lore.kernel.org/r/20161207152109.17545-1-uwe@kleine-koenig.org/ )

If you remove "marvell,orion-spi" you are going to break running a new
DT blob on an old kernel.

The compatible list is supposed to be most specific to most generic in
order. So "marvell,orion-spi" is the fall back option for Armada, you
don't get all the features, but it should work at a basic level. And
in this case, barebox did its best, it gave you a working but
unexpectedly slow bus. If you take away "marvell,orion-spi", and there
is no support for "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
in barebox, does that not then mean there is no SPI support at all?

   Andrew