[PATCH] arm64: dts: meson: fix g12a buses

[PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

periph and hiu bus addresses/size are wrong.
cbus, aobus and apb just don't exist in the memory map so remove them.

Fixes: 9c8c52f7cb4f ("arm64: dts: meson-g12a: add initial g12a s905d2 SoC DT support")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-g12a.dtsi | 70 +++++++--------------
 1 file changed, 24 insertions(+), 46 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
index 3b82a975c663..2aea5ba62fee 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a.dtsi
@@ -78,46 +78,40 @@
 		#size-cells = <2>;
 		ranges;
 
-		periphs: periphs@ff634000 {
+		periphs: bus@ff634400 {
 			compatible = "simple-bus";
-			reg = <0x0 0xff634000 0x0 0x2000>;
+			reg = <0x0 0xff634400 0x0 0x400>;
 			#address-cells = <2>;
 			#size-cells = <2>;
-			ranges = <0x0 0x0 0x0 0xff634000 0x0 0x2000>;
+			ranges = <0x0 0x0 0x0 0xff634400 0x0 0x400>;
 		};
 
-		hiubus: bus@ff63c000 {
+		hiu: bus@ff63c000 {
 			compatible = "simple-bus";
-			reg = <0x0 0xff63c000 0x0 0x1c00>;
+			reg = <0x0 0xff63c000 0x0 0x1400>;
 			#address-cells = <2>;
 			#size-cells = <2>;
-			ranges = <0x0 0x0 0x0 0xff63c000 0x0 0x1c00>;
+			ranges = <0x0 0x0 0x0 0xff63c000 0x0 0x1400>;
 		};
 
-		aobus: bus@ff800000 {
-			compatible = "simple-bus";
-			reg = <0x0 0xff800000 0x0 0x100000>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges = <0x0 0x0 0x0 0xff800000 0x0 0x100000>;
-
-			uart_AO: serial@3000 {
-				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
-				reg = <0x0 0x3000 0x0 0x18>;
-				interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>, <&xtal>, <&xtal>;
-				clock-names = "xtal", "pclk", "baud";
-				status = "disabled";
-			};
-
-			uart_AO_B: serial@4000 {
-				compatible = "amlogic,meson-gx-uart", "amlogic,meson-ao-uart";
-				reg = <0x0 0x4000 0x0 0x18>;
-				interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
-				clocks = <&xtal>, <&xtal>, <&xtal>;
-				clock-names = "xtal", "pclk", "baud";
-				status = "disabled";
-			};
+		uart_AO: serial@ff803000 {
+			compatible = "amlogic,meson-gx-uart",
+				     "amlogic,meson-ao-uart";
+			reg = <0x0 0xff803000 0x0 0x18>;
+			interrupts = <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&xtal>, <&xtal>, <&xtal>;
+			clock-names = "xtal", "pclk", "baud";
+			status = "disabled";
+		};
+
+		uart_AO_B: serial@ff804000 {
+			compatible = "amlogic,meson-gx-uart",
+				     "amlogic,meson-ao-uart";
+			reg = <0x0 0xff804000 0x0 0x18>;
+			interrupts = <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&xtal>, <&xtal>, <&xtal>;
+			clock-names = "xtal", "pclk", "baud";
+			status = "disabled";
 		};
 
 		gic: interrupt-controller@ffc01000 {
@@ -132,22 +126,6 @@
 			#interrupt-cells = <3>;
 			#address-cells = <0>;
 		};
-
-		cbus: bus@ffd00000 {
-			compatible = "simple-bus";
-			reg = <0x0 0xffd00000 0x0 0x25000>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges = <0x0 0x0 0x0 0xffd00000 0x0 0x25000>;
-		};
-
-		apb: apb@ffe00000 {
-			compatible = "simple-bus";
-			reg = <0x0 0xffe00000 0x0 0x200000>;
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges = <0x0 0x0 0x0 0xffe00000 0x0 0x200000>;
-		};
 	};
 
 	timer {
-- 
2.20.1

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> periph and hiu bus addresses/size are wrong.
> cbus, aobus and apb just don't exist in the memory map so remove them.
>
> Fixes: 9c8c52f7cb4f ("arm64: dts: meson-g12a: add initial g12a s905d2 SoC DT support")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Queuing as a fix for v5.0-rc,

Thanks,

Kevin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Kevin Hilman]

Jerome Brunet <jbrunet@baylibre.com> writes:

> periph and hiu bus addresses/size are wrong.
> cbus, aobus and apb just don't exist in the memory map so remove them.
>
> Fixes: 9c8c52f7cb4f ("arm64: dts: meson-g12a: add initial g12a s905d2 SoC DT support")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Queued as a fix for v5.0-rc (branch: v5.0/fixes)

Kevin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Martin Blumenstingl]

Hi Jerome,

On Wed, Jan 16, 2019 at 5:52 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> -               aobus: bus@ff800000 {
> -                       compatible = "simple-bus";
> -                       reg = <0x0 0xff800000 0x0 0x100000>;
are you sure about removing aobus?
in your patch "arm64: dts: meson: g12a: add pinctrl support
controllers" from [0] you're adding back an "rti" bus at the same
memory address, except that it's size is 0x1000 instead of 0x100000

I don't have any G12A datasheet so it's hard for me to ACK / NACK this
patch, here are some clues why I'm asking:
- GXM's public datasheet lists the "RTI" region at 0xC8100000 -
0xC81FFFFF, we call it "aobus" in our .dtsi
- buildroot_openlinux_kernel_4.9_fbdev_20180706/kernel/aml-4.9/arch/arm64/boot/dts/amlogic/mesong12a.dtsi
also uses an aobus node as well as an io_aobus_base sub-node for the
codec_io node (the latter is obviously not upstream)


Regards
Martin


[0] https://lore.kernel.org/patchwork/patch/1032981/

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

On Thu, 2019-01-17 at 21:03 +0100, Martin Blumenstingl wrote:
> Hi Jerome,
> 
> On Wed, Jan 16, 2019 at 5:52 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
> > -               aobus: bus@ff800000 {
> > -                       compatible = "simple-bus";
> > -                       reg = <0x0 0xff800000 0x0 0x100000>;
> are you sure about removing aobus?
> in your patch "arm64: dts: meson: g12a: add pinctrl support
> controllers" from [0] you're adding back an "rti" bus at the same
> memory address, except that it's size is 0x1000 instead of 0x100000

Yes, I'm sure. The fact that the aobus region size is completly made up, for
all I know, is the reason 

> 
> I don't have any G12A datasheet so it's hard for me to ACK / NACK this
> patch, here are some clues why I'm asking:
> - GXM's public datasheet lists the "RTI" region at 0xC8100000 -
> 0xC81FFFFF, we call it "aobus" in our .dtsi

... RTI region which will appear once i submit the clock controller

Yes GXM (and GXBB, GXL or AXG) has the same made up region with nothing
matching in the datasheet, but it is bit late fix that now.

> - buildroot_openlinux_kernel_4.9_fbdev_20180706/kernel/aml-
> 4.9/arch/arm64/boot/dts/amlogic/mesong12a.dtsi
> also uses an aobus node as well as an io_aobus_base sub-node for the
> codec_io node (the latter is obviously not upstream)

I know, but again, it does not map to anything in the doc, and it has been the
case for several SoC.

> 
> Regards
> Martin
> 
> 
> [0] https://lore.kernel.org/patchwork/patch/1032981/

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Martin Blumenstingl]

Hi Jerome,

On Thu, Jan 17, 2019 at 9:12 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Thu, 2019-01-17 at 21:03 +0100, Martin Blumenstingl wrote:
> > Hi Jerome,
> >
> > On Wed, Jan 16, 2019 at 5:52 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > [...]
> > > -               aobus: bus@ff800000 {
> > > -                       compatible = "simple-bus";
> > > -                       reg = <0x0 0xff800000 0x0 0x100000>;
> > are you sure about removing aobus?
> > in your patch "arm64: dts: meson: g12a: add pinctrl support
> > controllers" from [0] you're adding back an "rti" bus at the same
> > memory address, except that it's size is 0x1000 instead of 0x100000
>
> Yes, I'm sure. The fact that the aobus region size is completly made up, for
> all I know, is the reason
>
> >
> > I don't have any G12A datasheet so it's hard for me to ACK / NACK this
> > patch, here are some clues why I'm asking:
> > - GXM's public datasheet lists the "RTI" region at 0xC8100000 -
> > 0xC81FFFFF, we call it "aobus" in our .dtsi
>
> ... RTI region which will appear once i submit the clock controller
I assume you're speaking of G12A and not about GXM:
you already need it for your G12A pinctrl patches, which is why you
add it in: [0]

> Yes GXM (and GXBB, GXL or AXG) has the same made up region with nothing
> matching in the datasheet, but it is bit late fix that now.
looking at the datasheet: it seems that there's only a name mismatch
(aobus in our .dtsi, "rti" in the datasheet)
offset and size correspond with the datasheet

> > - buildroot_openlinux_kernel_4.9_fbdev_20180706/kernel/aml-
> > 4.9/arch/arm64/boot/dts/amlogic/mesong12a.dtsi
> > also uses an aobus node as well as an io_aobus_base sub-node for the
> > codec_io node (the latter is obviously not upstream)
>
> I know, but again, it does not map to anything in the doc, and it has been the
> case for several SoC.
OK, but we had incorrect documentation in the past. did you check this
with someone from Amlogic?

I'm curious because there seem to be two different approaches here:
1) hiubus name and offsets are being fixed within this patch
2) aobus is being dropped here and re-introduced with a different name later on

approach 1) can also be used for the "rti" region (at least in my
opinion, the patch doesn't explain why it can't be done):
rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
(both values can be found in mesong12a.dtsi from
buildroot_openlinux_kernel_4.9_fbdev_20180706)


Regards
Martin


[0] https://lore.kernel.org/patchwork/patch/1032981/

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> OK, but we had incorrect documentation in the past. did you check this
> with someone from Amlogic?
> 
> I'm curious because there seem to be two different approaches here:
> 1) hiubus name and offsets are being fixed within this patch
> 2) aobus is being dropped here and re-introduced with a different name later
> on
> 

