[PATCH] arm-soc: Add Sigma Designs Tango4 port

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

Add support for Tango4-based SoCs (SMP8756, SMP8758)

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
NOTE: Mans reviewed the patch, and noted that the proposed
clock tree is too simplified. I have an upcoming patch fixing
that issue.
---
 arch/arm/Kconfig                   |   2 +
 arch/arm/Makefile                  |   1 +
 arch/arm/boot/dts/Makefile         |   2 +
 arch/arm/boot/dts/tango4.dtsi      | 117 +++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/vantage-1172.dts |   8 +++
 arch/arm/mach-tangox/Kconfig       |  12 ++++
 arch/arm/mach-tangox/Makefile      |   1 +
 arch/arm/mach-tangox/setup.c       |   7 +++
 8 files changed, 150 insertions(+)
 create mode 100644 arch/arm/boot/dts/tango4.dtsi
 create mode 100644 arch/arm/boot/dts/vantage-1172.dts
 create mode 100644 arch/arm/mach-tangox/Kconfig
 create mode 100644 arch/arm/mach-tangox/Makefile
 create mode 100644 arch/arm/mach-tangox/setup.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1c5021002fe4..94a1a0277c94 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
 
 source "arch/arm/mach-prima2/Kconfig"
 
+source "arch/arm/mach-tangox/Kconfig"
+
 source "arch/arm/mach-tegra/Kconfig"
 
 source "arch/arm/mach-u300/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 7451b447cc2d..7fcb4c63cdf7 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
 machine-$(CONFIG_ARCH_STI)		+= sti
 machine-$(CONFIG_ARCH_STM32)		+= stm32
 machine-$(CONFIG_ARCH_SUNXI)		+= sunxi
+machine-$(CONFIG_ARCH_TANGOX)		+= tangox
 machine-$(CONFIG_ARCH_TEGRA)		+= tegra
 machine-$(CONFIG_ARCH_U300)		+= u300
 machine-$(CONFIG_ARCH_U8500)		+= ux500
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 246473a244f6..42ba8b1be0d5 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 dtb-$(CONFIG_MACH_SUN9I) += \
 	sun9i-a80-optimus.dtb \
 	sun9i-a80-cubieboard4.dtb
+dtb-$(CONFIG_ARCH_TANGOX) += \
+	vantage-1172.dtb
 dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
 	tegra20-harmony.dtb \
 	tegra20-iris-512.dtb \
diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
new file mode 100644
index 000000000000..7336fcc3ac1d
--- /dev/null
+++ b/arch/arm/boot/dts/tango4.dtsi
@@ -0,0 +1,117 @@
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	compatible = "sigma,tango4-soc";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		xtal: xtal {
+			compatible = "fixed-clock";
+			clock-frequency = <27000000>;
+			#clock-cells = <0>;
+		};
+
+		sysclk: sysclk {
+			compatible = "fixed-clock";
+			clock-frequency = <396000000>;
+			#clock-cells = <0>;
+		};
+
+		cpuclk: cpuclk {
+			compatible = "fixed-clock";
+			clock-frequency = <999000000>;
+			#clock-cells = <0>;
+		};
+
+		periphclk: periphclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpuclk>;
+			clock-mult = <1>;
+			clock-div  = <2>;
+			#clock-cells = <0>;
+		};
+	};
+
+	gic: gic at 20001000 {
+		compatible = "arm,cortex-a9-gic";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x20001000 0x1000>,
+		      <0x20000100 0x0100>;
+	};
+
+	twd-timer at 20000600 {
+		compatible = "arm,cortex-a9-twd-timer";
+		reg = <0x20000600 0x10>;
+		interrupts = <1 13 0xf01>;
+		interrupt-parent = <&gic>;
+		clocks = <&periphclk>;
+		twd_never_stops;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		xtal_in_cnt {
+			compatible = "sigma,xtal_in_cnt";
+			reg = <0x10048 0x4>;
+			clocks = <&xtal>;
+		};
+
+		uart0 {
+			compatible = "ralink,rt2880-uart";
+			reg = <0x10700 0x100>;
+			clock-frequency = <7372800>;
+			reg-shift = <2>;
+/*			fifo-size = <16>; BROKEN */
+		};
+
+		eth0: eth0 {
+			compatible = "sigma,smp8640-emac";
+			reg = <0x26000 0x800>;
+			interrupts = <38 4>;
+			interrupt-parent = <&irq0>;
+			mac-address = [ 00 16 e8 02 08 42 ];
+			clocks = <&sysclk>;
+		};
+
+		intc: intc at e000 {
+			compatible = "sigma,tango-intc";
+			reg = <0x6e000 0x1000>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			irq0: irq0 at 000 {
+				reg = <0x000 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 2 4>;
+			};
+
+			irq1: irq1 at 100 {
+				reg = <0x100 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 3 4>;
+			};
+
+			irq2: irq2 at 300 {
+				reg = <0x300 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 4 4>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
new file mode 100644
index 000000000000..56f6babe7093
--- /dev/null
+++ b/arch/arm/boot/dts/vantage-1172.dts
@@ -0,0 +1,8 @@
+/dts-v1/;
+
+#include "tango4.dtsi"
+
+&eth0 {
+	phy-connection-type = "rgmii";
+	max-speed = <1000>;
+};
diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
new file mode 100644
index 000000000000..152cdd487056
--- /dev/null
+++ b/arch/arm/mach-tangox/Kconfig
@@ -0,0 +1,12 @@
+# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
+
+config ARCH_TANGOX
+	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
+	select ARCH_HAS_HOLES_MEMORYMODEL
+	select ARM_ERRATA_754322
+	select ARM_ERRATA_764369 if SMP
+	select ARM_GIC
+	select GENERIC_IRQ_CHIP
+	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD
+	select SERIAL_8250_RT288X if SERIAL_8250
diff --git a/arch/arm/mach-tangox/Makefile b/arch/arm/mach-tangox/Makefile
new file mode 100644
index 000000000000..2b9dba458932
--- /dev/null
+++ b/arch/arm/mach-tangox/Makefile
@@ -0,0 +1 @@
+obj-y += setup.o
diff --git a/arch/arm/mach-tangox/setup.c b/arch/arm/mach-tangox/setup.c
new file mode 100644
index 000000000000..14baf14bbd49
--- /dev/null
+++ b/arch/arm/mach-tangox/setup.c
@@ -0,0 +1,7 @@
+#include <asm/mach/arch.h>
+
+static const char *tango_dt_compat[] = { "sigma,tango4-soc", NULL };
+
+DT_MACHINE_START(TANGO_DT, "Sigma TangoX DT")
+	.dt_compat	= tango_dt_compat,
+MACHINE_END
-- 
2.4.5

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Måns Rullgård]

Mason <slash.tmp@free.fr> writes:

> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
> new file mode 100644
> index 000000000000..7336fcc3ac1d
> --- /dev/null
> +++ b/arch/arm/boot/dts/tango4.dtsi
> @@ -0,0 +1,117 @@
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	compatible = "sigma,tango4-soc";
> +
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	clocks {
> +		ranges;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		xtal: xtal {
> +			compatible = "fixed-clock";
> +			clock-frequency = <27000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		sysclk: sysclk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <396000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		cpuclk: cpuclk {
> +			compatible = "fixed-clock";
> +			clock-frequency = <999000000>;
> +			#clock-cells = <0>;
> +		};
> +
> +		periphclk: periphclk {
> +			compatible = "fixed-factor-clock";
> +			clocks = <&cpuclk>;
> +			clock-mult = <1>;
> +			clock-div  = <2>;
> +			#clock-cells = <0>;
> +		};
> +	};

Once more with feeling, why don't you want to use the fine clock driver
I wrote?

> +	gic: gic at 20001000 {
> +		compatible = "arm,cortex-a9-gic";
> +		interrupt-controller;
> +		#interrupt-cells = <3>;
> +		reg = <0x20001000 0x1000>,
> +		      <0x20000100 0x0100>;
> +	};
> +
> +	twd-timer at 20000600 {
> +		compatible = "arm,cortex-a9-twd-timer";
> +		reg = <0x20000600 0x10>;
> +		interrupts = <1 13 0xf01>;
> +		interrupt-parent = <&gic>;
> +		clocks = <&periphclk>;
> +		twd_never_stops;
> +	};
> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		xtal_in_cnt {
> +			compatible = "sigma,xtal_in_cnt";
> +			reg = <0x10048 0x4>;
> +			clocks = <&xtal>;
> +		};
> +
> +		uart0 {
> +			compatible = "ralink,rt2880-uart";
> +			reg = <0x10700 0x100>;
> +			clock-frequency = <7372800>;
> +			reg-shift = <2>;
> +/*			fifo-size = <16>; BROKEN */

Either fix whatever is broken or drop that line.

> +		};
> +
> +		eth0: eth0 {
> +			compatible = "sigma,smp8640-emac";
> +			reg = <0x26000 0x800>;
> +			interrupts = <38 4>;
> +			interrupt-parent = <&irq0>;
> +			mac-address = [ 00 16 e8 02 08 42 ];

mac-address should not be hardcoded here or anywhere else.

> +			clocks = <&sysclk>;
> +		};
> +
> +		intc: intc at e000 {
> +			compatible = "sigma,tango-intc";

Why do you insist on using other names than the ones I've been using for
months?  Just want to leave your own mark on the code?

> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> new file mode 100644
> index 000000000000..152cdd487056
> --- /dev/null
> +++ b/arch/arm/mach-tangox/Kconfig
> @@ -0,0 +1,12 @@
> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.

This comment isn't relevant to the contents of the file.

-- 
M?ns Rullg?rd
mans at mansr.com

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 18:10, M?ns Rullg?rd wrote:

> Mason writes:
> 
>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
>> new file mode 100644
>> index 000000000000..7336fcc3ac1d
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/tango4.dtsi
>
> Once more with feeling, why don't you want to use the fine clock driver
> I wrote?

I'll send you my clock tree driver next week. Then we can discuss.

>> +		uart0 {
>> +			compatible = "ralink,rt2880-uart";
>> +			reg = <0x10700 0x100>;
>> +			clock-frequency = <7372800>;
>> +			reg-shift = <2>;
>> +/*			fifo-size = <16>; BROKEN */
> 
> Either fix whatever is broken or drop that line.

I can't leave TODO reminders in the platform Kconfig?
Even as comments?

>> +		};
>> +
>> +		eth0: eth0 {
>> +			compatible = "sigma,smp8640-emac";
>> +			reg = <0x26000 0x800>;
>> +			interrupts = <38 4>;
>> +			interrupt-parent = <&irq0>;
>> +			mac-address = [ 00 16 e8 02 08 42 ];
> 
> mac-address should not be hardcoded here or anywhere else.

Sorry. I missed that in my clean up.

>> +			clocks = <&sysclk>;
>> +		};
>> +
>> +		intc: intc at e000 {
>> +			compatible = "sigma,tango-intc";
> 
> Why do you insist on using other names than the ones I've been using for
> months?  Just want to leave your own mark on the code?

You're using "sigma,smp8640-intc".
The SMP8640 is a Tango3 (MIPS-based) platform.
It makes no sense to have references to Tango3 in tango4.dtsi
Aside from the CPU difference, Tango3 and Tango4 have a lot in common though.

Which reminds me that I forgot to change "sigma,smp8640-emac"
I'll send an updated patch.

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Måns Rullgård]

Mason <slash.tmp@free.fr> writes:

>>> +		uart0 {
>>> +			compatible = "ralink,rt2880-uart";
>>> +			reg = <0x10700 0x100>;
>>> +			clock-frequency = <7372800>;
>>> +			reg-shift = <2>;
>>> +/*			fifo-size = <16>; BROKEN */
>> 
>> Either fix whatever is broken or drop that line.
>
> I can't leave TODO reminders in the platform Kconfig?
> Even as comments?

Never mind, I've sent a patch fixing the problem.

>>> +		intc: intc at e000 {
>>> +			compatible = "sigma,tango-intc";
>> 
>> Why do you insist on using other names than the ones I've been using for
>> months?  Just want to leave your own mark on the code?
>
> You're using "sigma,smp8640-intc".
> The SMP8640 is a Tango3 (MIPS-based) platform.
> It makes no sense to have references to Tango3 in tango4.dtsi
> Aside from the CPU difference, Tango3 and Tango4 have a lot in common though.

It's commonplace to refer to peripherals by the earliest (supported)
chip using them.  This avoids naming conflicts if a future chip uses a
different component.  Some bits are incompatible even between different
devices in the tango3 family.

-- 
M?ns Rullg?rd
mans at mansr.com

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Russell King - ARM Linux]

On Fri, Oct 02, 2015 at 06:33:48PM +0200, Mason wrote:
> On 02/10/2015 18:10, M?ns Rullg?rd wrote:
> 
> > Mason writes:
> > 
> >> +		intc: intc at e000 {
> >> +			compatible = "sigma,tango-intc";
> > 
> > Why do you insist on using other names than the ones I've been using for
> > months?  Just want to leave your own mark on the code?
> 
> You're using "sigma,smp8640-intc".
> The SMP8640 is a Tango3 (MIPS-based) platform.

If it's the same device, then there's nothing wrong with re-using the
compatible.  The compatible property actually accepts multiple values,
so you can do:

			compatible = "sigma,tango4-intc", "sigma,smp8640-intc";

See the ePAPR spec - I'll include the relevent bit:

Property: compatible
Value type: <stringlist>
Description: The compatible property value consists of one or more strings
 that define the specific programming model for the device. This list of
 strings should be used by a client program for device driver selection.
 The property value consists of a concatenated list of null terminated
 strings, from most specific to most general. They allow a device to
 express its compatibility with a family of similar devices, potentially
 allowing a single device driver to match against several devices. ...
Example: compatible = "fsl,mpc8641-uart", "ns16550";
 In this example, an operating system would first try to locate a device
 driver that supported fsl,mpc8641-uart. If a driver was not found, it
 would then try to locate a driver that supported the more general
 ns16550 device type.

Note also that vendor prefixes should be listed in
Documentation/devicetree/bindings/vendor-prefixes.txt.  If it's not there,
you need to propose a separate patch (to the devicetree mailing list) to
add it, which must be done with their agreement.  Right now, the use of
"sigma" as a prefix is entirely non-standard and not acceptable in DT
files until this is done.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 18:55, M?ns Rullg?rd wrote:

> Mason writes:
> 
>>>> +		uart0 {
>>>> +			compatible = "ralink,rt2880-uart";
>>>> +			reg = <0x10700 0x100>;
>>>> +			clock-frequency = <7372800>;
>>>> +			reg-shift = <2>;
>>>> +/*			fifo-size = <16>; BROKEN */
>>>
>>> Either fix whatever is broken or drop that line.
>>
>> I can't leave TODO reminders in the platform Kconfig [and DT files]?
> 
> Never mind, I've sent a patch fixing the problem.

That's great news. Although back-porting to 3.14 is looking
more and more time-consuming. (But that's my problem.)

The question of comments in Kconfig and DT files still stands.
(No need to state your position again.)

>>>> +		intc: intc at e000 {
>>>> +			compatible = "sigma,tango-intc";
>>>
>>> Why do you insist on using other names than the ones I've been using for
>>> months?  Just want to leave your own mark on the code?
>>
>> You're using "sigma,smp8640-intc".
>> The SMP8640 is a Tango3 (MIPS-based) platform.
>> It makes no sense to have references to Tango3 in tango4.dtsi
>> Aside from the CPU difference, Tango3 and Tango4 have a lot in common though.
> 
> It's commonplace to refer to peripherals by the earliest (supported)
> chip using them.  This avoids naming conflicts if a future chip uses a
> different component.  Some bits are incompatible even between different
> devices in the tango3 family.

I'm confused. I like the way ARM did it in smp-twd.c

CLOCKSOURCE_OF_DECLARE(arm_twd_a9, "arm,cortex-a9-twd-timer", twd_local_timer_of_register);
CLOCKSOURCE_OF_DECLARE(arm_twd_a5, "arm,cortex-a5-twd-timer", twd_local_timer_of_register);
CLOCKSOURCE_OF_DECLARE(arm_twd_11mp, "arm,arm11mp-twd-timer", twd_local_timer_of_register);

They have several definitions for the different supported platforms.

They have several versions of the GIC:

IRQCHIP_DECLARE(gic_400, "arm,gic-400", gic_of_init);
IRQCHIP_DECLARE(arm11mp_gic, "arm,arm11mp-gic", gic_of_init);
IRQCHIP_DECLARE(arm1176jzf_dc_gic, "arm,arm1176jzf-devchip-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init);

For example, I know the interrupt controller was changed for Tango5.

So I was planning on using:

"sigma,tango-intc" for Tango3/Tango4
"sigma,tango-intc-v2" for Tango5

Would you change your code to accommodate this nomenclature? 

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 19:13, Russell King - ARM Linux wrote:
> On Fri, Oct 02, 2015 at 06:33:48PM +0200, Mason wrote:
>> On 02/10/2015 18:10, M?ns Rullg?rd wrote:
>>
>>> Mason writes:
>>>
>>>> +		intc: intc at e000 {
>>>> +			compatible = "sigma,tango-intc";
>>>
>>> Why do you insist on using other names than the ones I've been using for
>>> months?  Just want to leave your own mark on the code?
>>
>> You're using "sigma,smp8640-intc".
>> The SMP8640 is a Tango3 (MIPS-based) platform.
> 
> If it's the same device, then there's nothing wrong with re-using the
> compatible.  The compatible property actually accepts multiple values,
> so you can do:
> 
> 			compatible = "sigma,tango4-intc", "sigma,smp8640-intc";

I have access to resources unavailable to Mans. Since I know the
interrupt controller is the same on Tango2, Tango3 and Tango4,
doesn't it make sense to use the string "sigma,tango-intc"
given that the driver is not even upstream yet?

(If worse comes to worst, I suppose I can always write my own
driver from scratch; I just find it silly to duplicate work.)

> See the ePAPR spec - I'll include the relevant bit:
> 
> Property: compatible
> Value type: <stringlist>
> Description: The compatible property value consists of one or more strings
>  that define the specific programming model for the device. This list of
>  strings should be used by a client program for device driver selection.
>  The property value consists of a concatenated list of null terminated
>  strings, from most specific to most general. They allow a device to
>  express its compatibility with a family of similar devices, potentially
>  allowing a single device driver to match against several devices. ...
> Example: compatible = "fsl,mpc8641-uart", "ns16550";
>  In this example, an operating system would first try to locate a device
>  driver that supported fsl,mpc8641-uart. If a driver was not found, it
>  would then try to locate a driver that supported the more general
>  ns16550 device type.
> 
> Note also that vendor prefixes should be listed in
> Documentation/devicetree/bindings/vendor-prefixes.txt.  If it's not there,
> you need to propose a separate patch (to the devicetree mailing list) to
> add it, which must be done with their agreement.  Right now, the use of
> "sigma" as a prefix is entirely non-standard and not acceptable in DT
> files until this is done.

As far as the upstreaming process is concerned, I speak for Sigma.

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Russell King - ARM Linux]

On Fri, Oct 02, 2015 at 08:09:39PM +0200, Mason wrote:
> On 02/10/2015 19:13, Russell King - ARM Linux wrote:
> > On Fri, Oct 02, 2015 at 06:33:48PM +0200, Mason wrote:
> >> On 02/10/2015 18:10, M?ns Rullg?rd wrote:
> >>
> >>> Mason writes:
> >>>
> >>>> +		intc: intc at e000 {
> >>>> +			compatible = "sigma,tango-intc";
> >>>
> >>> Why do you insist on using other names than the ones I've been using for
> >>> months?  Just want to leave your own mark on the code?
> >>
> >> You're using "sigma,smp8640-intc".
> >> The SMP8640 is a Tango3 (MIPS-based) platform.
> > 
> > If it's the same device, then there's nothing wrong with re-using the
> > compatible.  The compatible property actually accepts multiple values,
> > so you can do:
> > 
> > 			compatible = "sigma,tango4-intc", "sigma,smp8640-intc";
> 
> I have access to resources unavailable to Mans. Since I know the
> interrupt controller is the same on Tango2, Tango3 and Tango4,
> doesn't it make sense to use the string "sigma,tango-intc"
> given that the driver is not even upstream yet?

You could do:
			compatible = "sigma,tango4-intc",
				     "sigma,tango-intc",
				     "sigma,smp8640-intc";

The advantage of using the most specific first is that if you then find
the hardware contains bugs, you can _just_ modify the device driver to
match on the specific ID, and implement workarounds based on that.  Older
device trees will then pick up those same workarounds without needing to
be modified.

Remember, device trees are supposed to describe the hardware.

> (If worse comes to worst, I suppose I can always write my own
> driver from scratch; I just find it silly to duplicate work.)

It's not about writing a separate driver.  What the above says is that
this device is first and foremost a "sigma,tango4-intc" device, but if
we don't have such a driver, it can be driven by a driver for
"sigma,tango-intc", but if we don't have one of those, then it can
be driven by a "sigma,smp8640-intc" driver.

Please read the quoted bit of the spec on this to get a proper
understanding of how these compatible strings are supposed to work:

> > See the ePAPR spec - I'll include the relevant bit:
> > 
> > Property: compatible
> > Value type: <stringlist>
> > Description: The compatible property value consists of one or more strings
> >  that define the specific programming model for the device. This list of
> >  strings should be used by a client program for device driver selection.
> >  The property value consists of a concatenated list of null terminated
> >  strings, from most specific to most general. They allow a device to
> >  express its compatibility with a family of similar devices, potentially
> >  allowing a single device driver to match against several devices. ...
> > Example: compatible = "fsl,mpc8641-uart", "ns16550";
> >  In this example, an operating system would first try to locate a device
> >  driver that supported fsl,mpc8641-uart. If a driver was not found, it
> >  would then try to locate a driver that supported the more general
> >  ns16550 device type.
> > 
> > Note also that vendor prefixes should be listed in
> > Documentation/devicetree/bindings/vendor-prefixes.txt.  If it's not there,
> > you need to propose a separate patch (to the devicetree mailing list) to
> > add it, which must be done with their agreement.  Right now, the use of
> > "sigma" as a prefix is entirely non-standard and not acceptable in DT
> > files until this is done.
> 
> As far as the upstreaming process is concerned, I speak for Sigma.

It doesn't matter who you speak for.  Your first patch should be to
_only_ add the vendor ID to that file above, and to get it acked by
the device tree maintainers.  That makes it "official" in the eyes of
the developers responsible for maintaining the sanity of device tree.

However, that has an impact on the above: you should therefore have
access to the folk who know the origins of the interrupt controller,
and whether it is a derivative of "sigma,smp8640-intc" or something
else.  If "sigma,smp8640-intc" and "sigma,tango-intc" are jointly
derived from a common ancestor, then you should not mention
"sigma,smp8640-intc" at all.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 20:53, Russell King - ARM Linux wrote:
> On Fri, Oct 02, 2015 at 08:09:39PM +0200, Mason wrote:
>> On 02/10/2015 19:13, Russell King - ARM Linux wrote:
>>
>>> Note also that vendor prefixes should be listed in
>>> Documentation/devicetree/bindings/vendor-prefixes.txt.  If it's not there,
>>> you need to propose a separate patch (to the devicetree mailing list) to
>>> add it, which must be done with their agreement.  Right now, the use of
>>> "sigma" as a prefix is entirely non-standard and not acceptable in DT
>>> files until this is done.
>>
>> As far as the upstreaming process is concerned, I speak for Sigma.
> 
> It doesn't matter who you speak for.  Your first patch should be to
> _only_ add the vendor ID to that file above, and to get it acked by
> the device tree maintainers.  That makes it "official" in the eyes of
> the developers responsible for maintaining the sanity of device tree.

OK. Mans took care of that in
"devicetree: add Sigma Designs vendor prefix"

> However, that has an impact on the above: you should therefore have
> access to the folk who know the origins of the interrupt controller,
> and whether it is a derivative of "sigma,smp8640-intc" or something
> else.  If "sigma,smp8640-intc" and "sigma,tango-intc" are jointly
> derived from a common ancestor, then you should not mention
> "sigma,smp8640-intc" at all.

I think there is some confusion surrounding Sigma's SoCs.

Briefly, Sigma Designs has gone through 4 major revisions of
its SoC architecture, Tango1 through Tango4. (Let's forget
Tango1 and Tango2, as they have fallen into oblivion.)

Within a major architecture, Sigma produces different designs,
sometimes just blowing fuses to differentiate packages, sometimes
actually adding hardware, or tweaking the design. These designs
are given "SMP8xxx" names, typically SMP86xx for Tango3 (MIPS)
and SMP87xx for Tango4 (ARM).

For example, SMP8756 is an ARM-based design, with a single
Cortex A9 core, while SMP8758 has two A9 cores.

SMP8640 was just one Tango3 SoC out of several. It's not special
in any way, as far as the interrupt controller is concerned.
I'll have to check the docs, but I seem to remember it has
remained unchanged throughout the Tango2-Tango4 period.
(But it will change in Tango5.)

This is why I insist on not committing to the smp8640-* nomenclature.
Because SMP8640 is nothing special in the Tango family.

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Arnd Bergmann]

On Friday 02 October 2015 18:02:04 Mason wrote:
> Add support for Tango4-based SoCs (SMP8756, SMP8758)
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>

Please write a proper changelog text here that tells us more about
the patch than the subject line.

> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  dtb-$(CONFIG_MACH_SUN9I) += \
>  	sun9i-a80-optimus.dtb \
>  	sun9i-a80-cubieboard4.dtb
> +dtb-$(CONFIG_ARCH_TANGOX) += \
> +	vantage-1172.dtb

This file name should start with the name of the chip,
e.g. 'tango4-vantage-1172.dtb', to make it easier to find.

> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
> new file mode 100644
> index 000000000000..7336fcc3ac1d
> --- /dev/null
> +++ b/arch/arm/boot/dts/tango4.dtsi
> @@ -0,0 +1,117 @@
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	compatible = "sigma,tango4-soc";

Move the root comaptible strings into the board specific file,
and add the name of the machine as a more specific one.

> +
> +	soc {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		xtal_in_cnt {
> +			compatible = "sigma,xtal_in_cnt";
> +			reg = <0x10048 0x4>;
> +			clocks = <&xtal>;
> +		};
> +
> +		uart0 {
> +			compatible = "ralink,rt2880-uart";
> +			reg = <0x10700 0x100>;
> +			clock-frequency = <7372800>;
> +			reg-shift = <2>;
> +/*			fifo-size = <16>; BROKEN */
> +		};

Please use standard node names here, the uart should be
named 'serial at 10700', the other names should be
'ethernet at 26000', 'interrupt-controller at 6e000' etc.

If the SoC contains more than one UART, please list them all
here, and mark them as 'status="disabled"' in the .dtsi file,
then add the appropriate aliases and 'status="okay"' override
for the ones that are actually used on that board.

> +
> +		eth0: eth0 {
> +			compatible = "sigma,smp8640-emac";
> +			reg = <0x26000 0x800>;
> +			interrupts = <38 4>;
> +			interrupt-parent = <&irq0>;
> +			mac-address = [ 00 16 e8 02 08 42 ];
> +			clocks = <&sysclk>;
> +		};
> +
> +		intc: intc at e000 {
> +			compatible = "sigma,tango-intc";
> +			reg = <0x6e000 0x1000>;
> +			interrupt-parent = <&gic>;
> +			interrupt-controller;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			irq0: irq0 at 000 {
> +				reg = <0x000 0x100>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 2 4>;
> +			};

You are missing a ranges property that describes what address
space these addresses are in.

> +			irq1: irq1 at 100 {
> +				reg = <0x100 0x100>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 3 4>;
> +			};
> +
> +			irq2: irq2 at 300 {
> +				reg = <0x300 0x100>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 4 4>;
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
> new file mode 100644
> index 000000000000..56f6babe7093
> --- /dev/null
> +++ b/arch/arm/boot/dts/vantage-1172.dts
> @@ -0,0 +1,8 @@
> +/dts-v1/;
> +
> +#include "tango4.dtsi"
> +
> +&eth0 {
> +	phy-connection-type = "rgmii";
> +	max-speed = <1000>;
> +};

You should normally have /chosen and /aliases nodes here as well.

> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> new file mode 100644
> index 000000000000..152cdd487056
> --- /dev/null
> +++ b/arch/arm/mach-tangox/Kconfig
> @@ -0,0 +1,12 @@
> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
> +
> +config ARCH_TANGOX
> +	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
> +	select ARCH_HAS_HOLES_MEMORYMODEL
> +	select ARM_ERRATA_754322
> +	select ARM_ERRATA_764369 if SMP
> +	select ARM_GIC
> +	select GENERIC_IRQ_CHIP
> +	select HAVE_ARM_SCU
> +	select HAVE_ARM_TWD
> +	select SERIAL_8250_RT288X if SERIAL_8250

Do not 'select' the uart driver, that can just be part of the defconfig
file.

	Arnd

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 21:56, Arnd Bergmann wrote:
> On Friday 02 October 2015 18:02:04 Mason wrote:
>> Add support for Tango4-based SoCs (SMP8756, SMP8758)
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> 
> Please write a proper changelog text here that tells us more about
> the patch than the subject line.

Sorry, I'm quite unimaginative. What kind of information do you
consider required?

>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>>  dtb-$(CONFIG_MACH_SUN9I) += \
>>  	sun9i-a80-optimus.dtb \
>>  	sun9i-a80-cubieboard4.dtb
>> +dtb-$(CONFIG_ARCH_TANGOX) += \
>> +	vantage-1172.dtb
> 
> This file name should start with the name of the chip,
> e.g. 'tango4-vantage-1172.dtb', to make it easier to find.

OK.

(Why doesn't arm do it like mips, with per-manufacturer folders?)

>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
>> new file mode 100644
>> index 000000000000..7336fcc3ac1d
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/tango4.dtsi
>> @@ -0,0 +1,117 @@
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/ {
>> +	compatible = "sigma,tango4-soc";
> 
> Move the root compatible strings into the board specific file,
> and add the name of the machine as a more specific one.

I don't understand this.

>> +	soc {
>> +		compatible = "simple-bus";
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ranges;
>> +
>> +		xtal_in_cnt {
>> +			compatible = "sigma,xtal_in_cnt";
>> +			reg = <0x10048 0x4>;
>> +			clocks = <&xtal>;
>> +		};
>> +
>> +		uart0 {
>> +			compatible = "ralink,rt2880-uart";
>> +			reg = <0x10700 0x100>;
>> +			clock-frequency = <7372800>;
>> +			reg-shift = <2>;
>> +/*			fifo-size = <16>; BROKEN */
>> +		};
> 
> Please use standard node names here, the uart should be
> named 'serial at 10700', the other names should be
> 'ethernet at 26000', 'interrupt-controller at 6e000' etc.

Where are the standard node names documented?

Can I still name labels freely?
Or is there a naming convention for labels?

Why is the base address duplicated? Can't the DTC figure
out the address from the reg property?

>> +		eth0: eth0 {
>> +			compatible = "sigma,smp8640-emac";
>> +			reg = <0x26000 0x800>;
>> +			interrupts = <38 4>;
>> +			interrupt-parent = <&irq0>;
>> +			mac-address = [ 00 16 e8 02 08 42 ];
>> +			clocks = <&sysclk>;
>> +		};
>> +
>> +		intc: intc at e000 {
>> +			compatible = "sigma,tango-intc";
>> +			reg = <0x6e000 0x1000>;
>> +			interrupt-parent = <&gic>;
>> +			interrupt-controller;
>> +			#address-cells = <1>;
>> +			#size-cells = <1>;
>> +
>> +			irq0: irq0 at 000 {
>> +				reg = <0x000 0x100>;
>> +				interrupt-controller;
>> +				#interrupt-cells = <2>;
>> +				interrupts = <0 2 4>;
>> +			};
> 
> You are missing a ranges property that describes what address
> space these addresses are in.

ranges; is not hierarchically inherited?

>> +			irq1: irq1 at 100 {
>> +				reg = <0x100 0x100>;
>> +				interrupt-controller;
>> +				#interrupt-cells = <2>;
>> +				interrupts = <0 3 4>;
>> +			};
>> +
>> +			irq2: irq2 at 300 {
>> +				reg = <0x300 0x100>;
>> +				interrupt-controller;
>> +				#interrupt-cells = <2>;
>> +				interrupts = <0 4 4>;
>> +			};
>> +		};
>> +	};
>> +};
>> diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
>> new file mode 100644
>> index 000000000000..56f6babe7093
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/vantage-1172.dts
>> @@ -0,0 +1,8 @@
>> +/dts-v1/;
>> +
>> +#include "tango4.dtsi"
>> +
>> +&eth0 {
>> +	phy-connection-type = "rgmii";
>> +	max-speed = <1000>;
>> +};
> 
> You should normally have /chosen and /aliases nodes here as well.

Sigh. I don't know what they are for.
http://devicetree.org/Device_Tree_Usage#Special_Nodes

So with aliases, I don't need to define labels in the .dtsi?

"Typically the chosen node is left empty in .dts source files and populated at boot time."
Should I put my current bootargs there?

>> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
>> new file mode 100644
>> index 000000000000..152cdd487056
>> --- /dev/null
>> +++ b/arch/arm/mach-tangox/Kconfig
>> @@ -0,0 +1,12 @@
>> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
>> +
>> +config ARCH_TANGOX
>> +	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
>> +	select ARCH_HAS_HOLES_MEMORYMODEL
>> +	select ARM_ERRATA_754322
>> +	select ARM_ERRATA_764369 if SMP
>> +	select ARM_GIC
>> +	select GENERIC_IRQ_CHIP
>> +	select HAVE_ARM_SCU
>> +	select HAVE_ARM_TWD
>> +	select SERIAL_8250_RT288X if SERIAL_8250
> 
> Do not 'select' the uart driver, that can just be part of the defconfig
> file.

Do you mean I should provide a defconfig with my patch?

Picking SERIAL_8250 but not SERIAL_8250_RT288X makes the kernel
panic. Doesn't that qualify for selecting it? (I don't understand
the rationale behind most conventions in kernel dev.)

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Arnd Bergmann]

On Friday 02 October 2015 22:53:11 Mason wrote:
> On 02/10/2015 21:56, Arnd Bergmann wrote:
> > On Friday 02 October 2015 18:02:04 Mason wrote:
> >> Add support for Tango4-based SoCs (SMP8756, SMP8758)
> >>
> >> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> > 
> > Please write a proper changelog text here that tells us more about
> > the patch than the subject line.
> 
> Sorry, I'm quite unimaginative. What kind of information do you
> consider required?

The line you have there is not needed, but it would be nice to have
a link to the data sheet (if available) and some information about
what SoCs they are.

For most patches, you describe what the original code does wrong
first, followed by how your patch addresses the situation, but for
a new platform that is a bit different.

> >> --- a/arch/arm/boot/dts/Makefile
> >> +++ b/arch/arm/boot/dts/Makefile
> >> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >>  dtb-$(CONFIG_MACH_SUN9I) += \
> >>  	sun9i-a80-optimus.dtb \
> >>  	sun9i-a80-cubieboard4.dtb
> >> +dtb-$(CONFIG_ARCH_TANGOX) += \
> >> +	vantage-1172.dtb
> > 
> > This file name should start with the name of the chip,
> > e.g. 'tango4-vantage-1172.dtb', to make it easier to find.
> 
> OK.
> 
> (Why doesn't arm do it like mips, with per-manufacturer folders?)

Historic mistake on our side, and it's become too hard to fix now
without breaking hundreds of patches that people are trying to
get merged.

> >> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
> >> new file mode 100644
> >> index 000000000000..7336fcc3ac1d
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/tango4.dtsi
> >> @@ -0,0 +1,117 @@
> >> +#include <dt-bindings/interrupt-controller/irq.h>
> >> +
> >> +/ {
> >> +	compatible = "sigma,tango4-soc";
> > 
> > Move the root compatible strings into the board specific file,
> > and add the name of the machine as a more specific one.
> 
> I don't understand this.

The compatible list should list both generic and specific names
if applicable. For an SoC based platform, that almost always
translates into board name, soc name and sometimes soc family name.

> >> +	soc {
> >> +		compatible = "simple-bus";
> >> +		#address-cells = <1>;
> >> +		#size-cells = <1>;
> >> +		ranges;
> >> +
> >> +		xtal_in_cnt {
> >> +			compatible = "sigma,xtal_in_cnt";
> >> +			reg = <0x10048 0x4>;
> >> +			clocks = <&xtal>;
> >> +		};
> >> +
> >> +		uart0 {
> >> +			compatible = "ralink,rt2880-uart";
> >> +			reg = <0x10700 0x100>;
> >> +			clock-frequency = <7372800>;
> >> +			reg-shift = <2>;
> >> +/*			fifo-size = <16>; BROKEN */
> >> +		};
> > 
> > Please use standard node names here, the uart should be
> > named 'serial at 10700', the other names should be
> > 'ethernet at 26000', 'interrupt-controller at 6e000' etc.
> 
> Where are the standard node names documented?

ePAPR has a list, for others look at the existing dts files.

> Can I still name labels freely?

Labels can be freely assigned, they are only used as syntactic
sugar to let you write phandle references in a more compact
way, but names are what ends up in the dts and they should
follow convention as much as possible.

> Or is there a naming convention for labels?
> 
> Why is the base address duplicated? Can't the DTC figure
> out the address from the reg property?

I don't know what exactly has led to this, I believe it was
like this in original open firmware, but not always enforced,
but we probably needed to represent nodes from real firmware
in dtb files.

> >> +		eth0: eth0 {
> >> +			compatible = "sigma,smp8640-emac";
> >> +			reg = <0x26000 0x800>;
> >> +			interrupts = <38 4>;
> >> +			interrupt-parent = <&irq0>;
> >> +			mac-address = [ 00 16 e8 02 08 42 ];
> >> +			clocks = <&sysclk>;
> >> +		};
> >> +
> >> +		intc: intc at e000 {
> >> +			compatible = "sigma,tango-intc";
> >> +			reg = <0x6e000 0x1000>;
> >> +			interrupt-parent = <&gic>;
> >> +			interrupt-controller;
> >> +			#address-cells = <1>;
> >> +			#size-cells = <1>;
> >> +
> >> +			irq0: irq0 at 000 {
> >> +				reg = <0x000 0x100>;
> >> +				interrupt-controller;
> >> +				#interrupt-cells = <2>;
> >> +				interrupts = <0 2 4>;
> >> +			};
> > 
> > You are missing a ranges property that describes what address
> > space these addresses are in.
> 
> ranges; is not hierarchically inherited?

'ranges;' would be wrong here, as the interrupt controller is
not a bus. If you have no ranges property, the bus is interpreted
as having its own address space with no relation to the parent bus
(e.g. an I2C bus uses addresses that are not memory mapped).

Just list the addresses that are actually decoded by child
devices here.

> >> diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
> >> new file mode 100644
> >> index 000000000000..56f6babe7093
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/vantage-1172.dts
> >> @@ -0,0 +1,8 @@
> >> +/dts-v1/;
> >> +
> >> +#include "tango4.dtsi"
> >> +
> >> +&eth0 {
> >> +	phy-connection-type = "rgmii";
> >> +	max-speed = <1000>;
> >> +};
> > 
> > You should normally have /chosen and /aliases nodes here as well.
> 
> Sigh. I don't know what they are for.
> http://devicetree.org/Device_Tree_Usage#Special_Nodes
> 
> So with aliases, I don't need to define labels in the .dtsi?

labels are always optional, you can write the aliases node using
either labels or open-coded paths, like

/aliases {
	serial0 = &uart_3;
	serial1 = "/soc/serial at 10700";
};
 
> "Typically the chosen node is left empty in .dts source files and populated at boot time."
> Should I put my current bootargs there?

I would put only the stdout-path property in there, as long as you can
boot with any bootargs.

> >> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> >> new file mode 100644
> >> index 000000000000..152cdd487056
> >> --- /dev/null
> >> +++ b/arch/arm/mach-tangox/Kconfig
> >> @@ -0,0 +1,12 @@
> >> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
> >> +
> >> +config ARCH_TANGOX
> >> +	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
> >> +	select ARCH_HAS_HOLES_MEMORYMODEL
> >> +	select ARM_ERRATA_754322
> >> +	select ARM_ERRATA_764369 if SMP
> >> +	select ARM_GIC
> >> +	select GENERIC_IRQ_CHIP
> >> +	select HAVE_ARM_SCU
> >> +	select HAVE_ARM_TWD
> >> +	select SERIAL_8250_RT288X if SERIAL_8250
> > 
> > Do not 'select' the uart driver, that can just be part of the defconfig
> > file.
> 
> Do you mean I should provide a defconfig with my patch?
> 
> Picking SERIAL_8250 but not SERIAL_8250_RT288X makes the kernel
> panic. Doesn't that qualify for selecting it? (I don't understand
> the rationale behind most conventions in kernel dev.)

Add it to multi_v7_defconfig

	Arnd

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Mason]

On 02/10/2015 23:11, Arnd Bergmann wrote:
> On Friday 02 October 2015 22:53:11 Mason wrote:
>> On 02/10/2015 21:56, Arnd Bergmann wrote:
>>> On Friday 02 October 2015 18:02:04 Mason wrote:
>>>> Add support for Tango4-based SoCs (SMP8756, SMP8758)
>>>>
>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>
>>> Please write a proper changelog text here that tells us more about
>>> the patch than the subject line.
>>
>> Sorry, I'm quite unimaginative. What kind of information do you
>> consider required?
> 
> The line you have there is not needed, but it would be nice to have
> a link to the data sheet (if available) and some information about
> what SoCs they are.

I'll ask, but I'm afraid there is no public documentation :-(

Is this the place to state that Tango4 SoCs are based on
ARM Cortex A9 MPCore? (Whereas Tango3 was based on MIPS.)

> For most patches, you describe what the original code does wrong
> first, followed by how your patch addresses the situation, but for
> a new platform that is a bit different.
> 
>>>> --- a/arch/arm/boot/dts/Makefile
>>>> +++ b/arch/arm/boot/dts/Makefile
>>>> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>>>>  dtb-$(CONFIG_MACH_SUN9I) += \
>>>>  	sun9i-a80-optimus.dtb \
>>>>  	sun9i-a80-cubieboard4.dtb
>>>> +dtb-$(CONFIG_ARCH_TANGOX) += \
>>>> +	vantage-1172.dtb
>>>
>>> This file name should start with the name of the chip,
>>> e.g. 'tango4-vantage-1172.dtb', to make it easier to find.
>>
>> OK.
>>
>> (Why doesn't arm do it like mips, with per-manufacturer folders?)
> 
> Historic mistake on our side, and it's become too hard to fix now
> without breaking hundreds of patches that people are trying to
> get merged.

OK. And even new platforms are not put in a separate folder?
(For consistency, I imagine.)

>>>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
>>>> new file mode 100644
>>>> index 000000000000..7336fcc3ac1d
>>>> --- /dev/null
>>>> +++ b/arch/arm/boot/dts/tango4.dtsi
>>>> @@ -0,0 +1,117 @@
>>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>>> +
>>>> +/ {
>>>> +	compatible = "sigma,tango4-soc";
>>>
>>> Move the root compatible strings into the board specific file,
>>> and add the name of the machine as a more specific one.
>>
>> I don't understand this.
> 
> The compatible list should list both generic and specific names
> if applicable. For an SoC based platform, that almost always
> translates into board name, soc name and sometimes soc family name.

In my case,
board name = vantage-1172
soc name would be the specific package? "SMP8758"
soc family name would be tango4? or just tango? tangox?

Note1: the same DT should work on "SMP8756" (single core Tango4)
Note2: Mans is using a slightly different package (SMP8759 IIUC)

>>>> +	soc {
>>>> +		compatible = "simple-bus";
>>>> +		#address-cells = <1>;
>>>> +		#size-cells = <1>;
>>>> +		ranges;
>>>> +
>>>> +		xtal_in_cnt {
>>>> +			compatible = "sigma,xtal_in_cnt";
>>>> +			reg = <0x10048 0x4>;
>>>> +			clocks = <&xtal>;
>>>> +		};
>>>> +
>>>> +		uart0 {
>>>> +			compatible = "ralink,rt2880-uart";
>>>> +			reg = <0x10700 0x100>;
>>>> +			clock-frequency = <7372800>;
>>>> +			reg-shift = <2>;
>>>> +/*			fifo-size = <16>; BROKEN */
>>>> +		};
>>>
>>> Please use standard node names here, the uart should be
>>> named 'serial at 10700', the other names should be
>>> 'ethernet at 26000', 'interrupt-controller at 6e000' etc.
>>
>> Where are the standard node names documented?
> 
> ePAPR has a list, for others look at the existing dts files.
> 
>> Can I still name labels freely?
> 
> Labels can be freely assigned, they are only used as syntactic
> sugar to let you write phandle references in a more compact
> way, but names are what ends up in the dts and they should
> follow convention as much as possible.

OK.

>> Or is there a naming convention for labels?
>>
>> Why is the base address duplicated? Can't the DTC figure
>> out the address from the reg property?
> 
> I don't know what exactly has led to this, I believe it was
> like this in original open firmware, but not always enforced,
> but we probably needed to represent nodes from real firmware
> in dtb files.

OK.

>>>> +		eth0: eth0 {
>>>> +			compatible = "sigma,smp8640-emac";
>>>> +			reg = <0x26000 0x800>;
>>>> +			interrupts = <38 4>;
>>>> +			interrupt-parent = <&irq0>;
>>>> +			mac-address = [ 00 16 e8 02 08 42 ];
>>>> +			clocks = <&sysclk>;
>>>> +		};
>>>> +
>>>> +		intc: intc at e000 {
>>>> +			compatible = "sigma,tango-intc";
>>>> +			reg = <0x6e000 0x1000>;
>>>> +			interrupt-parent = <&gic>;
>>>> +			interrupt-controller;
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <1>;
>>>> +
>>>> +			irq0: irq0 at 000 {
>>>> +				reg = <0x000 0x100>;
>>>> +				interrupt-controller;
>>>> +				#interrupt-cells = <2>;
>>>> +				interrupts = <0 2 4>;
>>>> +			};
>>>
>>> You are missing a ranges property that describes what address
>>> space these addresses are in.
>>
>> ranges; is not hierarchically inherited?
> 
> 'ranges;' would be wrong here, as the interrupt controller is
> not a bus. If you have no ranges property, the bus is interpreted
> as having its own address space with no relation to the parent bus
> (e.g. an I2C bus uses addresses that are not memory mapped).
> 
> Just list the addresses that are actually decoded by child
> devices here.

Sorry, I really don't understand the "ranges" property.
I used "subsections" like "clocks" and "soc" to group related
nodes together, not because they require special handling
for address translation, or some other need.

I mean the gic is at physical address 0x20000600, and
the UART is at physical address 0x10700. I was going to
say there is nothing different, but that's not completely
accurate. Talking to the GIC doesn't leave the CPU, while
talking to the UART goes over the global bus.

But the interrupt-controller is no different than the
UART or eth. Why does this one need special care and
not the others?

>>>> diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
>>>> new file mode 100644
>>>> index 000000000000..56f6babe7093
>>>> --- /dev/null
>>>> +++ b/arch/arm/boot/dts/vantage-1172.dts
>>>> @@ -0,0 +1,8 @@
>>>> +/dts-v1/;
>>>> +
>>>> +#include "tango4.dtsi"
>>>> +
>>>> +&eth0 {
>>>> +	phy-connection-type = "rgmii";
>>>> +	max-speed = <1000>;
>>>> +};
>>>
>>> You should normally have /chosen and /aliases nodes here as well.
>>
>> Sigh. I don't know what they are for.
>> http://devicetree.org/Device_Tree_Usage#Special_Nodes
>>
>> So with aliases, I don't need to define labels in the .dtsi?
> 
> labels are always optional, you can write the aliases node using
> either labels or open-coded paths, like

I meant: in the .dtsi I wrote e.g eth0: eth0 (making an eth0
label) just so I could write &eth0 in the .dts
If I define the eth0 alias, I don't need the label anymore?
Sorry for these dumb questions. In my head, DT is really a
mess of arbitrary rules (thus hard to follow).

> /aliases {
> 	serial0 = &uart_3;
> 	serial1 = "/soc/serial at 10700";
> };
>  
>> "Typically the chosen node is left empty in .dts source files and populated at boot time."
>> Should I put my current bootargs there?
> 
> I would put only the stdout-path property in there, as long as you can
> boot with any bootargs.

I'm pretty sure I have to specify the amount of memory Linux
can use. (I do that on the command line now.)
My current boot command is:
ip=dhcp root=/dev/nfs rdinit=/none console=ttyS0,115200 mem=640M debug

>>>> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
>>>> new file mode 100644
>>>> index 000000000000..152cdd487056
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-tangox/Kconfig
>>>> @@ -0,0 +1,12 @@
>>>> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
>>>> +
>>>> +config ARCH_TANGOX
>>>> +	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
>>>> +	select ARCH_HAS_HOLES_MEMORYMODEL
>>>> +	select ARM_ERRATA_754322
>>>> +	select ARM_ERRATA_764369 if SMP
>>>> +	select ARM_GIC
>>>> +	select GENERIC_IRQ_CHIP
>>>> +	select HAVE_ARM_SCU
>>>> +	select HAVE_ARM_TWD
>>>> +	select SERIAL_8250_RT288X if SERIAL_8250
>>>
>>> Do not 'select' the uart driver, that can just be part of the defconfig
>>> file.
>>
>> Do you mean I should provide a defconfig with my patch?
>>
>> Picking SERIAL_8250 but not SERIAL_8250_RT288X makes the kernel
>> panic. Doesn't that qualify for selecting it? (I don't understand
>> the rationale behind most conventions in kernel dev.)
> 
> Add it to multi_v7_defconfig

OK. And I can make a tango4_defconfig too, right?

Regards.

[PATCH] arm-soc: Add Sigma Designs Tango4 port

[Arnd Bergmann]

On Friday 02 October 2015 23:57:07 Mason wrote:
> On 02/10/2015 23:11, Arnd Bergmann wrote:
> > On Friday 02 October 2015 22:53:11 Mason wrote:

> > The line you have there is not needed, but it would be nice to have
> > a link to the data sheet (if available) and some information about
> > what SoCs they are.
> 
> I'll ask, but I'm afraid there is no public documentation :-(
> 
> Is this the place to state that Tango4 SoCs are based on
> ARM Cortex A9 MPCore? (Whereas Tango3 was based on MIPS.)

Yes.


> >> (Why doesn't arm do it like mips, with per-manufacturer folders?)
> > 
> > Historic mistake on our side, and it's become too hard to fix now
> > without breaking hundreds of patches that people are trying to
> > get merged.
> 
> OK. And even new platforms are not put in a separate folder?
> (For consistency, I imagine.)

Right.

> >>>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
> >>>> new file mode 100644
> >>>> index 000000000000..7336fcc3ac1d
> >>>> --- /dev/null
> >>>> +++ b/arch/arm/boot/dts/tango4.dtsi
> >>>> @@ -0,0 +1,117 @@
> >>>> +#include <dt-bindings/interrupt-controller/irq.h>
> >>>> +
> >>>> +/ {
> >>>> +	compatible = "sigma,tango4-soc";
> >>>
> >>> Move the root compatible strings into the board specific file,
> >>> and add the name of the machine as a more specific one.
> >>
> >> I don't understand this.
> > 
> > The compatible list should list both generic and specific names
> > if applicable. For an SoC based platform, that almost always
> > translates into board name, soc name and sometimes soc family name.
> 
> In my case,
> board name = vantage-1172
> soc name would be the specific package? "SMP8758"
> soc family name would be tango4? or just tango? tangox?
> 
> Note1: the same DT should work on "SMP8756" (single core Tango4)
> Note2: Mans is using a slightly different package (SMP8759 IIUC)

As many of them as you find reasonable. I don't know if the 'x' in
tangox is just a wildcard of if that is the official name of the SoC
line. If it's a wildcard, don't put it into a compatible string.

It could be something like

	compatible = "sigma,tango4-smp8758-vantage-1172", "sigma,tango4-vantage-1172",
			"sigma,tango4", "sigma,tango";

> >>>> +		eth0: eth0 {
> >>>> +			compatible = "sigma,smp8640-emac";
> >>>> +			reg = <0x26000 0x800>;
> >>>> +			interrupts = <38 4>;
> >>>> +			interrupt-parent = <&irq0>;
> >>>> +			mac-address = [ 00 16 e8 02 08 42 ];
> >>>> +			clocks = <&sysclk>;
> >>>> +		};
> >>>> +
> >>>> +		intc: intc at e000 {
> >>>> +			compatible = "sigma,tango-intc";
> >>>> +			reg = <0x6e000 0x1000>;
> >>>> +			interrupt-parent = <&gic>;
> >>>> +			interrupt-controller;
> >>>> +			#address-cells = <1>;
> >>>> +			#size-cells = <1>;
> >>>> +
> >>>> +			irq0: irq0 at 000 {
> >>>> +				reg = <0x000 0x100>;
> >>>> +				interrupt-controller;
> >>>> +				#interrupt-cells = <2>;
> >>>> +				interrupts = <0 2 4>;
> >>>> +			};
> >>>
> >>> You are missing a ranges property that describes what address
> >>> space these addresses are in.
> >>
> >> ranges; is not hierarchically inherited?
> > 
> > 'ranges;' would be wrong here, as the interrupt controller is
> > not a bus. If you have no ranges property, the bus is interpreted
> > as having its own address space with no relation to the parent bus
> > (e.g. an I2C bus uses addresses that are not memory mapped).
> > 
> > Just list the addresses that are actually decoded by child
> > devices here.
> 
> Sorry, I really don't understand the "ranges" property.
> I used "subsections" like "clocks" and "soc" to group related
> nodes together, not because they require special handling
> for address translation, or some other need.
> 
> I mean the gic is at physical address 0x20000600, and
> the UART is at physical address 0x10700. I was going to
> say there is nothing different, but that's not completely
> accurate. Talking to the GIC doesn't leave the CPU, while
> talking to the UART goes over the global bus.
> 
> But the interrupt-controller is no different than the
> UART or eth. Why does this one need special care and
> not the others?

The intc at e000 (interrupt-controller at 6e000 really) node has child nodes, the
other devices are leaf nodes.

If you want to interpret the "reg" property of the irq0 at 000
(interrupt-controller at 0) node, you need to know how the 000 translates
into an address of the root bus.

I assume you meant to write

	ranges = <0 0x6e000 0x300>;

to say that address 0x000 of the child node is at address 0x6e000 of the parent
bus.

> >>>> diff --git a/arch/arm/boot/dts/vantage-1172.dts b/arch/arm/boot/dts/vantage-1172.dts
> >>>> new file mode 100644
> >>>> index 000000000000..56f6babe7093
> >>>> --- /dev/null
> >>>> +++ b/arch/arm/boot/dts/vantage-1172.dts
> >>>> @@ -0,0 +1,8 @@
> >>>> +/dts-v1/;
> >>>> +
> >>>> +#include "tango4.dtsi"
> >>>> +
> >>>> +&eth0 {
> >>>> +	phy-connection-type = "rgmii";
> >>>> +	max-speed = <1000>;
> >>>> +};
> >>>
> >>> You should normally have /chosen and /aliases nodes here as well.
> >>
> >> Sigh. I don't know what they are for.
> >> http://devicetree.org/Device_Tree_Usage#Special_Nodes
> >>
> >> So with aliases, I don't need to define labels in the .dtsi?
> > 
> > labels are always optional, you can write the aliases node using
> > either labels or open-coded paths, like
> 
> I meant: in the .dtsi I wrote e.g eth0: eth0 (making an eth0
> label) just so I could write &eth0 in the .dts
> If I define the eth0 alias, I don't need the label anymore?
> Sorry for these dumb questions. In my head, DT is really a
> mess of arbitrary rules (thus hard to follow).

The aliases are mainly useful when something else refers to them.
uart drivers tend to use them to pick the right minor device numbers,
but a lot of other drivers don't look at them.

The stdout-path property can use the alias.

> > /aliases {
> > 	serial0 = &uart_3;
> > 	serial1 = "/soc/serial at 10700";
> > };
> >  
> >> "Typically the chosen node is left empty in .dts source files and populated at boot time."
> >> Should I put my current bootargs there?
> > 
> > I would put only the stdout-path property in there, as long as you can
> > boot with any bootargs.
> 
> I'm pretty sure I have to specify the amount of memory Linux
> can use. (I do that on the command line now.)
> My current boot command is:
> ip=dhcp root=/dev/nfs rdinit=/none console=ttyS0,115200 mem=640M debug

The mem= and console= arguments should not be needed here and go through
other nodes (/memory/reg and /chosen/stdout-path). "debug" should not
be part of this normally, and the rootfs should be set by the bootloader
according to its configuration.

> >>>> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> >>>> new file mode 100644
> >>>> index 000000000000..152cdd487056
> >>>> --- /dev/null
> >>>> +++ b/arch/arm/mach-tangox/Kconfig
> >>>> @@ -0,0 +1,12 @@
> >>>> +# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
> >>>> +
> >>>> +config ARCH_TANGOX
> >>>> +	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
> >>>> +	select ARCH_HAS_HOLES_MEMORYMODEL
> >>>> +	select ARM_ERRATA_754322
> >>>> +	select ARM_ERRATA_764369 if SMP
> >>>> +	select ARM_GIC
> >>>> +	select GENERIC_IRQ_CHIP
> >>>> +	select HAVE_ARM_SCU
> >>>> +	select HAVE_ARM_TWD
> >>>> +	select SERIAL_8250_RT288X if SERIAL_8250
> >>>
> >>> Do not 'select' the uart driver, that can just be part of the defconfig
> >>> file.
> >>
> >> Do you mean I should provide a defconfig with my patch?
> >>
> >> Picking SERIAL_8250 but not SERIAL_8250_RT288X makes the kernel
> >> panic. Doesn't that qualify for selecting it? (I don't understand
> >> the rationale behind most conventions in kernel dev.)
> > 
> > Add it to multi_v7_defconfig
> 
> OK. And I can make a tango4_defconfig too, right?

I'd make a tangox_defconfig, if there is (or will likely be) a tango5 that
is compatible enough to work with the same kernel. We try to have only
one defconfig per vendor, to keep the amount of our own testing on defconfig
files reasonable.

	Arnd

[PATCH v2] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

This patch adds support for Sigma Designs "Tango4" platform, which is
built around the ARM Cortex A9 MPCore (single and dual core SoCs).

Tango4 is not to be confused with Tango3, which was built around a
MIPS 74kf CPU.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 arch/arm/Kconfig                          |   2 +
 arch/arm/Makefile                         |   1 +
 arch/arm/boot/dts/Makefile                |   2 +
 arch/arm/boot/dts/tango4-vantage-1172.dts |  17 +++++
 arch/arm/boot/dts/tango4.dtsi             | 116 ++++++++++++++++++++++++++++++
 arch/arm/mach-tangox/Kconfig              |  11 +++
 arch/arm/mach-tangox/Makefile             |   1 +
 arch/arm/mach-tangox/setup.c              |   7 ++
 8 files changed, 157 insertions(+)
 create mode 100644 arch/arm/boot/dts/tango4-vantage-1172.dts
 create mode 100644 arch/arm/boot/dts/tango4.dtsi
 create mode 100644 arch/arm/mach-tangox/Kconfig
 create mode 100644 arch/arm/mach-tangox/Makefile
 create mode 100644 arch/arm/mach-tangox/setup.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1c5021002fe4..94a1a0277c94 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
 
 source "arch/arm/mach-prima2/Kconfig"
 
+source "arch/arm/mach-tangox/Kconfig"
+
 source "arch/arm/mach-tegra/Kconfig"
 
 source "arch/arm/mach-u300/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 7451b447cc2d..7fcb4c63cdf7 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
 machine-$(CONFIG_ARCH_STI)		+= sti
 machine-$(CONFIG_ARCH_STM32)		+= stm32
 machine-$(CONFIG_ARCH_SUNXI)		+= sunxi
+machine-$(CONFIG_ARCH_TANGOX)		+= tangox
 machine-$(CONFIG_ARCH_TEGRA)		+= tegra
 machine-$(CONFIG_ARCH_U300)		+= u300
 machine-$(CONFIG_ARCH_U8500)		+= ux500
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 246473a244f6..2499295051d5 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 dtb-$(CONFIG_MACH_SUN9I) += \
 	sun9i-a80-optimus.dtb \
 	sun9i-a80-cubieboard4.dtb
+dtb-$(CONFIG_ARCH_TANGOX) += \
+	tango4-vantage-1172.dtb
 dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
 	tegra20-harmony.dtb \
 	tegra20-iris-512.dtb \
diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
new file mode 100644
index 000000000000..3eff944e2103
--- /dev/null
+++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+
+#include "tango4.dtsi"
+
+/ {
+	model = "Sigma Designs SMP8758 Vantage-1172 dev board";
+	compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";
+
+	chosen {
+		stdout-path = &uart;
+	};
+};
+
+&eth0 {
+	phy-connection-type = "rgmii";
+	max-speed = <1000>;
+};
diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
new file mode 100644
index 000000000000..6ac2e9b9cf84
--- /dev/null
+++ b/arch/arm/boot/dts/tango4.dtsi
@@ -0,0 +1,116 @@
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	compatible = "sigma,tango4";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		xtal: xtal {
+			compatible = "fixed-clock";
+			clock-frequency = <27000000>;
+			#clock-cells = <0>;
+		};
+
+		sysclk: sysclk {
+			compatible = "fixed-clock";
+			clock-frequency = <396000000>;
+			#clock-cells = <0>;
+		};
+
+		cpuclk: cpuclk {
+			compatible = "fixed-clock";
+			clock-frequency = <999000000>;
+			#clock-cells = <0>;
+		};
+
+		periphclk: periphclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpuclk>;
+			clock-mult = <1>;
+			clock-div  = <2>;
+			#clock-cells = <0>;
+		};
+	};
+
+	gic: gic at 20001000 {
+		compatible = "arm,cortex-a9-gic";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x20001000 0x1000>,
+		      <0x20000100 0x0100>;
+	};
+
+	twd-timer at 20000600 {
+		compatible = "arm,cortex-a9-twd-timer";
+		reg = <0x20000600 0x10>;
+		interrupts = <1 13 0xf01>;
+		interrupt-parent = <&gic>;
+		clocks = <&periphclk>;
+		twd_never_stops;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tick-counter at 10048 {
+			compatible = "sigma,tick-counter";
+			reg = <0x10048 0x4>;
+			clocks = <&xtal>;
+		};
+
+		uart: serial at 10700 {
+			compatible = "ralink,rt2880-uart";
+			reg = <0x10700 0x100>;
+			clock-frequency = <7372800>;
+			reg-shift = <2>;
+		};
+
+		eth0: ethernet at 26000 {
+			compatible = "sigma,tango-emac";
+			reg = <0x26000 0x800>;
+			interrupts = <38 4>;
+			interrupt-parent = <&irq0>;
+			clocks = <&sysclk>;
+		};
+
+		interrupt-controller at 6e000 {
+			compatible = "sigma,tango-intc";
+			reg = <0x6e000 0x400>;
+			ranges = <0 0x6e000 0x400>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			irq0: irq0 at 6e000 {
+				reg = <0x000 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 2 4>;
+			};
+
+			irq1: irq1 at 6e100 {
+				reg = <0x100 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 3 4>;
+			};
+
+			irq2: irq2 at 6e300 {
+				reg = <0x300 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 4 4>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
new file mode 100644
index 000000000000..b8a308f08ec6
--- /dev/null
+++ b/arch/arm/mach-tangox/Kconfig
@@ -0,0 +1,11 @@
+# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
+
+config ARCH_TANGOX
+	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
+	select ARCH_HAS_HOLES_MEMORYMODEL
+	select ARM_ERRATA_754322
+	select ARM_ERRATA_764369 if SMP
+	select ARM_GIC
+	select GENERIC_IRQ_CHIP
+	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD
diff --git a/arch/arm/mach-tangox/Makefile b/arch/arm/mach-tangox/Makefile
new file mode 100644
index 000000000000..2b9dba458932
--- /dev/null
+++ b/arch/arm/mach-tangox/Makefile
@@ -0,0 +1 @@
+obj-y += setup.o
diff --git a/arch/arm/mach-tangox/setup.c b/arch/arm/mach-tangox/setup.c
new file mode 100644
index 000000000000..46ae91e49f81
--- /dev/null
+++ b/arch/arm/mach-tangox/setup.c
@@ -0,0 +1,7 @@
+#include <asm/mach/arch.h>
+
+static const char *tango_dt_compat[] = { "sigma,tango4", NULL };
+
+DT_MACHINE_START(TANGO_DT, "Sigma Tango DT")
+	.dt_compat	= tango_dt_compat,
+MACHINE_END
-- 
2.4.5

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

This patch adds support for Sigma Designs "Tango4" platform, which is
built around the ARM Cortex A9 MPCore (single and dual core SoCs).

Tango4 is not to be confused with Tango3, which was built around a
MIPS 74kf CPU.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
v3 changes: Updated clock tree DT (clk driver submitted)
---
 arch/arm/Kconfig                          |   2 +
 arch/arm/Makefile                         |   1 +
 arch/arm/boot/dts/Makefile                |   2 +
 arch/arm/boot/dts/tango4-vantage-1172.dts |  17 ++++
 arch/arm/boot/dts/tango4.dtsi             | 133 ++++++++++++++++++++++++++++++
 arch/arm/mach-tangox/Kconfig              |  11 +++
 arch/arm/mach-tangox/Makefile             |   1 +
 arch/arm/mach-tangox/setup.c              |   7 ++
 8 files changed, 174 insertions(+)
 create mode 100644 arch/arm/boot/dts/tango4-vantage-1172.dts
 create mode 100644 arch/arm/boot/dts/tango4.dtsi
 create mode 100644 arch/arm/mach-tangox/Kconfig
 create mode 100644 arch/arm/mach-tangox/Makefile
 create mode 100644 arch/arm/mach-tangox/setup.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1c5021002fe4..94a1a0277c94 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
 
 source "arch/arm/mach-prima2/Kconfig"
 
+source "arch/arm/mach-tangox/Kconfig"
+
 source "arch/arm/mach-tegra/Kconfig"
 
 source "arch/arm/mach-u300/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 7451b447cc2d..7fcb4c63cdf7 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)		+= socfpga
 machine-$(CONFIG_ARCH_STI)		+= sti
 machine-$(CONFIG_ARCH_STM32)		+= stm32
 machine-$(CONFIG_ARCH_SUNXI)		+= sunxi
+machine-$(CONFIG_ARCH_TANGOX)		+= tangox
 machine-$(CONFIG_ARCH_TEGRA)		+= tegra
 machine-$(CONFIG_ARCH_U300)		+= u300
 machine-$(CONFIG_ARCH_U8500)		+= ux500
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 246473a244f6..2499295051d5 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
 dtb-$(CONFIG_MACH_SUN9I) += \
 	sun9i-a80-optimus.dtb \
 	sun9i-a80-cubieboard4.dtb
+dtb-$(CONFIG_ARCH_TANGOX) += \
+	tango4-vantage-1172.dtb
 dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
 	tegra20-harmony.dtb \
 	tegra20-iris-512.dtb \
diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
new file mode 100644
index 000000000000..3eff944e2103
--- /dev/null
+++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
@@ -0,0 +1,17 @@
+/dts-v1/;
+
+#include "tango4.dtsi"
+
+/ {
+	model = "Sigma Designs SMP8758 Vantage-1172 dev board";
+	compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";
+
+	chosen {
+		stdout-path = &uart;
+	};
+};
+
+&eth0 {
+	phy-connection-type = "rgmii";
+	max-speed = <1000>;
+};
diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
new file mode 100644
index 000000000000..c682617866e9
--- /dev/null
+++ b/arch/arm/boot/dts/tango4.dtsi
@@ -0,0 +1,133 @@
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	compatible = "sigma,tango4";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		xtal: xtal {
+			compatible = "fixed-clock";
+			clock-frequency = <27000000>;
+			#clock-cells = <0>;
+		};
+
+		pll0: pll0 {
+			compatible = "sigma,tango4-pll";
+			reg = <0x10000 4>;
+			clocks = <&xtal>;
+			#clock-cells = <0>;
+		};
+
+		pll1: pll1 {
+			compatible = "sigma,tango4-pll";
+			reg = <0x10008 4>;
+			clocks = <&xtal>;
+			#clock-cells = <0>;
+		};
+
+		cpuclk: cpuclk {
+			compatible = "sigma,tango4-cpuclk";
+			reg = <0x10024 4>;
+			clocks = <&pll0>;
+			#clock-cells = <0>;
+		};
+
+		periphclk: periphclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cpuclk>;
+			clock-mult = <1>;
+			clock-div  = <2>;
+			#clock-cells = <0>;
+		};
+
+		sysclk: sysclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&pll1>;
+			clock-mult = <1>;
+			clock-div  = <3>; /* HW bug precludes other dividers */
+			#clock-cells = <0>;
+		};
+	};
+
+	gic: gic at 20001000 {
+		compatible = "arm,cortex-a9-gic";
+		interrupt-controller;
+		#interrupt-cells = <3>;
+		reg = <0x20001000 0x1000>,
+		      <0x20000100 0x0100>;
+	};
+
+	twd-timer at 20000600 {
+		compatible = "arm,cortex-a9-twd-timer";
+		reg = <0x20000600 0x10>;
+		interrupts = <1 13 0xf01>;
+		interrupt-parent = <&gic>;
+		clocks = <&periphclk>;
+		twd-never-stops;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tick-counter at 10048 {
+			compatible = "sigma,tick-counter";
+			reg = <0x10048 0x4>;
+			clocks = <&xtal>;
+		};
+
+		uart: serial at 10700 {
+			compatible = "ralink,rt2880-uart";
+			reg = <0x10700 0x100>;
+			clock-frequency = <7372800>;
+			reg-shift = <2>;
+		};
+
+		eth0: ethernet at 26000 {
+			compatible = "sigma,tango-emac";
+			reg = <0x26000 0x800>;
+			interrupts = <38 4>;
+			interrupt-parent = <&irq0>;
+			clocks = <&sysclk>;
+		};
+
+		interrupt-controller at 6e000 {
+			compatible = "sigma,tango-intc";
+			reg = <0x6e000 0x400>;
+			ranges = <0 0x6e000 0x400>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			irq0: irq0 at 6e000 {
+				reg = <0x000 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 2 4>;
+			};
+
+			irq1: irq1 at 6e100 {
+				reg = <0x100 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 3 4>;
+			};
+
+			irq2: irq2 at 6e300 {
+				reg = <0x300 0x100>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 4 4>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
new file mode 100644
index 000000000000..b8a308f08ec6
--- /dev/null
+++ b/arch/arm/mach-tangox/Kconfig
@@ -0,0 +1,11 @@
+# Tango3 was based on MIPS 74kf. Tango4 is based on ARM Cortex A9 MPCore.
+
+config ARCH_TANGOX
+	bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7
+	select ARCH_HAS_HOLES_MEMORYMODEL
+	select ARM_ERRATA_754322
+	select ARM_ERRATA_764369 if SMP
+	select ARM_GIC
+	select GENERIC_IRQ_CHIP
+	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD
diff --git a/arch/arm/mach-tangox/Makefile b/arch/arm/mach-tangox/Makefile
new file mode 100644
index 000000000000..2b9dba458932
--- /dev/null
+++ b/arch/arm/mach-tangox/Makefile
@@ -0,0 +1 @@
+obj-y += setup.o
diff --git a/arch/arm/mach-tangox/setup.c b/arch/arm/mach-tangox/setup.c
new file mode 100644
index 000000000000..46ae91e49f81
--- /dev/null
+++ b/arch/arm/mach-tangox/setup.c
@@ -0,0 +1,7 @@
+#include <asm/mach/arch.h>
+
+static const char *tango_dt_compat[] = { "sigma,tango4", NULL };
+
+DT_MACHINE_START(TANGO_DT, "Sigma Tango DT")
+	.dt_compat	= tango_dt_compat,
+MACHINE_END
-- 
2.4.5

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Arnd Bergmann]

On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
> This patch adds support for Sigma Designs "Tango4" platform, which is
> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
> 
> Tango4 is not to be confused with Tango3, which was built around a
> MIPS 74kf CPU.
> 
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> v3 changes: Updated clock tree DT (clk driver submitted)
> 

Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
like he is the original author of the port.

	Arnd

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 09/10/2015 15:18, Arnd Bergmann wrote:

> On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
>> This patch adds support for Sigma Designs "Tango4" platform, which is
>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>
>> Tango4 is not to be confused with Tango3, which was built around a
>> MIPS 74kf CPU.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> v3 changes: Updated clock tree DT (clk driver submitted)
>>
> 
> Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
> like he is the original author of the port.

I have a v4 coming up to account for the change from "twd-never-stops"
to "always-on" requested by Mark Rutland.

I started the port by looking at Mans' Tango3 port, but the code
submitted was written by me (bugs included).

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Rob Herring]

On Tue, Oct 6, 2015 at 10:57 AM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> This patch adds support for Sigma Designs "Tango4" platform, which is
> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>
> Tango4 is not to be confused with Tango3, which was built around a
> MIPS 74kf CPU.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> v3 changes: Updated clock tree DT (clk driver submitted)
> ---
>  arch/arm/Kconfig                          |   2 +
>  arch/arm/Makefile                         |   1 +
>  arch/arm/boot/dts/Makefile                |   2 +
>  arch/arm/boot/dts/tango4-vantage-1172.dts |  17 ++++
>  arch/arm/boot/dts/tango4.dtsi             | 133 ++++++++++++++++++++++++++++++
>  arch/arm/mach-tangox/Kconfig              |  11 +++
>  arch/arm/mach-tangox/Makefile             |   1 +
>  arch/arm/mach-tangox/setup.c              |   7 ++
>  8 files changed, 174 insertions(+)
>  create mode 100644 arch/arm/boot/dts/tango4-vantage-1172.dts
>  create mode 100644 arch/arm/boot/dts/tango4.dtsi
>  create mode 100644 arch/arm/mach-tangox/Kconfig
>  create mode 100644 arch/arm/mach-tangox/Makefile
>  create mode 100644 arch/arm/mach-tangox/setup.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 1c5021002fe4..94a1a0277c94 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
>
>  source "arch/arm/mach-prima2/Kconfig"
>
> +source "arch/arm/mach-tangox/Kconfig"
> +
>  source "arch/arm/mach-tegra/Kconfig"
>
>  source "arch/arm/mach-u300/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 7451b447cc2d..7fcb4c63cdf7 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)              += socfpga
>  machine-$(CONFIG_ARCH_STI)             += sti
>  machine-$(CONFIG_ARCH_STM32)           += stm32
>  machine-$(CONFIG_ARCH_SUNXI)           += sunxi
> +machine-$(CONFIG_ARCH_TANGOX)          += tangox
>  machine-$(CONFIG_ARCH_TEGRA)           += tegra
>  machine-$(CONFIG_ARCH_U300)            += u300
>  machine-$(CONFIG_ARCH_U8500)           += ux500
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 246473a244f6..2499295051d5 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>  dtb-$(CONFIG_MACH_SUN9I) += \
>         sun9i-a80-optimus.dtb \
>         sun9i-a80-cubieboard4.dtb
> +dtb-$(CONFIG_ARCH_TANGOX) += \
> +       tango4-vantage-1172.dtb
>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>         tegra20-harmony.dtb \
>         tegra20-iris-512.dtb \
> diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
> new file mode 100644
> index 000000000000..3eff944e2103
> --- /dev/null
> +++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
> @@ -0,0 +1,17 @@
> +/dts-v1/;
> +
> +#include "tango4.dtsi"
> +
> +/ {
> +       model = "Sigma Designs SMP8758 Vantage-1172 dev board";
> +       compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";
> +
> +       chosen {
> +               stdout-path = &uart;
> +       };
> +};
> +
> +&eth0 {
> +       phy-connection-type = "rgmii";
> +       max-speed = <1000>;
> +};
> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
> new file mode 100644
> index 000000000000..c682617866e9
> --- /dev/null
> +++ b/arch/arm/boot/dts/tango4.dtsi
> @@ -0,0 +1,133 @@
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +       compatible = "sigma,tango4";
> +
> +       #address-cells = <1>;
> +       #size-cells = <1>;

No memory node?

cpus node?

No pl310? A9 performance mon?

Rob

[PATCH v2] arm-soc: Add support for Sigma Designs Tango4

[Rob Herring]

On Mon, Oct 5, 2015 at 11:25 AM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> This patch adds support for Sigma Designs "Tango4" platform, which is
> built around the ARM Cortex A9 MPCore (single and dual core SoCs).

> --- /dev/null
> +++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
> @@ -0,0 +1,17 @@
> +/dts-v1/;
> +
> +#include "tango4.dtsi"
> +
> +/ {
> +       model = "Sigma Designs SMP8758 Vantage-1172 dev board";
> +       compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";

Also, these compatible strings need to be documented.

Rob

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 09/10/2015 16:08, Rob Herring wrote:

> Marc Gonzalez wrote:
>
>> This patch adds support for Sigma Designs "Tango4" platform, which is
>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>
>> Tango4 is not to be confused with Tango3, which was built around a
>> MIPS 74kf CPU.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> v3 changes: Updated clock tree DT (clk driver submitted)
>> ---
>>  arch/arm/Kconfig                          |   2 +
>>  arch/arm/Makefile                         |   1 +
>>  arch/arm/boot/dts/Makefile                |   2 +
>>  arch/arm/boot/dts/tango4-vantage-1172.dts |  17 ++++
>>  arch/arm/boot/dts/tango4.dtsi             | 133 ++++++++++++++++++++++++++++++
>>  arch/arm/mach-tangox/Kconfig              |  11 +++
>>  arch/arm/mach-tangox/Makefile             |   1 +
>>  arch/arm/mach-tangox/setup.c              |   7 ++
>>  8 files changed, 174 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/tango4-vantage-1172.dts
>>  create mode 100644 arch/arm/boot/dts/tango4.dtsi
>>  create mode 100644 arch/arm/mach-tangox/Kconfig
>>  create mode 100644 arch/arm/mach-tangox/Makefile
>>  create mode 100644 arch/arm/mach-tangox/setup.c
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 1c5021002fe4..94a1a0277c94 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
>>
>>  source "arch/arm/mach-prima2/Kconfig"
>>
>> +source "arch/arm/mach-tangox/Kconfig"
>> +
>>  source "arch/arm/mach-tegra/Kconfig"
>>
>>  source "arch/arm/mach-u300/Kconfig"
>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> index 7451b447cc2d..7fcb4c63cdf7 100644
>> --- a/arch/arm/Makefile
>> +++ b/arch/arm/Makefile
>> @@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)              += socfpga
>>  machine-$(CONFIG_ARCH_STI)             += sti
>>  machine-$(CONFIG_ARCH_STM32)           += stm32
>>  machine-$(CONFIG_ARCH_SUNXI)           += sunxi
>> +machine-$(CONFIG_ARCH_TANGOX)          += tangox
>>  machine-$(CONFIG_ARCH_TEGRA)           += tegra
>>  machine-$(CONFIG_ARCH_U300)            += u300
>>  machine-$(CONFIG_ARCH_U8500)           += ux500
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 246473a244f6..2499295051d5 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>>  dtb-$(CONFIG_MACH_SUN9I) += \
>>         sun9i-a80-optimus.dtb \
>>         sun9i-a80-cubieboard4.dtb
>> +dtb-$(CONFIG_ARCH_TANGOX) += \
>> +       tango4-vantage-1172.dtb
>>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>>         tegra20-harmony.dtb \
>>         tegra20-iris-512.dtb \
>> diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
>> new file mode 100644
>> index 000000000000..3eff944e2103
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
>> @@ -0,0 +1,17 @@
>> +/dts-v1/;
>> +
>> +#include "tango4.dtsi"
>> +
>> +/ {
>> +       model = "Sigma Designs SMP8758 Vantage-1172 dev board";
>> +       compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";
>> +
>> +       chosen {
>> +               stdout-path = &uart;
>> +       };
>> +};
>> +
>> +&eth0 {
>> +       phy-connection-type = "rgmii";
>> +       max-speed = <1000>;
>> +};
>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
>> new file mode 100644
>> index 000000000000..c682617866e9
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/tango4.dtsi
>> @@ -0,0 +1,133 @@
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/ {
>> +       compatible = "sigma,tango4";
>> +
>> +       #address-cells = <1>;
>> +       #size-cells = <1>;
> 
> No memory node?
> 
> cpus node?
> 
> No pl310? A9 performance mon?

Can't these nodes be added at a later time?

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Måns Rullgård]

Arnd Bergmann <arnd@arndb.de> writes:

> On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
>> This patch adds support for Sigma Designs "Tango4" platform, which is
>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>> 
>> Tango4 is not to be confused with Tango3, which was built around a
>> MIPS 74kf CPU.
>> 
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> v3 changes: Updated clock tree DT (clk driver submitted)
>> 
>
> Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
> like he is the original author of the port.

NAK.  The clock parts are a complete shambles (compare to the patch I
sent earlier today), and the rest has a bunch of pointless and
misleading changes compared to my code.  This version won't work
correctly even on the SMP8759 hardware I have, let alone on the older
members of the chip family.  My code does.

The reason I haven't posted my patches is that I'm still working them
(albeit slowly), and I don't yet consider the code fit for upstreaming.

I've tried to work nicely with Sigma on this, but that's proving
difficult.  The patches posted so far by Marc seem to me like nothing
but poor NIH rewrites of my code.

-- 
M?ns Rullg?rd
mans at mansr.com

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Rob Herring]

On Fri, Oct 9, 2015 at 9:16 AM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> On 09/10/2015 16:08, Rob Herring wrote:
>
>> Marc Gonzalez wrote:
>>
>>> This patch adds support for Sigma Designs "Tango4" platform, which is
>>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>>
>>> Tango4 is not to be confused with Tango3, which was built around a
>>> MIPS 74kf CPU.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> v3 changes: Updated clock tree DT (clk driver submitted)
>>> ---
>>>  arch/arm/Kconfig                          |   2 +
>>>  arch/arm/Makefile                         |   1 +
>>>  arch/arm/boot/dts/Makefile                |   2 +
>>>  arch/arm/boot/dts/tango4-vantage-1172.dts |  17 ++++
>>>  arch/arm/boot/dts/tango4.dtsi             | 133 ++++++++++++++++++++++++++++++
>>>  arch/arm/mach-tangox/Kconfig              |  11 +++
>>>  arch/arm/mach-tangox/Makefile             |   1 +
>>>  arch/arm/mach-tangox/setup.c              |   7 ++
>>>  8 files changed, 174 insertions(+)
>>>  create mode 100644 arch/arm/boot/dts/tango4-vantage-1172.dts
>>>  create mode 100644 arch/arm/boot/dts/tango4.dtsi
>>>  create mode 100644 arch/arm/mach-tangox/Kconfig
>>>  create mode 100644 arch/arm/mach-tangox/Makefile
>>>  create mode 100644 arch/arm/mach-tangox/setup.c
>>>
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 1c5021002fe4..94a1a0277c94 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -934,6 +934,8 @@ source "arch/arm/mach-sunxi/Kconfig"
>>>
>>>  source "arch/arm/mach-prima2/Kconfig"
>>>
>>> +source "arch/arm/mach-tangox/Kconfig"
>>> +
>>>  source "arch/arm/mach-tegra/Kconfig"
>>>
>>>  source "arch/arm/mach-u300/Kconfig"
>>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>>> index 7451b447cc2d..7fcb4c63cdf7 100644
>>> --- a/arch/arm/Makefile
>>> +++ b/arch/arm/Makefile
>>> @@ -203,6 +203,7 @@ machine-$(CONFIG_ARCH_SOCFPGA)              += socfpga
>>>  machine-$(CONFIG_ARCH_STI)             += sti
>>>  machine-$(CONFIG_ARCH_STM32)           += stm32
>>>  machine-$(CONFIG_ARCH_SUNXI)           += sunxi
>>> +machine-$(CONFIG_ARCH_TANGOX)          += tangox
>>>  machine-$(CONFIG_ARCH_TEGRA)           += tegra
>>>  machine-$(CONFIG_ARCH_U300)            += u300
>>>  machine-$(CONFIG_ARCH_U8500)           += ux500
>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>>> index 246473a244f6..2499295051d5 100644
>>> --- a/arch/arm/boot/dts/Makefile
>>> +++ b/arch/arm/boot/dts/Makefile
>>> @@ -605,6 +605,8 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>>>  dtb-$(CONFIG_MACH_SUN9I) += \
>>>         sun9i-a80-optimus.dtb \
>>>         sun9i-a80-cubieboard4.dtb
>>> +dtb-$(CONFIG_ARCH_TANGOX) += \
>>> +       tango4-vantage-1172.dtb
>>>  dtb-$(CONFIG_ARCH_TEGRA_2x_SOC) += \
>>>         tegra20-harmony.dtb \
>>>         tegra20-iris-512.dtb \
>>> diff --git a/arch/arm/boot/dts/tango4-vantage-1172.dts b/arch/arm/boot/dts/tango4-vantage-1172.dts
>>> new file mode 100644
>>> index 000000000000..3eff944e2103
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/tango4-vantage-1172.dts
>>> @@ -0,0 +1,17 @@
>>> +/dts-v1/;
>>> +
>>> +#include "tango4.dtsi"
>>> +
>>> +/ {
>>> +       model = "Sigma Designs SMP8758 Vantage-1172 dev board";
>>> +       compatible = "sigma,vantage-1172", "sigma,smp8758", "sigma,tango4";
>>> +
>>> +       chosen {
>>> +               stdout-path = &uart;
>>> +       };
>>> +};
>>> +
>>> +&eth0 {
>>> +       phy-connection-type = "rgmii";
>>> +       max-speed = <1000>;
>>> +};
>>> diff --git a/arch/arm/boot/dts/tango4.dtsi b/arch/arm/boot/dts/tango4.dtsi
>>> new file mode 100644
>>> index 000000000000..c682617866e9
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/tango4.dtsi
>>> @@ -0,0 +1,133 @@
>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +/ {
>>> +       compatible = "sigma,tango4";
>>> +
>>> +       #address-cells = <1>;
>>> +       #size-cells = <1>;
>>
>> No memory node?
>>
>> cpus node?
>>
>> No pl310? A9 performance mon?
>
> Can't these nodes be added at a later time?

IMO, no, you add anything for which bindings already exist. For
bindings you have to figure out sure, those can come later. Certainly
the memory node should be there. Bootloaders generally only expect to
adjust the memory range, not add everything.

Rob

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Mason]

On 09/10/2015 15:18, Arnd Bergmann wrote:
> On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
>> This patch adds support for Sigma Designs "Tango4" platform, which is
>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>
>> Tango4 is not to be confused with Tango3, which was built around a
>> MIPS 74kf CPU.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> v3 changes: Updated clock tree DT (clk driver submitted)
>>
> 
> Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
> like he is the original author of the port.

Arnd,

It seems that Mans has a problem with my submission :-(

I understand that there is some bad blood between him and Sigma.
I also understand that Sigma's open source track record is below par.

However, I think it needs to be pointed out that:

1) I don't speak for Sigma, I merely work for them; and I always
try to "do the right thing".

2) I accept valid technical criticism of my code, but Mans' points
are sometimes clouded in a bit of hyperbole. (Especially wrt to the
clk driver)

Looking at the diffstat:

 arch/arm/Kconfig                          |   2 +
 arch/arm/Makefile                         |   1 +
 arch/arm/boot/dts/Makefile                |   2 +
 arch/arm/boot/dts/tango4-vantage-1172.dts |  17 ++++
 arch/arm/boot/dts/tango4.dtsi             | 133 ++++++++++++++++++++++++++++++
 arch/arm/mach-tangox/Kconfig              |  11 +++
 arch/arm/mach-tangox/Makefile             |   1 +
 arch/arm/mach-tangox/setup.c              |   7 ++

it can be argued that /all/ my changes are trivial, except for
the device tree. Perhaps I should credit Mans by adding his
copyright at the top of the tango4.dtsi? (Although he has
expressed disdain for it, so I'm not sure he would welcome
such an addition.)

Arnd, I understand you are the arm-soc maintainer? How am I
supposed to resolve this gridlock?

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Måns Rullgård]

Mason <slash.tmp@free.fr> writes:

> On 09/10/2015 15:18, Arnd Bergmann wrote:
>> On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
>>> This patch adds support for Sigma Designs "Tango4" platform, which is
>>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>>
>>> Tango4 is not to be confused with Tango3, which was built around a
>>> MIPS 74kf CPU.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> v3 changes: Updated clock tree DT (clk driver submitted)
>>>
>> 
>> Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
>> like he is the original author of the port.
>
> Arnd,
>
> It seems that Mans has a problem with my submission :-(
>
> I understand that there is some bad blood between him and Sigma.
> I also understand that Sigma's open source track record is below par.
>
> However, I think it needs to be pointed out that:
>
> 1) I don't speak for Sigma,

You've previously said that you do:
http://www.spinics.net/lists/arm-kernel/msg449191.html

> I merely work for them; and I always try to "do the right thing".
>
> 2) I accept valid technical criticism of my code, but Mans' points
> are sometimes clouded in a bit of hyperbole. (Especially wrt to the
> clk driver)

You keep insisting that your overly simplistic driver is smaller and
therefore better.  The truth is that the clock tree in these chips is
somewhat complex, and the right thing to do is to represent it as
accurately as possible.  If you do not, it will fail in some setup or
other, and this is not just a hypothetical.  Your clock driver gives the
wrong frequencies on my boards.

-- 
M?ns Rullg?rd
mans at mansr.com

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Mason]

On 09/10/2015 22:24, M?ns Rullg?rd wrote:

> Mason wrote:
> 
>> On 09/10/2015 15:18, Arnd Bergmann wrote:
>>> On Tuesday 06 October 2015 17:57:00 Marc Gonzalez wrote:
>>>> This patch adds support for Sigma Designs "Tango4" platform, which is
>>>> built around the ARM Cortex A9 MPCore (single and dual core SoCs).
>>>>
>>>> Tango4 is not to be confused with Tango3, which was built around a
>>>> MIPS 74kf CPU.
>>>>
>>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>>> ---
>>>> v3 changes: Updated clock tree DT (clk driver submitted)
>>>>
>>>
>>> Looks all reasonable to me now. Can I get an Ack from M?ns? It sounds
>>> like he is the original author of the port.
>>
>> Arnd,
>>
>> It seems that Mans has a problem with my submission :-(
>>
>> I understand that there is some bad blood between him and Sigma.
>> I also understand that Sigma's open source track record is below par.
>>
>> However, I think it needs to be pointed out that:
>>
>> 1) I don't speak for Sigma,
> 
> You've previously said that you do:
> http://www.spinics.net/lists/arm-kernel/msg449191.html

"As far as the upstreaming process is concerned, I speak for Sigma."
means "Sigma allows me to handle the upstreaming process details."

"I don't speak for Sigma." means
"I am not responsible for Sigma's past (or present) business
decisions, as I have no influence on that process, apart from
requesting time for working on upstreaming."

>> I merely work for them; and I always try to "do the right thing".
>>
>> 2) I accept valid technical criticism of my code, but Mans' points
>> are sometimes clouded in a bit of hyperbole. (Especially wrt to the
>> clk driver)
> 
> You keep insisting that your overly simplistic driver is smaller and
> therefore better.  The truth is that the clock tree in these chips is
> somewhat complex, and the right thing to do is to represent it as
> accurately as possible.  If you do not, it will fail in some setup or
> other, and this is not just a hypothetical.  Your clock driver gives the
> wrong frequencies on my boards.

Let me get this straight.

You took my clk driver, labeled "Tango4 cpuclk driver", named clk-tango4.c,
which exports compatible "sigma,tango4-pll" and "sigma,tango4-cpuclk" and
which reads a register that doesn't exist on Tango3.

Then you took it for a spin on your Tango3 boards, and... would you look
at that! It doesn't even work.

Why don't you submit your clk driver for Tango3, and just let me finish
the Tango4 port? I have gone out of my way to send you a dev board and
some documentation, but now you just keep sniping at my work. Why?

EOT

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 09/10/2015 16:08, Rob Herring wrote:

> No memory node?

"3.4 Memory node

A memory device node is required for all device trees and describes
the physical memory layout for the system. If a system has multiple
ranges of memory, multiple memory nodes can be created, or the ranges
can be specified in the reg property of a single memory node."

(This is a board property then.)

Suppose a board provides 2 GB of RAM, spanning physaddr 0x8000_0000
to 0xFFFF_FFFF; the memory node should be written like this?

	memory at 80000000 {
		device_type = "memory";
		reg = <0x80000000 0x80000000>; /* 2 GB */
	};

Does it make a difference if the 2 GB are provided by 1, 2, or even
4 memory modules?

Assume a different board only provides 1 GB using two 512 MB modules
DIMM0: 0x8000_0000 to 0xA000_000
DIMM1: 0xC000_0000 to 0xE000_000
What would the memory node look like?
(Do I have to set #address-cells=2 and #size-cells=1?)

> cpus node?

Is this used to document the CPU?
I didn't see any code making use of that information.

> No pl310? A9 performance mon?

About the pl310, it seems my SoC is one of a few running in ARM's
"non-secure" world (TrustZone thingy).

Russell discussed this topic in:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/441454/focus=441806

AFAIU, the firmware on my platform was made to behave like OMAP's.
I suppose this means I can copy OMAP's DT and corresponding code
for L2 interaction.

omap4_l2c310_write_sec, omap_smc1

Russell mentioned .l2c_aux_mask and .l2c_aux_val

$ git grep l2c_aux_ arch/arm/kernel/
arch/arm/kernel/irq.c:      (machine_desc->l2c_aux_mask || machine_desc->l2c_aux_val)) {
arch/arm/kernel/irq.c:          ret = l2x0_of_init(machine_desc->l2c_aux_val,
arch/arm/kernel/irq.c:                             machine_desc->l2c_aux_mask);

They seem to be used exclusively in the l2x0_of_init call.
Are they documented somewhere?

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Rob Herring]

On Tue, Oct 13, 2015 at 10:54 AM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> On 09/10/2015 16:08, Rob Herring wrote:
>
>> No memory node?
>
> "3.4 Memory node
>
> A memory device node is required for all device trees and describes
> the physical memory layout for the system. If a system has multiple
> ranges of memory, multiple memory nodes can be created, or the ranges
> can be specified in the reg property of a single memory node."
>
> (This is a board property then.)
>
> Suppose a board provides 2 GB of RAM, spanning physaddr 0x8000_0000
> to 0xFFFF_FFFF; the memory node should be written like this?
>
>         memory at 80000000 {
>                 device_type = "memory";
>                 reg = <0x80000000 0x80000000>; /* 2 GB */
>         };
>
> Does it make a difference if the 2 GB are provided by 1, 2, or even
> 4 memory modules?

No, as long as the OS supports that and Linux does.

> Assume a different board only provides 1 GB using two 512 MB modules
> DIMM0: 0x8000_0000 to 0xA000_000
> DIMM1: 0xC000_0000 to 0xE000_000
> What would the memory node look like?
> (Do I have to set #address-cells=2 and #size-cells=1?)

reg = <0x80000000 0x20000000>,
 <0xc0000000 0x20000000>;

#address-cells is how big the address is (in 32-bit words), not how
many address ranges you have.

>
>> cpus node?
>
> Is this used to document the CPU?
> I didn't see any code making use of that information.

The SMP code uses it: arch/arm/kernel/devtree.c

>> No pl310? A9 performance mon?
>
> About the pl310, it seems my SoC is one of a few running in ARM's
> "non-secure" world (TrustZone thingy).

highbank is also.

>
> Russell discussed this topic in:
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/441454/focus=441806
>
> AFAIU, the firmware on my platform was made to behave like OMAP's.
> I suppose this means I can copy OMAP's DT and corresponding code
> for L2 interaction.
>
> omap4_l2c310_write_sec, omap_smc1

Right. Highbank is similar.

> Russell mentioned .l2c_aux_mask and .l2c_aux_val
>
> $ git grep l2c_aux_ arch/arm/kernel/
> arch/arm/kernel/irq.c:      (machine_desc->l2c_aux_mask || machine_desc->l2c_aux_val)) {
> arch/arm/kernel/irq.c:          ret = l2x0_of_init(machine_desc->l2c_aux_val,
> arch/arm/kernel/irq.c:                             machine_desc->l2c_aux_mask);
>
> They seem to be used exclusively in the l2x0_of_init call.
> Are they documented somewhere?

The register is in the PL310 TRM.

Ideally, your firmware should set aux ctrl register and you only need
to enable the L2.

Rob

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 13/10/2015 19:55, Rob Herring wrote:
> Marc Gonzalez wrote:
>> On 09/10/2015 16:08, Rob Herring wrote:
>>
>>> No cpus node?
>>
>> Is this used to document the CPU?
>> I didn't see any code making use of that information.
> 
> The SMP code uses it: arch/arm/kernel/devtree.c

Now I see arm_dt_init_cpu_maps()

<confused> I thought DT was used to specify things that cannot be
dynamically discovered? Isn't it possible for the OS to discover at
run-time how many cores and/or CPUs are present?

On a related topic, I have a DTS for my board, which includes the
DTS for the architecture. However, there are single-core SoCs and
dual-core SoCs. Where is the cpus node supposed to appear?

Or should I specify in the architecture DTS the maximum number
of cores, as in

    cpus {
        #address-cells = <1>;
        #size-cells = <0>;
        cpu at 0 {
            compatible = "arm,cortex-a9";
            reg = <0>;
        };
        cpu at 1 {
            compatible = "arm,cortex-a9";
            reg = <1>;
        };
    };


>>> No pl310? A9 performance mon?

Found "arm,cortex-a9-pmu" in arch/arm/kernel/perf_event_v7.c
Are the PMU registers available from non-secure mode, or is TrustZone
going to get in the way?


About the cache controller, I was confused by this comment:
	/*
	 * Always enable non-secure access to the lockdown registers -
	 * we write to them as part of the L2C enable sequence so they
	 * need to be accessible.
	 */
	l2x0_saved_regs.aux_ctrl = aux | L310_AUX_CTRL_NS_LOCKDOWN;

I see no lock() function, only unlock().

But the unlock function merely writes 0 to the relevant registers,
and 0 is the value at reset for those registers. Since nothing ever
sets the registers to non-zero, why is the unlock needed at all?


Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Rob Herring]

On Mon, Oct 19, 2015 at 6:09 AM, Marc Gonzalez
<marc_gonzalez@sigmadesigns.com> wrote:
> On 13/10/2015 19:55, Rob Herring wrote:
>> Marc Gonzalez wrote:
>>> On 09/10/2015 16:08, Rob Herring wrote:
>>>
>>>> No cpus node?
>>>
>>> Is this used to document the CPU?
>>> I didn't see any code making use of that information.
>>
>> The SMP code uses it: arch/arm/kernel/devtree.c
>
> Now I see arm_dt_init_cpu_maps()
>
> <confused> I thought DT was used to specify things that cannot be
> dynamically discovered? Isn't it possible for the OS to discover at
> run-time how many cores and/or CPUs are present?

Yes, at first we didn't have them either. It is all the other things
associated with the cpu's that we need such as enable-method prop,
clocks, power domains, etc. that cause you to need them.

> On a related topic, I have a DTS for my board, which includes the
> DTS for the architecture. However, there are single-core SoCs and
> dual-core SoCs. Where is the cpus node supposed to appear?

You could do 3 levels of dts includes (common, SOC, board), or you
could put both cores in marking the second core disabled, and then
enable it in the bootloader checking some fuse or id. Kind of depends
on how different the chips are and how you want to manage dtb files.

> Or should I specify in the architecture DTS the maximum number
> of cores, as in
>
>     cpus {
>         #address-cells = <1>;
>         #size-cells = <0>;
>         cpu at 0 {
>             compatible = "arm,cortex-a9";
>             reg = <0>;
>         };
>         cpu at 1 {
>             compatible = "arm,cortex-a9";
>             reg = <1>;
>         };
>     };
>
>
>>>> No pl310? A9 performance mon?
>
> Found "arm,cortex-a9-pmu" in arch/arm/kernel/perf_event_v7.c
> Are the PMU registers available from non-secure mode, or is TrustZone
> going to get in the way?

I don't recall off-hand.

> About the cache controller, I was confused by this comment:
>         /*
>          * Always enable non-secure access to the lockdown registers -
>          * we write to them as part of the L2C enable sequence so they
>          * need to be accessible.
>          */
>         l2x0_saved_regs.aux_ctrl = aux | L310_AUX_CTRL_NS_LOCKDOWN;
>
> I see no lock() function, only unlock().
>
> But the unlock function merely writes 0 to the relevant registers,
> and 0 is the value at reset for those registers. Since nothing ever
> sets the registers to non-zero, why is the unlock needed at all?

It was because some bootloaders set those registers. Linux just wants
them to be all unlocked.

Rob

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Mark Rutland]

> >>>> No pl310? A9 performance mon?
> >
> > Found "arm,cortex-a9-pmu" in arch/arm/kernel/perf_event_v7.c
> > Are the PMU registers available from non-secure mode, or is TrustZone
> > going to get in the way?
> 
> I don't recall off-hand.

Judging by the access permissions tables in c12.9 of the ARM ARM (ARM DDI
0406C.c), they're always accessible from non-secure PL1 in the absence
of the virtualization extensions.

Thanks,
Mark.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 19/10/2015 19:32, Mark Rutland wrote:

> Marc Gonzalez wrote:
>
>> Found "arm,cortex-a9-pmu" in arch/arm/kernel/perf_event_v7.c
>> Are the PMU registers available from non-secure mode, or is
>> TrustZone going to get in the way?
> 
> Judging by the access permissions tables in c12.9 of the ARM ARM (ARM DDI
> 0406C.c), they're always accessible from non-secure PL1 in the absence
> of the virtualization extensions.

Thanks for digging up the info.

One more thing is unclear about the PMU. While things like the TWD block
seem to have a well-defined IRQ number, when I look at other platforms'
DT pmu node, everyone seems to have a different IRQ setup. Why is that?

Some use GIC_SPI, some use GIC_PPI
Some list 1 interrupt, others 2, others 4
Some have 3 cells (as expected by the GIC), exynos4 only has 2 (interrupts = <2 2>, <3 2>;)
Some use IRQ_TYPE_EDGE_RISING, others use IRQ_TYPE_LEVEL_HIGH

My SoC documentation only states:
1.12.2 ARM core interrupt vector
ARM A9MP core has a 32-bit interrupt vector that drives the ARM interrupt controller (GIC).
Input vector is the following :
- bit[1:0]: 0 / Unused.
- bit[2]: cpu_block IRQ controller0.
- bit[3]: cpu_block IRQ controller1.
- bit[4]: cpu_block IRQ controller2.
- bit[7:5]: 0 / Unused.
- bit[11:8]: Core N cross trigger interface IRQ (Coresight component).
- bit[15:12]: Core N performance management unit IRQ (Coresight component) .
- bit[31:16]: 0 / Unused.

So I'm thinking I should use
- GIC_SPI
- interrupts 12 and 13 (should I list the other lines if no SoC has more than 2 cores?)
- no idea on edge vs level, I'm guessing level

So I should add

	pmu {
		compatible = "arm,cortex-a9-pmu";
		interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
	};

Does that look (likely to be) correct?

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 19/10/2015 18:39, Rob Herring wrote:

> Marc Gonzalez wrote:
>
>> About the cache controller, I was confused by this comment:
>>         /*
>>          * Always enable non-secure access to the lockdown registers -
>>          * we write to them as part of the L2C enable sequence so they
>>          * need to be accessible.
>>          */
>>         l2x0_saved_regs.aux_ctrl = aux | L310_AUX_CTRL_NS_LOCKDOWN;
>>
>> I see no lock() function, only unlock().
>>
>> But the unlock function merely writes 0 to the relevant registers,
>> and 0 is the value at reset for those registers. Since nothing ever
>> sets the registers to non-zero, why is the unlock needed at all?
> 
> It was because some bootloaders set those registers. Linux just wants
> them to be all unlocked.

I see.

My problem then, is that my current firmware does not set L310_AUX_CTRL_NS_LOCKDOWN
and does not allow updating that bit.

So when l2c_unlock() is called, Linux (running in non-secure mode)
tries to write to read-only registers:

> On reset, the Non-Secure Lockdown Enable bit is set to 0 and Lockdown
> Registers are not permitted to be modified by non-secure accesses. In
> that configuration, if a non-secure access tries to write to those
> registers, the write response returns a DECERR response. This decode 
> error results in the registers not being updated.

I suppose "a DECERR response" means Linux will oops?

I see several options to work-around this problem:

A) Have the firmware set L310_AUX_CTRL_NS_LOCKDOWN at boot

B) Have the firmware allow Linux to set L310_AUX_CTRL_NS_LOCKDOWN

C) Have a way in Linux to define .unlock as a NOP (trusting the
firmware to NOT have locked anything)
Perhaps adding a "no-unlock-required;" boolean property to the l2cc
node, which would override the .unlock method?

I'd like to hear suggestions on the "best" approach.

Regards.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Russell King - ARM Linux]

On Tue, Oct 20, 2015 at 11:50:14AM +0200, Marc Gonzalez wrote:
> My problem then, is that my current firmware does not set L310_AUX_CTRL_NS_LOCKDOWN
> and does not allow updating that bit.

And if the bit isn't set, then l2c_unlock won't be called due to:

static void l2c310_unlock(void __iomem *base, unsigned num_lock)
{
        if (readl_relaxed(base + L2X0_AUX_CTRL) & L310_AUX_CTRL_NS_LOCKDOWN)
                l2c_unlock(base, num_lock);
}

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] arm-soc: Add support for Sigma Designs Tango4

[Marc Gonzalez]

On 20/10/2015 12:04, Russell King - ARM Linux wrote:
> On Tue, Oct 20, 2015 at 11:50:14AM +0200, Marc Gonzalez wrote:
>> My problem then, is that my current firmware does not set L310_AUX_CTRL_NS_LOCKDOWN
>> and does not allow updating that bit.
> 
> And if the bit isn't set, then l2c_unlock won't be called due to:
> 
> static void l2c310_unlock(void __iomem *base, unsigned num_lock)
> {
>         if (readl_relaxed(base + L2X0_AUX_CTRL) & L310_AUX_CTRL_NS_LOCKDOWN)
>                 l2c_unlock(base, num_lock);
> }

Thanks Russell, that's one less problem on my plate.

BTW, did you see my question about the CP15 FLOZ bit + prefetch bits?

Regards.