[PATCH] arm64: dts: imx8mn: Fix duplicate node name

[PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Shengjiu Wang]

Error log:
sysfs: cannot create duplicate filename '/bus/platform/devices/30000000.bus'

The spba bus name is duplicate with aips bus name.
Refine spba bus name to fix this issue.

Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate Converter")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index fd669c0f3fe5..30762eb4f0a7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -246,7 +246,7 @@ aips1: bus@30000000 {
 			#size-cells = <1>;
 			ranges;
 
-			spba: bus@30000000 {
+			spba: spba-bus@30000000 {
 				compatible = "fsl,spba-bus", "simple-bus";
 				#address-cells = <1>;
 				#size-cells = <1>;
-- 
2.27.0

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> Error log:
> sysfs: cannot create duplicate filename '/bus/platform/devices/30000000.bus'
> 
> The spba bus name is duplicate with aips bus name.
> Refine spba bus name to fix this issue.
> 
> Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate Converter")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index fd669c0f3fe5..30762eb4f0a7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -246,7 +246,7 @@ aips1: bus@30000000 {
>  			#size-cells = <1>;
>  			ranges;
>  
> -			spba: bus@30000000 {
> +			spba: spba-bus@30000000 {

The proper node name is "bus" so basically you introduce wrong name to
other problem.  Introducing wrong names at least requires a comment.

However the actual problem here is not in node names but in addresses:

	aips1: bus@30000000 {
		spba: bus@30000000 {

You have to devices with the same unit address. How do you share the
address space?

I think this should be rather fixed.

Best regards,
Krzysztof


>  				compatible = "fsl,spba-bus", "simple-bus";
>  				#address-cells = <1>;
>  				#size-cells = <1>;
> -- 
> 2.27.0
> 

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > Error log:
> > sysfs: cannot create duplicate filename '/bus/platform/devices/30000000.bus'
> > 
> > The spba bus name is duplicate with aips bus name.
> > Refine spba bus name to fix this issue.
> > 
> > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate Converter")
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > index fd669c0f3fe5..30762eb4f0a7 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> >  			#size-cells = <1>;
> >  			ranges;
> >  
> > -			spba: bus@30000000 {
> > +			spba: spba-bus@30000000 {
> 
> The proper node name is "bus" so basically you introduce wrong name to
> other problem.  Introducing wrong names at least requires a comment.

I just noticed that my message was barely understandable... so let me
fix it:

The proper node name is "bus" so basically you introduce wrong name to
_fix_ other problem.  Introducing wrong names at least requires a comment.

> However the actual problem here is not in node names but in addresses:
> 
> 	aips1: bus@30000000 {
> 		spba: bus@30000000 {
> 
> You have to devices with the same unit address. How do you share the
> address space?
> 
> I think this should be rather fixed.

And again, hungry keyboard ate a letter, so:

You have _two_ devices with the same unit address. How do you share the
address space?
I think this should be rather fixed.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Adam Ford]

On Mon, Dec 7, 2020 at 7:36 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > Error log:
> > > sysfs: cannot create duplicate filename '/bus/platform/devices/30000000.bus'
> > >
> > > The spba bus name is duplicate with aips bus name.
> > > Refine spba bus name to fix this issue.
> > >
> > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate Converter")
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > >                     #size-cells = <1>;
> > >                     ranges;
> > >
> > > -                   spba: bus@30000000 {
> > > +                   spba: spba-bus@30000000 {

I originally attempted to call it spba-bus@30000000, but I was told to
switch it to 'bus' but all my testing was with the spba-bus name, so I
didn't even notice the conflict.

> >
> > The proper node name is "bus" so basically you introduce wrong name to
> > other problem.  Introducing wrong names at least requires a comment.
>
> I just noticed that my message was barely understandable... so let me
> fix it:
>
> The proper node name is "bus" so basically you introduce wrong name to
> _fix_ other problem.  Introducing wrong names at least requires a comment.
>
> > However the actual problem here is not in node names but in addresses:
> >
> >       aips1: bus@30000000 {
> >               spba: bus@30000000 {
> >
> > You have to devices with the same unit address. How do you share the
> > address space?
> >
> > I think this should be rather fixed.
>
> And again, hungry keyboard ate a letter, so:
>
> You have _two_ devices with the same unit address. How do you share the
> address space?
> I think this should be rather fixed.

I am no expert on this driver, but from what I can tell, the SDMA
driver searches for the memory range assigned to the bus associated
with the spba so the DMA has an idea of which peripheral memory ranges
are shared.
from drivers/dma/imx-sdma.c:

    spba_bus = of_find_compatible_node(NULL, NULL, "fsl,spba-bus");
    ret = of_address_to_resource(spba_bus, 0, &spba_res);
    if (!ret) {
        sdma->spba_start_addr = spba_res.start;
        sdma->spba_end_addr = spba_res.end;
    }

Table 2-3 of the reference manual for the i.MX8M Nano shows the memory
range from 3000_0000 - 300F_FFFF is defined as AIPS-1 (s_b_1, via
SPBA) Glob. Module Enable which implies to me that the SPBA needs to
start at the same place as the AIPS1, but AIPS1 goes from 3000_0000 to
303F_FFFF, so the spba-bus is a subset of the AIPS1.

NXP/Freescale has a variety of SoC's that use the SPBA and in every
instance, they do it this way.  Having the spba-bus with the same
starting address of its parent aips bus isn't unique to this SoC:

imx6q.dtsi
imx6sl.dtsi
imx31.dtsi
imx7s.dtsi
imx6sll.dtsi
imx51.dtsi
imx6qdl.dtsi
imx35.dtsi
imx50.dtsi
imx25.dtsi
imx6sx.dtsi
imx6ul.dtsi
imx53.dtsi

However, in each instance, the bus associated to the SPBA is called
spba-bus and not just 'bus'

adam
>
> Best regards,
> Krzysztof
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Shawn Guo]

On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> Error log:
> sysfs: cannot create duplicate filename '/bus/platform/devices/30000000.bus'
> 
> The spba bus name is duplicate with aips bus name.
> Refine spba bus name to fix this issue.
> 
> Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample Rate Converter")
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Applied, thanks.

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[S.j. Wang]

> 
> On Tue, Dec 08, 2020 at 08:57:49AM +0000, S.j. Wang wrote:
> > > On Tue, Dec 08, 2020 at 08:44:51AM +0000, S.j. Wang wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof
> > > > > > > > Kozlowski
> > > wrote:
> > > > > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang
> wrote:
> > > > > > > > > > Error log:
> > > > > > > > > > sysfs: cannot create duplicate filename
> > > > > > > > '/bus/platform/devices/30000000.bus'
> > > > > > > > > >
> > > > > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > > > > Refine spba bus name to fix this issue.
> > > > > > > > > >
> > > > > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable
> > > > > > > > > > Asynchronous Sample Rate Converter")
> > > > > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > > > > > ---
> > > > > > > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > > > > > > >                     #size-cells = <1>;
> > > > > > > > > >                     ranges;
> > > > > > > > > >
> > > > > > > > > > -                   spba: bus@30000000 {
> > > > > > > > > > +                   spba: spba-bus@30000000 {
> > > > > > > > >
> > > > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > > > wrong name to other problem.  Introducing wrong names at
> > > > > > > > > least requires a
> > > > > comment.
> > > > > > > >
> > > > > > > > I just noticed that my message was barely
> > > > > > > > understandable... so let me
> > > > > fix it:
> > > > > > > >
> > > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > > wrong name to _fix_ other problem.  Introducing wrong
> > > > > > > > names at least
> > > > > requires a comment.
> > > > > > > >
> > > > > > > > > However the actual problem here is not in node names but
> > > > > > > > > in
> > > > > addresses:
> > > > > > > > >
> > > > > > > > >       aips1: bus@30000000 {
> > > > > > > > >               spba: bus@30000000 {
> > > > > > > > >
> > > > > > > > > You have to devices with the same unit address. How do
> > > > > > > > > you share the address space?
> > > > > > > > >
> > > > > > > > > I think this should be rather fixed.
> > > > > > > >
> > > > > > > > And again, hungry keyboard ate a letter, so:
> > > > > > > >
> > > > > > > > You have _two_ devices with the same unit address. How do
> > > > > > > > you share the address space?
> > > > > > > > I think this should be rather fixed.
> > > > > > > >
> > > > > > >
> > > > > > > spba is the first block of aips1 space, so it has same start
> > > > > > > address as aips1.
> > > > > >
> > > > > > The reference manual describes it "Reserved for SDMA2 internal
> > > > > > memory", so indeed it is first address but does it have to be mapped?
> > > > > > Anyway, why don't you use ranges to remove the conflict?
> > > > >
> > > > > The IO address space remapping could be a solution but there is
> > > > > another problem - the hardware representation in DT does not
> > > > > match what reference manual is saying.
> > > > >
> > > > > The AIPS bus @30000000 has several IPs:
> > > > >  - SAI2@30020000
> > > > >  - ...
> > > > >  - GPIO1@30200000
> > > > >
> > > > > However in DTS you will find additional SPBA bus for 30000000 -
> > > 300c0000.
> > > > > It's not really the SDMA, as SDMA is at different address. It is
> > > > > rather an address space which SDMA should map... but it is not a
> > > > > bus
> > > with children.
> > > > > Adding spba-bus@30000000 with its children does not look like
> > > > > correct representation of HW in DTS.
> > > > >
> > > >
> > > > In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
> > > > Range is (30000000 - 300FFFFF)
> > >
> > > No, AIPS-1 is till 303F_FFFF.
> >
> > Yes,  AIPSA-1 is till 303F_FFFF,  but it is divided to 2 parts.
> > (30000000 - 300FFFFF) is the first part.
> >
> > Please go to table 2-3 AIPS1 memory map in RM.  In the "region"
> > column, There is " AIPS-1 (s_b_1, via SPBA) Glob. Module Enable". It
> > means This part is connect to SPBA bus.
> 
> Thanks, I see it now. Indeed you have two buses which start at the same
> address space. You can:
> 1. Remap addresses,
> 2. Rename APIS and SPBA to bus-1 and bus-2, 3. Add specific (non-generic)
> name to spba-bus which you did initially.
> 
> All of these are rather workarounds so I don't mind your approach (3).
> 

Thanks.
I would like to keep my approach, which is to align with other i.MX platforms.

Best regards
Wang Shengjiu

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 08:57:49AM +0000, S.j. Wang wrote:
> > On Tue, Dec 08, 2020 at 08:44:51AM +0000, S.j. Wang wrote:
> > > > > > >
> > > > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski
> > wrote:
> > > > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > > > > Error log:
> > > > > > > > > sysfs: cannot create duplicate filename
> > > > > > > '/bus/platform/devices/30000000.bus'
> > > > > > > > >
> > > > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > > > Refine spba bus name to fix this issue.
> > > > > > > > >
> > > > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable
> > > > > > > > > Asynchronous Sample Rate Converter")
> > > > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > > > > ---
> > > > > > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > >
> > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > > > > > >                     #size-cells = <1>;
> > > > > > > > >                     ranges;
> > > > > > > > >
> > > > > > > > > -                   spba: bus@30000000 {
> > > > > > > > > +                   spba: spba-bus@30000000 {
> > > > > > > >
> > > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > > wrong name to other problem.  Introducing wrong names at
> > > > > > > > least requires a
> > > > comment.
> > > > > > >
> > > > > > > I just noticed that my message was barely understandable... so
> > > > > > > let me
> > > > fix it:
> > > > > > >
> > > > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > > > name to _fix_ other problem.  Introducing wrong names at least
> > > > requires a comment.
> > > > > > >
> > > > > > > > However the actual problem here is not in node names but in
> > > > addresses:
> > > > > > > >
> > > > > > > >       aips1: bus@30000000 {
> > > > > > > >               spba: bus@30000000 {
> > > > > > > >
> > > > > > > > You have to devices with the same unit address. How do you
> > > > > > > > share the address space?
> > > > > > > >
> > > > > > > > I think this should be rather fixed.
> > > > > > >
> > > > > > > And again, hungry keyboard ate a letter, so:
> > > > > > >
> > > > > > > You have _two_ devices with the same unit address. How do you
> > > > > > > share the address space?
> > > > > > > I think this should be rather fixed.
> > > > > > >
> > > > > >
> > > > > > spba is the first block of aips1 space, so it has same start
> > > > > > address as aips1.
> > > > >
> > > > > The reference manual describes it "Reserved for SDMA2 internal
> > > > > memory", so indeed it is first address but does it have to be mapped?
> > > > > Anyway, why don't you use ranges to remove the conflict?
> > > >
> > > > The IO address space remapping could be a solution but there is
> > > > another problem - the hardware representation in DT does not match
> > > > what reference manual is saying.
> > > >
> > > > The AIPS bus @30000000 has several IPs:
> > > >  - SAI2@30020000
> > > >  - ...
> > > >  - GPIO1@30200000
> > > >
> > > > However in DTS you will find additional SPBA bus for 30000000 -
> > 300c0000.
> > > > It's not really the SDMA, as SDMA is at different address. It is
> > > > rather an address space which SDMA should map... but it is not a bus
> > with children.
> > > > Adding spba-bus@30000000 with its children does not look like
> > > > correct representation of HW in DTS.
> > > >
> > >
> > > In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
> > > Range is (30000000 - 300FFFFF)
> > 
> > No, AIPS-1 is till 303F_FFFF.
> 
> Yes,  AIPSA-1 is till 303F_FFFF,  but it is divided to 2 parts.
> (30000000 - 300FFFFF) is the first part. 
> 
> Please go to table 2-3 AIPS1 memory map in RM.  In the "region" column,
> There is " AIPS-1 (s_b_1, via SPBA) Glob. Module Enable". It means
> This part is connect to SPBA bus.

Thanks, I see it now. Indeed you have two buses which start at the same
address space. You can:
1. Remap addresses,
2. Rename APIS and SPBA to bus-1 and bus-2,
3. Add specific (non-generic) name to spba-bus which you did initially.

All of these are rather workarounds so I don't mind your approach (3).

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[S.j. Wang]

> On Tue, Dec 08, 2020 at 08:44:51AM +0000, S.j. Wang wrote:
> > > > > >
> > > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski
> wrote:
> > > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > > > Error log:
> > > > > > > > sysfs: cannot create duplicate filename
> > > > > > '/bus/platform/devices/30000000.bus'
> > > > > > > >
> > > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > > Refine spba bus name to fix this issue.
> > > > > > > >
> > > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable
> > > > > > > > Asynchronous Sample Rate Converter")
> > > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > > > ---
> > > > > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > > > > >                     #size-cells = <1>;
> > > > > > > >                     ranges;
> > > > > > > >
> > > > > > > > -                   spba: bus@30000000 {
> > > > > > > > +                   spba: spba-bus@30000000 {
> > > > > > >
> > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > wrong name to other problem.  Introducing wrong names at
> > > > > > > least requires a
> > > comment.
> > > > > >
> > > > > > I just noticed that my message was barely understandable... so
> > > > > > let me
> > > fix it:
> > > > > >
> > > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > > name to _fix_ other problem.  Introducing wrong names at least
> > > requires a comment.
> > > > > >
> > > > > > > However the actual problem here is not in node names but in
> > > addresses:
> > > > > > >
> > > > > > >       aips1: bus@30000000 {
> > > > > > >               spba: bus@30000000 {
> > > > > > >
> > > > > > > You have to devices with the same unit address. How do you
> > > > > > > share the address space?
> > > > > > >
> > > > > > > I think this should be rather fixed.
> > > > > >
> > > > > > And again, hungry keyboard ate a letter, so:
> > > > > >
> > > > > > You have _two_ devices with the same unit address. How do you
> > > > > > share the address space?
> > > > > > I think this should be rather fixed.
> > > > > >
> > > > >
> > > > > spba is the first block of aips1 space, so it has same start
> > > > > address as aips1.
> > > >
> > > > The reference manual describes it "Reserved for SDMA2 internal
> > > > memory", so indeed it is first address but does it have to be mapped?
> > > > Anyway, why don't you use ranges to remove the conflict?
> > >
> > > The IO address space remapping could be a solution but there is
> > > another problem - the hardware representation in DT does not match
> > > what reference manual is saying.
> > >
> > > The AIPS bus @30000000 has several IPs:
> > >  - SAI2@30020000
> > >  - ...
> > >  - GPIO1@30200000
> > >
> > > However in DTS you will find additional SPBA bus for 30000000 -
> 300c0000.
> > > It's not really the SDMA, as SDMA is at different address. It is
> > > rather an address space which SDMA should map... but it is not a bus
> with children.
> > > Adding spba-bus@30000000 with its children does not look like
> > > correct representation of HW in DTS.
> > >
> >
> > In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
> > Range is (30000000 - 300FFFFF)
> 
> No, AIPS-1 is till 303F_FFFF.

Yes,  AIPSA-1 is till 303F_FFFF,  but it is divided to 2 parts.
(30000000 - 300FFFFF) is the first part. 

Please go to table 2-3 AIPS1 memory map in RM.  In the "region" column,
There is " AIPS-1 (s_b_1, via SPBA) Glob. Module Enable". It means
This part is connect to SPBA bus.

Best regards
Wang Shengjiu

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 08:44:51AM +0000, S.j. Wang wrote:
> > > > >
> > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > > Error log:
> > > > > > > sysfs: cannot create duplicate filename
> > > > > '/bus/platform/devices/30000000.bus'
> > > > > > >
> > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > Refine spba bus name to fix this issue.
> > > > > > >
> > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous
> > > > > > > Sample Rate Converter")
> > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > > ---
> > > > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > > > >                     #size-cells = <1>;
> > > > > > >                     ranges;
> > > > > > >
> > > > > > > -                   spba: bus@30000000 {
> > > > > > > +                   spba: spba-bus@30000000 {
> > > > > >
> > > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > > name to other problem.  Introducing wrong names at least requires a
> > comment.
> > > > >
> > > > > I just noticed that my message was barely understandable... so let me
> > fix it:
> > > > >
> > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > name to _fix_ other problem.  Introducing wrong names at least
> > requires a comment.
> > > > >
> > > > > > However the actual problem here is not in node names but in
> > addresses:
> > > > > >
> > > > > >       aips1: bus@30000000 {
> > > > > >               spba: bus@30000000 {
> > > > > >
> > > > > > You have to devices with the same unit address. How do you share
> > > > > > the address space?
> > > > > >
> > > > > > I think this should be rather fixed.
> > > > >
> > > > > And again, hungry keyboard ate a letter, so:
> > > > >
> > > > > You have _two_ devices with the same unit address. How do you
> > > > > share the address space?
> > > > > I think this should be rather fixed.
> > > > >
> > > >
> > > > spba is the first block of aips1 space, so it has same start address
> > > > as aips1.
> > >
> > > The reference manual describes it "Reserved for SDMA2 internal
> > > memory", so indeed it is first address but does it have to be mapped?
> > > Anyway, why don't you use ranges to remove the conflict?
> > 
> > The IO address space remapping could be a solution but there is another
> > problem - the hardware representation in DT does not match what reference
> > manual is saying.
> > 
> > The AIPS bus @30000000 has several IPs:
> >  - SAI2@30020000
> >  - ...
> >  - GPIO1@30200000
> > 
> > However in DTS you will find additional SPBA bus for 30000000 - 300c0000.
> > It's not really the SDMA, as SDMA is at different address. It is rather an
> > address space which SDMA should map... but it is not a bus with children.
> > Adding spba-bus@30000000 with its children does not look like correct
> > representation of HW in DTS.
> > 
> 
> In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
> Range is (30000000 - 300FFFFF)

No, AIPS-1 is till 303F_FFFF.

> 
> SPBA is a sub-bus under AIPS1. The SAI2@30020000 -  ASRC@300c0000
> Are the devices under SPBA bus.

Where did you find SPBA bus description in the Reference Manual?

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[S.j. Wang]

> > > >
> > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > Error log:
> > > > > > sysfs: cannot create duplicate filename
> > > > '/bus/platform/devices/30000000.bus'
> > > > > >
> > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > Refine spba bus name to fix this issue.
> > > > > >
> > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous
> > > > > > Sample Rate Converter")
> > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > > >                     #size-cells = <1>;
> > > > > >                     ranges;
> > > > > >
> > > > > > -                   spba: bus@30000000 {
> > > > > > +                   spba: spba-bus@30000000 {
> > > > >
> > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > name to other problem.  Introducing wrong names at least requires a
> comment.
> > > >
> > > > I just noticed that my message was barely understandable... so let me
> fix it:
> > > >
> > > > The proper node name is "bus" so basically you introduce wrong
> > > > name to _fix_ other problem.  Introducing wrong names at least
> requires a comment.
> > > >
> > > > > However the actual problem here is not in node names but in
> addresses:
> > > > >
> > > > >       aips1: bus@30000000 {
> > > > >               spba: bus@30000000 {
> > > > >
> > > > > You have to devices with the same unit address. How do you share
> > > > > the address space?
> > > > >
> > > > > I think this should be rather fixed.
> > > >
> > > > And again, hungry keyboard ate a letter, so:
> > > >
> > > > You have _two_ devices with the same unit address. How do you
> > > > share the address space?
> > > > I think this should be rather fixed.
> > > >
> > >
> > > spba is the first block of aips1 space, so it has same start address
> > > as aips1.
> >
> > The reference manual describes it "Reserved for SDMA2 internal
> > memory", so indeed it is first address but does it have to be mapped?
> > Anyway, why don't you use ranges to remove the conflict?
> 
> The IO address space remapping could be a solution but there is another
> problem - the hardware representation in DT does not match what reference
> manual is saying.
> 
> The AIPS bus @30000000 has several IPs:
>  - SAI2@30020000
>  - ...
>  - GPIO1@30200000
> 
> However in DTS you will find additional SPBA bus for 30000000 - 300c0000.
> It's not really the SDMA, as SDMA is at different address. It is rather an
> address space which SDMA should map... but it is not a bus with children.
> Adding spba-bus@30000000 with its children does not look like correct
> representation of HW in DTS.
> 

In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
Range is (30000000 - 300FFFFF)

SPBA is a sub-bus under AIPS1. The SAI2@30020000 -  ASRC@300c0000
Are the devices under SPBA bus.

So it doesn't violate RM.

Best regards
Wang shengjiu

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 09:03:34AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 08, 2020 at 03:16:35AM +0000, S.j. Wang wrote:
> > Hi
> > 
> > > 
> > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > Error log:
> > > > > sysfs: cannot create duplicate filename
> > > '/bus/platform/devices/30000000.bus'
> > > > >
> > > > > The spba bus name is duplicate with aips bus name.
> > > > > Refine spba bus name to fix this issue.
> > > > >
> > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > > > Rate Converter")
> > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > > ---
> > > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > > >                     #size-cells = <1>;
> > > > >                     ranges;
> > > > >
> > > > > -                   spba: bus@30000000 {
> > > > > +                   spba: spba-bus@30000000 {
> > > >
> > > > The proper node name is "bus" so basically you introduce wrong name to
> > > > other problem.  Introducing wrong names at least requires a comment.
> > > 
> > > I just noticed that my message was barely understandable... so let me fix it:
> > > 
> > > The proper node name is "bus" so basically you introduce wrong name to
> > > _fix_ other problem.  Introducing wrong names at least requires a comment.
> > > 
> > > > However the actual problem here is not in node names but in addresses:
> > > >
> > > >       aips1: bus@30000000 {
> > > >               spba: bus@30000000 {
> > > >
> > > > You have to devices with the same unit address. How do you share the
> > > > address space?
> > > >
> > > > I think this should be rather fixed.
> > > 
> > > And again, hungry keyboard ate a letter, so:
> > > 
> > > You have _two_ devices with the same unit address. How do you share the
> > > address space?
> > > I think this should be rather fixed.
> > > 
> > 
> > spba is the first block of aips1 space, so it has same start address as
> > aips1.
> 
> The reference manual describes it "Reserved for SDMA2 internal memory",
> so indeed it is first address but does it have to be mapped?
> Anyway, why don't you use ranges to remove the conflict?

The IO address space remapping could be a solution but there is another
problem - the hardware representation in DT does not match what
reference manual is saying.

The AIPS bus @30000000 has several IPs:
 - SAI2@30020000
 - ...
 - GPIO1@30200000

However in DTS you will find additional SPBA bus for 30000000 -
300c0000. It's not really the SDMA, as SDMA is at different address. It
is rather an address space which SDMA should map... but it is not a bus
with children. Adding spba-bus@30000000 with its children does not look
like correct representation of HW in DTS.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[Krzysztof Kozlowski]

On Tue, Dec 08, 2020 at 03:16:35AM +0000, S.j. Wang wrote:
> Hi
> 
> > 
> > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > Error log:
> > > > sysfs: cannot create duplicate filename
> > '/bus/platform/devices/30000000.bus'
> > > >
> > > > The spba bus name is duplicate with aips bus name.
> > > > Refine spba bus name to fix this issue.
> > > >
> > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > > Rate Converter")
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > > >                     #size-cells = <1>;
> > > >                     ranges;
> > > >
> > > > -                   spba: bus@30000000 {
> > > > +                   spba: spba-bus@30000000 {
> > >
> > > The proper node name is "bus" so basically you introduce wrong name to
> > > other problem.  Introducing wrong names at least requires a comment.
> > 
> > I just noticed that my message was barely understandable... so let me fix it:
> > 
> > The proper node name is "bus" so basically you introduce wrong name to
> > _fix_ other problem.  Introducing wrong names at least requires a comment.
> > 
> > > However the actual problem here is not in node names but in addresses:
> > >
> > >       aips1: bus@30000000 {
> > >               spba: bus@30000000 {
> > >
> > > You have to devices with the same unit address. How do you share the
> > > address space?
> > >
> > > I think this should be rather fixed.
> > 
> > And again, hungry keyboard ate a letter, so:
> > 
> > You have _two_ devices with the same unit address. How do you share the
> > address space?
> > I think this should be rather fixed.
> > 
> 
> spba is the first block of aips1 space, so it has same start address as
> aips1.

The reference manual describes it "Reserved for SDMA2 internal memory",
so indeed it is first address but does it have to be mapped?
Anyway, why don't you use ranges to remove the conflict?

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name

[S.j. Wang]

Hi

> 
> On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > Error log:
> > > sysfs: cannot create duplicate filename
> '/bus/platform/devices/30000000.bus'
> > >
> > > The spba bus name is duplicate with aips bus name.
> > > Refine spba bus name to fix this issue.
> > >
> > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > Rate Converter")
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > @@ -246,7 +246,7 @@ aips1: bus@30000000 {
> > >                     #size-cells = <1>;
> > >                     ranges;
> > >
> > > -                   spba: bus@30000000 {
> > > +                   spba: spba-bus@30000000 {
> >
> > The proper node name is "bus" so basically you introduce wrong name to
> > other problem.  Introducing wrong names at least requires a comment.
> 
> I just noticed that my message was barely understandable... so let me fix it:
> 
> The proper node name is "bus" so basically you introduce wrong name to
> _fix_ other problem.  Introducing wrong names at least requires a comment.
> 
> > However the actual problem here is not in node names but in addresses:
> >
> >       aips1: bus@30000000 {
> >               spba: bus@30000000 {
> >
> > You have to devices with the same unit address. How do you share the
> > address space?
> >
> > I think this should be rather fixed.
> 
> And again, hungry keyboard ate a letter, so:
> 
> You have _two_ devices with the same unit address. How do you share the
> address space?
> I think this should be rather fixed.
> 

spba is the first block of aips1 space, so it has same start address as
aips1.

Best regards
Wang shengjiu