because hiu exist and aobus does not, for which both the name and size was
wrong

> approach 1) can also be used for the "rti" region (at least in my
> opinion, the patch doesn't explain why it can't be done):

THe patch remove aobus (instead of fixing name and size) because, of the
multiple region documented covered by this 'made region', I did not anticipate
which one will be required and I did not want to add them all.

Better to add them as needed, which is want I done for pinctrl as you pointed
out

> rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
> (both values can be found in mesong12a.dtsi from
> buildroot_openlinux_kernel_4.9_fbdev_20180706)

RTI is added here:
https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com

I don't really understand the problem ? result is the same

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Martin Blumenstingl]

On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
> On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> > OK, but we had incorrect documentation in the past. did you check this
> > with someone from Amlogic?
> >
> > I'm curious because there seem to be two different approaches here:
> > 1) hiubus name and offsets are being fixed within this patch
> > 2) aobus is being dropped here and re-introduced with a different name later
> > on
> >
>
> because hiu exist and aobus does not, for which both the name and size was
> wrong
>
> > approach 1) can also be used for the "rti" region (at least in my
> > opinion, the patch doesn't explain why it can't be done):
>
> THe patch remove aobus (instead of fixing name and size) because, of the
> multiple region documented covered by this 'made region', I did not anticipate
> which one will be required and I did not want to add them all.
>
> Better to add them as needed, which is want I done for pinctrl as you pointed
> out
>
> > rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
> > (both values can be found in mesong12a.dtsi from
> > buildroot_openlinux_kernel_4.9_fbdev_20180706)
>
> RTI is added here:
> https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
>
> I don't really understand the problem ? result is the same
the actual problem is "me" as I have conflicting information:
- Amlogic's buildroot kernel (for G12A) uses similar bus definitions
as the GX SoCs (for which there are public datasheets) - this is how
Jianxin added it to meson-g12a.dtsi originally
- this patch does it different - but cannot check if this is correct
(no public datasheet is available for G12A or AXG) nor do I have a
"big picture" of upcoming changes

