diff mbox series

[v1,3/3] arch/arm: dts: introduce meson-a1 device tree

Message ID 20230222115020.55867-4-avromanov@sberdevices.ru (mailing list archive)
State New, archived
Headers show
Series Meson A1 32-bit support | expand

Commit Message

Alexey Romanov Feb. 22, 2023, 11:50 a.m. UTC 
Add basic support for the 32-bit Amlogic A1. This device tree
describes following compontents: CPU, GIC, IRQ, Timer, UART,
PIN controller. It's capable of booting up into
the serial console.

This is based on arm64 version of meson-a1.dtsi.

Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
---
 arch/arm/boot/dts/meson-a1.dtsi | 151 ++++++++++++++++++++++++++++++++
 1 file changed, 151 insertions(+)
 create mode 100644 arch/arm/boot/dts/meson-a1.dtsi

Comments

Krzysztof Kozlowski Feb. 23, 2023, 9:08 a.m. UTC | #1 
On 22/02/2023 12:50, Alexey Romanov wrote:
> Add basic support for the 32-bit Amlogic A1. This device tree

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

> describes following compontents: CPU, GIC, IRQ, Timer, UART,
> PIN controller. It's capable of booting up into
> the serial console.
> 
> This is based on arm64 version of meson-a1.dtsi.
> 
> Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
> ---
>  arch/arm/boot/dts/meson-a1.dtsi | 151 ++++++++++++++++++++++++++++++++

There is such file and there is such DTS/hardware support. I don't see
any reason why entire DTSI should be duplicated. What's more, your
commit does not explain it - does not justify duplication.

Best regards,
Krzysztof
Krzysztof Kozlowski Feb. 23, 2023, 9:09 a.m. UTC | #2 
On 23/02/2023 10:08, Krzysztof Kozlowski wrote:
> On 22/02/2023 12:50, Alexey Romanov wrote:
>> Add basic support for the 32-bit Amlogic A1. This device tree
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
> 
>> describes following compontents: CPU, GIC, IRQ, Timer, UART,
>> PIN controller. It's capable of booting up into
>> the serial console.
>>
>> This is based on arm64 version of meson-a1.dtsi.
>>
>> Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
>> ---
>>  arch/arm/boot/dts/meson-a1.dtsi | 151 ++++++++++++++++++++++++++++++++
> 
> There is such file and there is such DTS/hardware support. I don't see
> any reason why entire DTSI should be duplicated. What's more, your
> commit does not explain it - does not justify duplication.

One more comment - I think you just added dead code. It's
uncompilable/untestable. Otherwise, please share how to build this DTSI
without DTS.

Best regards,
Krzysztof
Dmitry Rokosov Feb. 27, 2023, 2:39 p.m. UTC | #3 
Hello Krzysztof!

On Thu, Feb 23, 2023 at 10:09:25AM +0100, Krzysztof Kozlowski wrote:

[...]

> >> describes following compontents: CPU, GIC, IRQ, Timer, UART,
> >> PIN controller. It's capable of booting up into
> >> the serial console.
> >>
> >> This is based on arm64 version of meson-a1.dtsi.
> >>
> >> Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
> >> ---
> >>  arch/arm/boot/dts/meson-a1.dtsi | 151 ++++++++++++++++++++++++++++++++
> > 
> > There is such file and there is such DTS/hardware support. I don't see
> > any reason why entire DTSI should be duplicated. What's more, your
> > commit does not explain it - does not justify duplication.
> 
> One more comment - I think you just added dead code. It's
> uncompilable/untestable. Otherwise, please share how to build this DTSI
> without DTS.

