Message ID | 5613EF4C.30603@sigmadesigns.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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.
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; > + }; > +}; > + > +ð0 { > + 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
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; >> + }; >> +}; >> + >> +ð0 { >> + 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.
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.
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; >>> + }; >>> +}; >>> + >>> +ð0 { >>> + 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
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.
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.
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
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@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.
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@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
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@0 { compatible = "arm,cortex-a9"; reg = <0>; }; cpu@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.
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@0 { > compatible = "arm,cortex-a9"; > reg = <0>; > }; > cpu@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
> >>>> 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.
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.
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.
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); }
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.
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; + }; +}; + +ð0 { + 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@20001000 { + compatible = "arm,cortex-a9-gic"; + interrupt-controller; + #interrupt-cells = <3>; + reg = <0x20001000 0x1000>, + <0x20000100 0x0100>; + }; + + twd-timer@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@10048 { + compatible = "sigma,tick-counter"; + reg = <0x10048 0x4>; + clocks = <&xtal>; + }; + + uart: serial@10700 { + compatible = "ralink,rt2880-uart"; + reg = <0x10700 0x100>; + clock-frequency = <7372800>; + reg-shift = <2>; + }; + + eth0: ethernet@26000 { + compatible = "sigma,tango-emac"; + reg = <0x26000 0x800>; + interrupts = <38 4>; + interrupt-parent = <&irq0>; + clocks = <&sysclk>; + }; + + interrupt-controller@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@6e000 { + reg = <0x000 0x100>; + interrupt-controller; + #interrupt-cells = <2>; + interrupts = <0 2 4>; + }; + + irq1: irq1@6e100 { + reg = <0x100 0x100>; + interrupt-controller; + #interrupt-cells = <2>; + interrupts = <0 3 4>; + }; + + irq2: irq2@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
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