Cc'ing Jianxin: can you please review Jerome's patch and give some
more details on the memory map on G12A so further contributions can be
reviewed easier?


Regards
Martin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

On Thu, 2019-01-17 at 22:20 +0100, Martin Blumenstingl wrote:
> On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> > > OK, but we had incorrect documentation in the past. did you check this
> > > with someone from Amlogic?
> > > 
> > > I'm curious because there seem to be two different approaches here:
> > > 1) hiubus name and offsets are being fixed within this patch
> > > 2) aobus is being dropped here and re-introduced with a different name
> > > later
> > > on
> > > 
> > 
> > because hiu exist and aobus does not, for which both the name and size was
> > wrong
> > 
> > > approach 1) can also be used for the "rti" region (at least in my
> > > opinion, the patch doesn't explain why it can't be done):
> > 
> > THe patch remove aobus (instead of fixing name and size) because, of the
> > multiple region documented covered by this 'made region', I did not
> > anticipate
> > which one will be required and I did not want to add them all.
> > 
> > Better to add them as needed, which is want I done for pinctrl as you
> > pointed
> > out
> > 
> > > rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
> > > (both values can be found in mesong12a.dtsi from
> > > buildroot_openlinux_kernel_4.9_fbdev_20180706)
> > 
> > RTI is added here:
> > https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
> > 
> > I don't really understand the problem ? result is the same
> the actual problem is "me" as I have conflicting information:
> - Amlogic's buildroot kernel (for G12A) uses similar bus definitions
> as the GX SoCs (for which there are public datasheets) - this is how
> Jianxin added it to meson-g12a.dtsi originally

