| Message ID | 1567667251-33466-5-git-send-email-jianxin.pan@amlogic.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: Add basic support for Amlogic A1 SoC Family | expand |
Hi Jianxin, (it's great to see that you and your team are upstreaming this early) On Thu, Sep 5, 2019 at 9:08 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: [...] > + memory@0 { > + device_type = "memory"; > + reg = <0x0 0x0 0x0 0x8000000>; > + /*linux,usable-memory = <0x0 0x0 0x0 0x8000000>;*/ why do we need that comment here (I don't understand it - why doesn't the "reg" property cover this)? > + }; > +}; > + > +&uart_AO_B { > + status = "okay"; > +}; > diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi > new file mode 100644 > index 00000000..4d476ac > --- /dev/null > +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi > @@ -0,0 +1,122 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. > + */ > + > +#include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > + > +/ { > + compatible = "amlogic,a1"; > + > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpus { > + #address-cells = <0x2>; > + #size-cells = <0x0>; only now I notice that all our other .dtsi also use hex values (instead of decimal as just a few lines above) here do you know if there is a particular reason for this? [...] > + uart_AO_B: serial@fe002000 { > + compatible = "amlogic,meson-gx-uart", > + "amlogic,meson-ao-uart"; > + reg = <0x0 0xfe002000 0x0 0x18>; the indentation of the "reg" property is off here also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here aren't there any busses defined in the A1 SoC implementation or are were you planning to add them later? Martin
Hi Martin, Thanks for the review, we really appreciate your time. Please see my comments below. On 2019/9/6 4:15, Martin Blumenstingl wrote: > Hi Jianxin, > > (it's great to see that you and your team are upstreaming this early) > > On Thu, Sep 5, 2019 at 9:08 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > [...] >> + memory@0 { >> + device_type = "memory"; >> + reg = <0x0 0x0 0x0 0x8000000>; >> + /*linux,usable-memory = <0x0 0x0 0x0 0x8000000>;*/ > why do we need that comment here (I don't understand it - why doesn't > the "reg" property cover this)? > I replaced "linux,usable-memory" with reg, but forgot to remove this comment line. I will remove this line in the next version. Thank you. >> + }; >> +}; >> + >> +&uart_AO_B { >> + status = "okay"; >> +}; >> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi >> new file mode 100644 >> index 00000000..4d476ac >> --- /dev/null >> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi >> @@ -0,0 +1,122 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +/* >> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. >> + */ >> + >> +#include <dt-bindings/interrupt-controller/irq.h> >> +#include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> +/ { >> + compatible = "amlogic,a1"; >> + >> + interrupt-parent = <&gic>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + cpus { >> + #address-cells = <0x2>; >> + #size-cells = <0x0>; > only now I notice that all our other .dtsi also use hex values > (instead of decimal as just a few lines above) here > do you know if there is a particular reason for this? > I just copied from the previous series, and didn't notice the difference before.> [...] >> + uart_AO_B: serial@fe002000 { >> + compatible = "amlogic,meson-gx-uart", >> + "amlogic,meson-ao-uart"; >> + reg = <0x0 0xfe002000 0x0 0x18>; > the indentation of the "reg" property is off here OK, I will fix it. > > also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here > aren't there any busses defined in the A1 SoC implementation or are > were you planning to add them later? >Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. > > Martin > > . >
Hi Jianxin, On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: [...] > > also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here > > aren't there any busses defined in the A1 SoC implementation or are > > were you planning to add them later? > Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. > Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. OK, thank you for the explanation since you're going to re-send the patch anyways: can you please include the apb_32b bus? all other upstream Amlogic .dts are using the bus definitions, so that will make A1 consistent with the other SoCs Martin
On Sat 07 Sep 2019 at 17:02, Martin Blumenstingl wrote: > Hi Jianxin, > > On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > [...] >> > also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here >> > aren't there any busses defined in the A1 SoC implementation or are >> > were you planning to add them later? >> Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. >> Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. > OK, thank you for the explanation > since you're going to re-send the patch anyways: can you please > include the apb_32b bus? unless there is an 64 bits apb bus as well, I suppose 'apb' would be enough ? > all other upstream Amlogic .dts are using the bus definitions, so that > will make A1 consistent with the other SoCs > > > Martin
Hi Martin, On 2019/9/7 23:02, Martin Blumenstingl wrote: > Hi Jianxin, > > On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > [...] >>> also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here >>> aren't there any busses defined in the A1 SoC implementation or are >>> were you planning to add them later? >> Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. >> Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. > OK, thank you for the explanation > since you're going to re-send the patch anyways: can you please > include the apb_32b bus? > all other upstream Amlogic .dts are using the bus definitions, so that > will make A1 consistent with the other SoCs In A1 (and the later C1), BUS is not mentioned in the memmap and register spec. Registers are organized and grouped by functions, and we can not find information about buses from the SoC document. Maybe it's better to remove bus definitions for these chips. > > > Martin > > . >
Hi Jerome, On 2019/9/9 19:36, Jerome Brunet wrote: > > On Sat 07 Sep 2019 at 17:02, Martin Blumenstingl wrote: > >> Hi Jianxin, >> >> On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: >> [...] >>>> also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here >>>> aren't there any busses defined in the A1 SoC implementation or are >>>> were you planning to add them later? >>> Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. >>> Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. >> OK, thank you for the explanation >> since you're going to re-send the patch anyways: can you please >> include the apb_32b bus? > > unless there is an 64 bits apb bus as well, I suppose 'apb' would be enough ? > There is no 64bits apb bus in A1, only apb32. Unlike the previous series, For A1 and C1, we can not get bus information for each register from the memmap and datesheet. Do we need to add bus description for them too? If yes, I can add 'apb' . >> all other upstream Amlogic .dts are using the bus definitions, so that >> will make A1 consistent with the other SoCs >> >> >> Martin > > . >
Hi Jianxin, On Mon, Sep 9, 2019 at 2:03 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > > Hi Martin, > > On 2019/9/7 23:02, Martin Blumenstingl wrote: > > Hi Jianxin, > > > > On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > > [...] > >>> also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here > >>> aren't there any busses defined in the A1 SoC implementation or are > >>> were you planning to add them later? > >> Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. > >> Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. > > OK, thank you for the explanation > > since you're going to re-send the patch anyways: can you please > > include the apb_32b bus? > > all other upstream Amlogic .dts are using the bus definitions, so that > > will make A1 consistent with the other SoCs > In A1 (and the later C1), BUS is not mentioned in the memmap and register spec. > Registers are organized and grouped by functions, and we can not find information about buses from the SoC document. do you know why the busses are not part of the documentation? > Maybe it's better to remove bus definitions for these chips. my understanding is that devicetree describes the hardware so if there's a bus in hardware (that we know about) then we should describe it in devicetree personally I think busses also make the .dts easier to read: instead of a huge .dts with all nodes on one level it's split into multiple smaller sub-nodes - thus making it easier to keep track of "where am I in this file". Martin
Hi Martin, On 2019/9/10 1:24, Martin Blumenstingl wrote: > Hi Jianxin, > > On Mon, Sep 9, 2019 at 2:03 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: >> >> Hi Martin, >> >> On 2019/9/7 23:02, Martin Blumenstingl wrote: >>> Hi Jianxin, >>> >>> On Fri, Sep 6, 2019 at 7:58 AM Jianxin Pan <jianxin.pan@amlogic.com> wrote: >>> [...] >>>>> also I'm a bit surprised to see no busses (like aobus, cbus, periphs, ...) here >>>>> aren't there any busses defined in the A1 SoC implementation or are >>>>> were you planning to add them later? >>>> Unlike previous series,there is no Cortex-M3 AO CPU in A1, and there is no AO/EE power domain. >>>> Most of the registers are on the apb_32b bus. aobus, cbus and periphs are not used in A1. >>> OK, thank you for the explanation >>> since you're going to re-send the patch anyways: can you please >>> include the apb_32b bus? >>> all other upstream Amlogic .dts are using the bus definitions, so that >>> will make A1 consistent with the other SoCs >> In A1 (and the later C1), BUS is not mentioned in the memmap and register spec. >> Registers are organized and grouped by functions, and we can not find information about buses from the SoC document. > do you know why the busses are not part of the documentation? > >> Maybe it's better to remove bus definitions for these chips. > my understanding is that devicetree describes the hardware > so if there's a bus in hardware (that we know about) then we should > describe it in devicetree > > personally I think busses also make the .dts easier to read: > instead of a huge .dts with all nodes on one level it's split into > multiple smaller sub-nodes - thus making it easier to keep track of > "where am I in this file". > OK, I will add the bus description for A1. Thank you for your suggestion. > > Martin > > . >
diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile index 84afecb..a90be52 100644 --- a/arch/arm64/boot/dts/amlogic/Makefile +++ b/arch/arm64/boot/dts/amlogic/Makefile @@ -36,3 +36,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxm-rbox-pro.dtb dtb-$(CONFIG_ARCH_MESON) += meson-gxm-vega-s96.dtb dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb +dtb-$(CONFIG_ARCH_MESON) += meson-a1-ad401.dtb diff --git a/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts new file mode 100644 index 00000000..190dedf --- /dev/null +++ b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. + */ + +/dts-v1/; + +#include "meson-a1.dtsi" + +/ { + compatible = "amlogic,ad401", "amlogic,a1"; + model = "Amlogic Meson A1 AD401 Development Board"; + + aliases { + serial0 = &uart_AO_B; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + memory@0 { + device_type = "memory"; + reg = <0x0 0x0 0x0 0x8000000>; + /*linux,usable-memory = <0x0 0x0 0x0 0x8000000>;*/ + }; +}; + +&uart_AO_B { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi new file mode 100644 index 00000000..4d476ac --- /dev/null +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. + */ + +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/interrupt-controller/arm-gic.h> + +/ { + compatible = "amlogic,a1"; + + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + cpus { + #address-cells = <0x2>; + #size-cells = <0x0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "arm,cortex-a35"; + reg = <0x0 0x0>; + enable-method = "psci"; + next-level-cache = <&l2>; + }; + + cpu1: cpu@1 { + device_type = "cpu"; + compatible = "arm,cortex-a35"; + reg = <0x0 0x1>; + enable-method = "psci"; + next-level-cache = <&l2>; + }; + + l2: l2-cache0 { + compatible = "cache"; + }; + }; + + psci { + compatible = "arm,psci-1.0"; + method = "smc"; + }; + + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + linux,cma { + compatible = "shared-dma-pool"; + reusable; + size = <0x0 0x800000>; + alignment = <0x0 0x400000>; + linux,cma-default; + }; + }; + + sm: secure-monitor { + compatible = "amlogic,meson-gxbb-sm"; + }; + + soc { + compatible = "simple-bus"; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + uart_AO: serial@fe001c00 { + compatible = "amlogic,meson-gx-uart", + "amlogic,meson-ao-uart"; + reg = <0x0 0xfe001c00 0x0 0x18>; + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>; + clocks = <&xtal>, <&xtal>, <&xtal>; + clock-names = "xtal", "pclk", "baud"; + status = "disabled"; + }; + + uart_AO_B: serial@fe002000 { + compatible = "amlogic,meson-gx-uart", + "amlogic,meson-ao-uart"; + reg = <0x0 0xfe002000 0x0 0x18>; + interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>; + clocks = <&xtal>, <&xtal>, <&xtal>; + clock-names = "xtal", "pclk", "baud"; + status = "disabled"; + }; + + gic: interrupt-controller@ff901000 { + compatible = "arm,gic-400"; + reg = <0x0 0xff901000 0x0 0x1000>, + <0x0 0xff902000 0x0 0x2000>, + <0x0 0xff904000 0x0 0x2000>, + <0x0 0xff906000 0x0 0x2000>; + interrupt-controller; + interrupts = <GIC_PPI 9 + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>; + #interrupt-cells = <3>; + #address-cells = <0>; + }; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <GIC_PPI 13 + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 14 + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 11 + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 10 + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>; + }; + + xtal: xtal-clk { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + clock-output-names = "xtal"; + #clock-cells = <0>; + }; +};