2016-11-08 16:30:49

by Marcin Wojtas

[permalink] [raw]
Subject: [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers

Enabling SPI controllers, which are attached to different busses
inside an SoC, may result in overlapping enumeration and cause
sysfs registration failure. Example log after enabling two
controllers on Armada 8040 SoC with same identifiers:

[ 3.740415] sysfs: cannot create duplicate filename
'/class/spi_master/spi0'
[ 3.747510] ------------[ cut here ]------------
[ 3.752145] WARNING: at fs/sysfs/dir.c:31
[...]
[ 4.002299] orion_spi: probe of f4700600.spi failed with error -17

spi-orion driver offers dedicated DT property ('cell-index'), that
allow setting unique identifiers. Recently added support for CP110-slave
HW block introduced two new SPI controllers' nodes with same ID as
ones from CP110-master.

This commit fixes the issue by assigning different 'cell-index' values
for CP110-slave SPI controllers.

Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
CP110 in Armada 8K")
Signed-off-by: Marcin Wojtas <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index abb3fa2..d94d592 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -132,7 +132,7 @@
reg = <0x700600 0x50>;
#address-cells = <0x1>;
#size-cells = <0x0>;
- cell-index = <1>;
+ cell-index = <3>;
clocks = <&cps_syscon0 1 21>;
status = "disabled";
};
@@ -142,7 +142,7 @@
reg = <0x700680 0x50>;
#address-cells = <1>;
#size-cells = <0>;
- cell-index = <2>;
+ cell-index = <4>;
clocks = <&cps_syscon0 1 21>;
status = "disabled";
};
--
1.8.3.1


2016-11-08 16:49:34

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers

Hello,

On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
> Enabling SPI controllers, which are attached to different busses
> inside an SoC, may result in overlapping enumeration and cause
> sysfs registration failure. Example log after enabling two
> controllers on Armada 8040 SoC with same identifiers:
>
> [ 3.740415] sysfs: cannot create duplicate filename
> '/class/spi_master/spi0'
> [ 3.747510] ------------[ cut here ]------------
> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
> [...]
> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>
> spi-orion driver offers dedicated DT property ('cell-index'), that
> allow setting unique identifiers. Recently added support for CP110-slave
> HW block introduced two new SPI controllers' nodes with same ID as
> ones from CP110-master.
>
> This commit fixes the issue by assigning different 'cell-index' values
> for CP110-slave SPI controllers.
>
> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
> CP110 in Armada 8K")
> Signed-off-by: Marcin Wojtas <[email protected]>

It's sad that we need to hardcode those indexes in the Device Tree
(which by no means are a description of the HW by the way), but that's
what the SPI framework expects I believe. Therefore:

Acked-by: Thomas Petazzoni <[email protected]>

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

2016-11-08 16:52:53

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers

Hi Thomas,

2016-11-08 17:48 GMT+01:00 Thomas Petazzoni
<[email protected]>:
> Hello,
>
> On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
>> Enabling SPI controllers, which are attached to different busses
>> inside an SoC, may result in overlapping enumeration and cause
>> sysfs registration failure. Example log after enabling two
>> controllers on Armada 8040 SoC with same identifiers:
>>
>> [ 3.740415] sysfs: cannot create duplicate filename
>> '/class/spi_master/spi0'
>> [ 3.747510] ------------[ cut here ]------------
>> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
>> [...]
>> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>>
>> spi-orion driver offers dedicated DT property ('cell-index'), that
>> allow setting unique identifiers. Recently added support for CP110-slave
>> HW block introduced two new SPI controllers' nodes with same ID as
>> ones from CP110-master.
>>
>> This commit fixes the issue by assigning different 'cell-index' values
>> for CP110-slave SPI controllers.
>>
>> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
>> CP110 in Armada 8K")
>> Signed-off-by: Marcin Wojtas <[email protected]>
>
> It's sad that we need to hardcode those indexes in the Device Tree
> (which by no means are a description of the HW by the way), but that's
> what the SPI framework expects I believe.