And it was the same for the GX family. AOBUS in the DT while there nothing
about this in the memory map. Keep wrong patterns does not make them right.

I'm merely reading the memory map here

> - this patch does it different - but cannot check if this is correct
> (no public datasheet is available for G12A or AXG) nor do I have a
> "big picture" of upcoming changes

Yes it does it differently. We should have picked up on this a while ago,
since gxbb at least, and we did not. There reason to create bus that don't
exist in the memory map of any recent SoC.

Creating bus should at least require a start and end offset explained
somewhere. Copying vendor DT is not enough

> 
> Cc'ing Jianxin: can you please review Jerome's patch and give some
> more details on the memory map on G12A so further contributions can be
> reviewed easier?
> 
> 
> Regards
> Martin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Martin Blumenstingl]

On Thu, Jan 17, 2019 at 10:45 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
> On Thu, 2019-01-17 at 22:20 +0100, Martin Blumenstingl wrote:
> > On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > > On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> > > > OK, but we had incorrect documentation in the past. did you check this
> > > > with someone from Amlogic?
> > > >
> > > > I'm curious because there seem to be two different approaches here:
> > > > 1) hiubus name and offsets are being fixed within this patch
> > > > 2) aobus is being dropped here and re-introduced with a different name
> > > > later
> > > > on
> > > >
> > >
> > > because hiu exist and aobus does not, for which both the name and size was
> > > wrong
> > >
> > > > approach 1) can also be used for the "rti" region (at least in my
> > > > opinion, the patch doesn't explain why it can't be done):
> > >
> > > THe patch remove aobus (instead of fixing name and size) because, of the
> > > multiple region documented covered by this 'made region', I did not
> > > anticipate
> > > which one will be required and I did not want to add them all.
> > >
> > > Better to add them as needed, which is want I done for pinctrl as you
> > > pointed
> > > out
> > >
> > > > rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
> > > > (both values can be found in mesong12a.dtsi from
> > > > buildroot_openlinux_kernel_4.9_fbdev_20180706)
> > >
> > > RTI is added here:
> > > https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
> > >
> > > I don't really understand the problem ? result is the same
> > the actual problem is "me" as I have conflicting information:
> > - Amlogic's buildroot kernel (for G12A) uses similar bus definitions
> > as the GX SoCs (for which there are public datasheets) - this is how
> > Jianxin added it to meson-g12a.dtsi originally
>
> And it was the same for the GX family. AOBUS in the DT while there nothing
> about this in the memory map. Keep wrong patterns does not make them right.
>
> I'm merely reading the memory map here
can you explain which "memory map" you are reading exactly?

my source is the "memory map" section in the public S905X and S912 datasheets:
- "S912_Datasheet_V0.220170314publicversion-Wesion.pdf" page 48
- "S905X_Datasheet V0.3 20170314publicversion-Wesion.pdf" page 43