You are right, Alexey doesn't provide any exact *.dts file for any
board, and *.dtsi file should be included somewhere, otherwise this is
dead code.
Unfortunately, our internal board *.dts file is useless for kernel
community, cause there is not any chance to burn locally compiled kernel
to our product due to secureboot protection.
But I think there is one possible option. We have reference Amlogic
boards somewhere in the office. So we can test 32-bit configuration on
it and prepare proper *.dts file for that. What do you think, it
reasanoble?
Krzysztof Kozlowski Feb. 27, 2023, 2:41 p.m. UTC | #4 
On 27/02/2023 15:39, Dmitry Rokosov wrote:
> Hello Krzysztof!
> 
> On Thu, Feb 23, 2023 at 10:09:25AM +0100, Krzysztof Kozlowski wrote:
> 
> [...]
> 
>>>> describes following compontents: CPU, GIC, IRQ, Timer, UART,
>>>> PIN controller. It's capable of booting up into
>>>> the serial console.
>>>>
>>>> This is based on arm64 version of meson-a1.dtsi.
>>>>
>>>> Signed-off-by: Alexey Romanov <avromanov@sberdevices.ru>
>>>> ---
>>>>  arch/arm/boot/dts/meson-a1.dtsi | 151 ++++++++++++++++++++++++++++++++
>>>
>>> There is such file and there is such DTS/hardware support. I don't see
>>> any reason why entire DTSI should be duplicated. What's more, your
>>> commit does not explain it - does not justify duplication.
>>
>> One more comment - I think you just added dead code. It's
>> uncompilable/untestable. Otherwise, please share how to build this DTSI
>> without DTS.
> 
> You are right, Alexey doesn't provide any exact *.dts file for any
> board, and *.dtsi file should be included somewhere, otherwise this is
> dead code.
> Unfortunately, our internal board *.dts file is useless for kernel
> community, cause there is not any chance to burn locally compiled kernel
> to our product due to secureboot protection.
> But I think there is one possible option. We have reference Amlogic
> boards somewhere in the office. So we can test 32-bit configuration on
> it and prepare proper *.dts file for that. What do you think, it
> reasanoble?

You just need to provide valid board which works in 32-bit mode. Anyway
duplicating DTSI is a no-go and we do not do it in other platforms.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/meson-a1.dtsi b/arch/arm/boot/dts/meson-a1.dtsi
new file mode 100644
index 000000000000..1d900fe86f8e
--- /dev/null
+++ b/arch/arm/boot/dts/meson-a1.dtsi
@@ -0,0 +1,151 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2023 SberDevices.
+ * Author: Alexey Romanov <avromanov@sberdevices.ru>
+ */
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/gpio/meson-a1-gpio.h>
+
+/ {
+	compatible = "amlogic,a1";
+
+	interrupt-parent = <&gic>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x0>;
+			enable-method = "psci";
+			next-level-cache = <&l2>;
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			reg = <0x1>;
+			enable-method = "psci";
+			next-level-cache = <&l2>;
+		};
+
+		l2: l2-cache0 {
+			compatible = "cache";
+		};
+	};
+
+	psci {
+		compatible = "arm,psci-1.0";
+		method = "smc";
+	};
+
+	sm: secure-monitor {
+		compatible = "amlogic,meson-gxbb-sm";
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		apb: bus@fe000000 {
+			compatible = "simple-bus";
+			reg = <0xfe000000 0x1000000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0xfe000000 0x1000000>;
+
+			reset: reset-controller@0 {
+				compatible = "amlogic,meson-a1-reset";
+				reg = <0x0 0x8c>;
+				#reset-cells = <1>;
+			};
+
+			periphs_pinctrl: pinctrl@400 {
+				compatible = "amlogic,meson-a1-periphs-pinctrl";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges;
+
+				gpio: bank@400 {
+					reg = <0x0400 0x003c>,
+					      <0x0480 0x0118>;
+					reg-names = "mux", "gpio";
+					gpio-controller;
+					#gpio-cells = <2>;
+					gpio-ranges = <&periphs_pinctrl 0 0 62>;
+				};
+
+			};
+
+			uart_AO: serial@1c00 {
+				compatible = "amlogic,meson-gx-uart",
+					     "amlogic,meson-ao-uart";
+				reg = <0x1c00 0x18>;
+				interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
+				clocks = <&xtal>, <&xtal>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
+				status = "disabled";
+			};
+
+			uart_AO_B: serial@2000 {
+				compatible = "amlogic,meson-gx-uart",
+					     "amlogic,meson-ao-uart";
+				reg = <0x2000 0x18>;
+				interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
+				clocks = <&xtal>, <&xtal>, <&xtal>;
+				clock-names = "xtal", "pclk", "baud";
+				status = "disabled";
+			};
+
+			gpio_intc: interrupt-controller@0440 {
+				compatible = "amlogic,meson-a1-gpio-intc",
+					     "amlogic,meson-gpio-intc";
+				reg = <0x0440 0x14>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				amlogic,channel-interrupts =
+					<49 50 51 52 53 54 55 56>;
+			};
+		};
+
+		gic: interrupt-controller@ff901000 {
+			compatible = "arm,gic-400";
+			reg = <0xff901000 0x1000>,
+			      <0xff902000 0x2000>,
+			      <0xff904000 0x2000>,
+			      <0xff906000 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>;
+	};
+};