Right. Devices are enumerated by generic code beginning from '0' on
each bus (AP806, CP110-master, CP110-slave) independently and
I didn't see any nice solution for it either.

> Therefore:
>
> Acked-by: Thomas Petazzoni <[email protected]>
>

Thanks,
Marcin

2016-11-09 08:47:35

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers

Hi Marcin,

On mar., nov. 08 2016, Thomas Petazzoni <[email protected]> wrote:

> Hello,
>
> On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
>> Enabling SPI controllers, which are attached to different busses
>> inside an SoC, may result in overlapping enumeration and cause
>> sysfs registration failure. Example log after enabling two
>> controllers on Armada 8040 SoC with same identifiers:
>>
>> [ 3.740415] sysfs: cannot create duplicate filename
>> '/class/spi_master/spi0'
>> [ 3.747510] ------------[ cut here ]------------
>> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
>> [...]
>> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>>
>> spi-orion driver offers dedicated DT property ('cell-index'), that
>> allow setting unique identifiers. Recently added support for CP110-slave
>> HW block introduced two new SPI controllers' nodes with same ID as
>> ones from CP110-master.
>>
>> This commit fixes the issue by assigning different 'cell-index' values
>> for CP110-slave SPI controllers.
>>
>> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
>> CP110 in Armada 8K")
>> Signed-off-by: Marcin Wojtas <[email protected]>
>
> It's sad that we need to hardcode those indexes in the Device Tree
> (which by no means are a description of the HW by the way), but that's
> what the SPI framework expects I believe. Therefore:
>
> Acked-by: Thomas Petazzoni <[email protected]>


Applied on mvebu/fixes with acked-by from Thomas.
In the same time I also applied "arm64: dts: marvell: fix clocksource
for CP110 slave SPI0" which didn't find his way to mainline yet.

Thanks,

Gregory


>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2016-11-09 08:50:40

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: marvell: add unique identifiers for Armada A8k SPI controllers

Thanks a lot!

Marcin

2016-11-09 9:47 GMT+01:00 Gregory CLEMENT <[email protected]>:
> Hi Marcin,
>
> On mar., nov. 08 2016, Thomas Petazzoni <[email protected]> wrote:
>
>> Hello,
>>
>> On Tue, 8 Nov 2016 17:31:32 +0100, Marcin Wojtas wrote:
>>> Enabling SPI controllers, which are attached to different busses
>>> inside an SoC, may result in overlapping enumeration and cause
>>> sysfs registration failure. Example log after enabling two
>>> controllers on Armada 8040 SoC with same identifiers:
>>>
>>> [ 3.740415] sysfs: cannot create duplicate filename
>>> '/class/spi_master/spi0'
>>> [ 3.747510] ------------[ cut here ]------------
>>> [ 3.752145] WARNING: at fs/sysfs/dir.c:31
>>> [...]
>>> [ 4.002299] orion_spi: probe of f4700600.spi failed with error -17
>>>
>>> spi-orion driver offers dedicated DT property ('cell-index'), that
>>> allow setting unique identifiers. Recently added support for CP110-slave
>>> HW block introduced two new SPI controllers' nodes with same ID as
>>> ones from CP110-master.
>>>
>>> This commit fixes the issue by assigning different 'cell-index' values
>>> for CP110-slave SPI controllers.
>>>
>>> Fixes: 4eef78a0091b ("arm64: dts: marvell: add description for the slave
>>> CP110 in Armada 8K")
>>> Signed-off-by: Marcin Wojtas <[email protected]>
>>
>> It's sad that we need to hardcode those indexes in the Device Tree
>> (which by no means are a description of the HW by the way), but that's
>> what the SPI framework expects I believe. Therefore:
>>
>> Acked-by: Thomas Petazzoni <[email protected]>
>
>
> Applied on mvebu/fixes with acked-by from Thomas.
> In the same time I also applied "arm64: dts: marvell: fix clocksource
> for CP110 slave SPI0" which didn't find his way to mainline yet.
>
> Thanks,
>
> Gregory
>
>
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
>
> --
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com