> > - this patch does it different - but cannot check if this is correct
> > (no public datasheet is available for G12A or AXG) nor do I have a
> > "big picture" of upcoming changes
>
> Yes it does it differently. We should have picked up on this a while ago,
> since gxbb at least, and we did not. There reason to create bus that don't
> exist in the memory map of any recent SoC.
in the GXL and GXM datasheets from above there is:
range: 0xC8100000 - 0xC81FFFFF
name: RTI

thus in my opinion, based on the two datasheets from above:
- the bus definition (start, size) in meson-gx.dtsi is fine
- probably have the wrong name for this bus in meson-gx.dtsi
- (assuming that GXBB, GXL and GXM are all the same - I only compared
the last two)

> Creating bus should at least require a start and end offset explained
> somewhere. Copying vendor DT is not enough
I fully agree on this one!


Regards
Martin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

On Thu, 2019-01-17 at 23:42 +0100, Martin Blumenstingl wrote:
> On Thu, Jan 17, 2019 at 10:45 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Thu, 2019-01-17 at 22:20 +0100, Martin Blumenstingl wrote:
> > > On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com>
> > > wrote:
> > > > On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> > > > > OK, but we had incorrect documentation in the past. did you check
> > > > > this
> > > > > with someone from Amlogic?
> > > > > 
> > > > > I'm curious because there seem to be two different approaches here:
> > > > > 1) hiubus name and offsets are being fixed within this patch
> > > > > 2) aobus is being dropped here and re-introduced with a different
> > > > > name
> > > > > later
> > > > > on
> > > > > 
> > > > 
> > > > because hiu exist and aobus does not, for which both the name and size
> > > > was
> > > > wrong
> > > > 
> > > > > approach 1) can also be used for the "rti" region (at least in my
> > > > > opinion, the patch doesn't explain why it can't be done):
> > > > 
> > > > THe patch remove aobus (instead of fixing name and size) because, of
> > > > the
> > > > multiple region documented covered by this 'made region', I did not
> > > > anticipate
> > > > which one will be required and I did not want to add them all.
> > > > 
> > > > Better to add them as needed, which is want I done for pinctrl as you
> > > > pointed
> > > > out
> > > > 
> > > > > rename "aobus" to "rti" and change the size to either 0x1000 or
> > > > > 0xb000
> > > > > (both values can be found in mesong12a.dtsi from
> > > > > buildroot_openlinux_kernel_4.9_fbdev_20180706)
> > > > 
> > > > RTI is added here:
> > > > https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
> > > > 
> > > > I don't really understand the problem ? result is the same
> > > the actual problem is "me" as I have conflicting information:
> > > - Amlogic's buildroot kernel (for G12A) uses similar bus definitions
> > > as the GX SoCs (for which there are public datasheets) - this is how
> > > Jianxin added it to meson-g12a.dtsi originally
> > 
> > And it was the same for the GX family. AOBUS in the DT while there nothing
> > about this in the memory map. Keep wrong patterns does not make them
> > right.
> > 
> > I'm merely reading the memory map here
> can you explain which "memory map" you are reading exactly?
> 
> my source is the "memory map" section in the public S905X and S912
> datasheets:
> - "S912_Datasheet_V0.220170314publicversion-Wesion.pdf" page 48
> - "S905X_Datasheet V0.3 20170314publicversion-Wesion.pdf" page 43

Those section yes

> 
> > > - this patch does it different - but cannot check if this is correct
> > > (no public datasheet is available for G12A or AXG) nor do I have a
> > > "big picture" of upcoming changes
> > 
> > Yes it does it differently. We should have picked up on this a while ago,
> > since gxbb at least, and we did not. There reason to create bus that don't
> > exist in the memory map of any recent SoC.
> in the GXL and GXM datasheets from above there is:
> range: 0xC8100000 - 0xC81FFFFF
> name: RTI

So we apparently agree that aobus does not exist

RTI simply goes fron 0xff800000 - 0xff800fff on the G12a doc, it is as simple
as that - as done in the pinctrl.

> 
> thus in my opinion, based on the two datasheets from above:
> - the bus definition (start, size) in meson-gx.dtsi is fine
> - probably have the wrong name for this bus in meson-gx.dtsi
> - (assuming that GXBB, GXL and GXM are all the same - I only compared
> the last two)

