| Message ID | 20161207152109.17545-1-uwe@kleine-koenig.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Uwe, 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". Gregory > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > 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 >
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. Best regards Uwe
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
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
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"; };