I still dont understand this focus on aobus which simply does not exist in the
map, whether it is by name of size or offset. Same goes cbus and apb.

> 
> > Creating bus should at least require a start and end offset explained
> > somewhere. Copying vendor DT is not enough
> I fully agree on this one!
> 
> 
> Regards
> Martin

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jianxin Pan]

Hi Martin and Jerome,

On 2019/1/18 5:20, Martin Blumenstingl wrote:
> On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>>
>> On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
>>> OK, but we had incorrect documentation in the past. did you check this
>>> with someone from Amlogic?
>>>
>>> I'm curious because there seem to be two different approaches here:
>>> 1) hiubus name and offsets are being fixed within this patch
>>> 2) aobus is being dropped here and re-introduced with a different name later
>>> on
>>>
>>
>> because hiu exist and aobus does not, for which both the name and size was
>> wrong
>>
>>> approach 1) can also be used for the "rti" region (at least in my
>>> opinion, the patch doesn't explain why it can't be done):
>>
>> THe patch remove aobus (instead of fixing name and size) because, of the
>> multiple region documented covered by this 'made region', I did not anticipate
>> which one will be required and I did not want to add them all.
>>
>> Better to add them as needed, which is want I done for pinctrl as you pointed
>> out
>>
>>> rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
>>> (both values can be found in mesong12a.dtsi from
>>> buildroot_openlinux_kernel_4.9_fbdev_20180706)
>>
>> RTI is added here:
>> https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
>>
>> I don't really understand the problem ? result is the same
> the actual problem is "me" as I have conflicting information:
> - Amlogic's buildroot kernel (for G12A) uses similar bus definitions
> as the GX SoCs (for which there are public datasheets) - this is how
> Jianxin added it to meson-g12a.dtsi originally
> - this patch does it different - but cannot check if this is correct
> (no public datasheet is available for G12A or AXG) nor do I have a
> "big picture" of upcoming changes
> 
> Cc'ing Jianxin: can you please review Jerome's patch and give some
> more details on the memory map on G12A so further contributions can be
> reviewed easier?
> 
1. "aobus: bus@ff800000" describes the following registers:
ahb	ao_reg	reserved	980	FF80B000
ahb	ao_reg	ao_mailbox	4	FF80A000
ahb	ao_reg	sar_adc		4	FF809000
ahb	ao_reg	ir_dec		4	FF808000
ahb	ao_reg	pwm_ab		4	FF807000
ahb	ao_reg	i2c_s		4	FF806000
ahb	ao_reg	i2c_m		4	FF805000
ahb	ao_reg	uart2		4	FF804000
ahb	ao_reg	uart		4	FF803000
ahb	ao_reg	pwm_cd		4	FF802000
ahb	ao_reg	reserved	4	FF801000
ahb	ao_reg	rti		4	FF800000


2. "cbus: bus@ffd00000" describes the following registers:
capb3	cbus	reserved	872	FFD26000	FFDFFFFF
capb3	cbus	sc		4	FFD25000	FFD25FFF
capb3	cbus	uart0		4	FFD24000	FFD24FFF
capb3	cbus	uart1		4	FFD23000	FFD23FFF
capb3	cbus	uart2		4	FFD22000	FFD22FFF
capb3	cbus	reserved	4	FFD21000	FFD21FFF
capb3	cbus	reserved	4	FFD20000	FFD20FFF
capb3	cbus	i2c_m0		4	FFD1F000	FFD1FFFF
capb3	cbus	i2c_m1		4	FFD1E000	FFD1EFFF
capb3	cbus	i2c_m2		4	FFD1D000	FFD1DFFF
capb3	cbus	i2c_m3		4	FFD1C000	FFD1CFFF
capb3	cbus	pwm_ab		4	FFD1B000	FFD1BFFF
capb3	cbus	pwm_cd		4	FFD1A000	FFD1AFFF
capb3	cbus	pwm_ef		4	FFD19000	FFD19FFF
capb3	cbus	msr_clk		4	FFD18000	FFD18FFF
capb3	cbus	reserved	4	FFD17000	FFD17FFF
capb3	cbus	reserved	4	FFD16000	FFD16FFF
capb3	cbus	spicc_1		4	FFD15000	FFD15FFF
capb3	cbus	spifc		4	FFD14000	FFD14FFF
capb3	cbus	spicc_0		4	FFD13000	FFD13FFF
capb3	cbus	reserved	4	FFD12000	FFD12FFF
capb3	cbus	reserved	4	FFD11000	FFD11FFF
capb3	cbus	reserved	4	FFD10000	FFD10FFF
capb3	cbus	isa		4	FFD0F000	FFD0FFFF
capb3	cbus	parser		4	FFD0E000	FFD0EFFF
capb3	cbus	reserved	4	FFD0D000	FFD0DFFF
capb3	cbus	sana		4	FFD0C000	FFD0CFFF
capb3	cbus	stream		4	FFD0B000	FFD0BFFF
capb3	cbus	async_fifo	4	FFD0A000	FFD0AFFF
capb3	cbus	async_fifo2	4	FFD09000	FFD09FFF
capb3	cbus	assist		4	FFD08000	FFD08FFF
capb3	cbus	mipi_dsi_host	4	FFD07000	FFD07FFF
capb3	cbus	stb		4	FFD06000	FFD06FFF
capb3	cbus	aififo		4	FFD05000	FFD05FFF
capb3	cbus	reserved	4	FFD04000	FFD04FFF
capb3	cbus	reserved	4	FFD03000	FFD03FFF
capb3	cbus	reserved	4	FFD02000	FFD02FFF
capb3	cbus	reset		4	FFD01000	FFD01FFF
capb3	cbus	reserved	4	FFD00000	FFD00FFF

3. In public data sheet, I can only found memory map about sub-regions of each bus. 
And no information about the bus itself.

> 
> Regards
> Martin
> 
> .
> 

Re: [PATCH] arm64: dts: meson: fix g12a buses

[Jerome Brunet]

On Sat, 2019-01-19 at 02:09 +0800, Jianxin Pan wrote:
> Hi Martin and Jerome,
> 
> On 2019/1/18 5:20, Martin Blumenstingl wrote:
> > On Thu, Jan 17, 2019 at 9:39 PM Jerome Brunet <jbrunet@baylibre.com>
> > wrote:
> > > On Thu, 2019-01-17 at 21:27 +0100, Martin Blumenstingl wrote:
> > > > OK, but we had incorrect documentation in the past. did you check this
> > > > with someone from Amlogic?
> > > > 
> > > > I'm curious because there seem to be two different approaches here:
> > > > 1) hiubus name and offsets are being fixed within this patch
> > > > 2) aobus is being dropped here and re-introduced with a different name
> > > > later
> > > > on
> > > > 
> > > 
> > > because hiu exist and aobus does not, for which both the name and size
> > > was
> > > wrong
> > > 
> > > > approach 1) can also be used for the "rti" region (at least in my
> > > > opinion, the patch doesn't explain why it can't be done):
> > > 
> > > THe patch remove aobus (instead of fixing name and size) because, of the
> > > multiple region documented covered by this 'made region', I did not
> > > anticipate
> > > which one will be required and I did not want to add them all.
> > > 
> > > Better to add them as needed, which is want I done for pinctrl as you
> > > pointed
> > > out
> > > 
> > > > rename "aobus" to "rti" and change the size to either 0x1000 or 0xb000
> > > > (both values can be found in mesong12a.dtsi from
> > > > buildroot_openlinux_kernel_4.9_fbdev_20180706)
> > > 
> > > RTI is added here:
> > > https://lkml.kernel.org/r/20190117103151.3349-1-jbrunet@baylibre.com
> > > 
> > > I don't really understand the problem ? result is the same
> > the actual problem is "me" as I have conflicting information:
> > - Amlogic's buildroot kernel (for G12A) uses similar bus definitions
> > as the GX SoCs (for which there are public datasheets) - this is how
> > Jianxin added it to meson-g12a.dtsi originally
> > - this patch does it different - but cannot check if this is correct
> > (no public datasheet is available for G12A or AXG) nor do I have a
> > "big picture" of upcoming changes
> > 
> > Cc'ing Jianxin: can you please review Jerome's patch and give some
> > more details on the memory map on G12A so further contributions can be
> > reviewed easier?
> > 
> 1. "aobus: bus@ff800000" describes the following registers:
> ahb	ao_reg	reserved	980	FF80B000
> ahb	ao_reg	ao_mailbox	4	FF80A000
> ahb	ao_reg	sar_adc		4	FF809000
> ahb	ao_reg	ir_dec		4	FF808000
> ahb	ao_reg	pwm_ab		4	FF807000
> ahb	ao_reg	i2c_s		4	FF806000
> ahb	ao_reg	i2c_m		4	FF805000
> ahb	ao_reg	uart2		4	FF804000
> ahb	ao_reg	uart		4	FF803000
> ahb	ao_reg	pwm_cd		4	FF802000
> ahb	ao_reg	reserved	4	FF801000
> ahb	ao_reg	rti		4	FF800000
> 
> 
> 2. "cbus: bus@ffd00000" describes the following registers:
> capb3	cbus	reserved	872	FFD26000	FFDFFFFF
> capb3	cbus	sc		4	FFD25000	FFD25FFF
> capb3	cbus	uart0		4	FFD24000	FFD24FFF
> capb3	cbus	uart1		4	FFD23000	FFD23FFF
> capb3	cbus	uart2		4	FFD22000	FFD22FFF
> capb3	cbus	reserved	4	FFD21000	FFD21FFF
> capb3	cbus	reserved	4	FFD20000	FFD20FFF
> capb3	cbus	i2c_m0		4	FFD1F000	FFD1FFFF
> capb3	cbus	i2c_m1		4	FFD1E000	FFD1EFFF
> capb3	cbus	i2c_m2		4	FFD1D000	FFD1DFFF
> capb3	cbus	i2c_m3		4	FFD1C000	FFD1CFFF
> capb3	cbus	pwm_ab		4	FFD1B000	FFD1BFFF
> capb3	cbus	pwm_cd		4	FFD1A000	FFD1AFFF
> capb3	cbus	pwm_ef		4	FFD19000	FFD19FFF
> capb3	cbus	msr_clk		4	FFD18000	FFD18FFF
> capb3	cbus	reserved	4	FFD17000	FFD17FFF
> capb3	cbus	reserved	4	FFD16000	FFD16FFF
> capb3	cbus	spicc_1		4	FFD15000	FFD15FFF
> capb3	cbus	spifc		4	FFD14000	FFD14FFF
> capb3	cbus	spicc_0		4	FFD13000	FFD13FFF
> capb3	cbus	reserved	4	FFD12000	FFD12FFF
> capb3	cbus	reserved	4	FFD11000	FFD11FFF
> capb3	cbus	reserved	4	FFD10000	FFD10FFF
> capb3	cbus	isa		4	FFD0F000	FFD0FFFF
> capb3	cbus	parser		4	FFD0E000	FFD0EFFF
> capb3	cbus	reserved	4	FFD0D000	FFD0DFFF
> capb3	cbus	sana		4	FFD0C000	FFD0CFFF
> capb3	cbus	stream		4	FFD0B000	FFD0BFFF
> capb3	cbus	async_fifo	4	FFD0A000	FFD0AFFF
> capb3	cbus	async_fifo2	4	FFD09000	FFD09FFF
> capb3	cbus	assist		4	FFD08000	FFD08FFF
> capb3	cbus	mipi_dsi_host	4	FFD07000	FFD07FFF
> capb3	cbus	stb		4	FFD06000	FFD06FFF
> capb3	cbus	aififo		4	FFD05000	FFD05FFF
> capb3	cbus	reserved	4	FFD04000	FFD04FFF
> capb3	cbus	reserved	4	FFD03000	FFD03FFF
> capb3	cbus	reserved	4	FFD02000	FFD02FFF
> capb3	cbus	reset		4	FFD01000	FFD01FFF
> capb3	cbus	reserved	4	FFD00000	FFD00FFF

Hu Jianxin,

Thanks for sharing this. Any other region we should know about ? what about
the apb region that isn't documented either ?

> 
> 3. In public data sheet, I can only found memory map about sub-regions of
> each bus. 
> And no information about the bus itself.
> 
> > Regards
> > Martin
> > 
> > .
> >