| Message ID | 564C9558.4020100@sigmadesigns.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" > platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the > Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). > > Support for older MIPS-based platforms can be found elsewhere: > https://github.com/mansr/linux-tangox > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> > --- > arch/arm/Kconfig | 2 ++ > arch/arm/Makefile | 1 + > arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ > arch/arm/mach-tangox/Makefile | 2 ++ > arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ > arch/arm/mach-tangox/smc.S | 9 +++++++++ > arch/arm/mach-tangox/smc.h | 5 +++++ Potential bike-shed fodder, but, a dumb question: is the family name actually "tangox" or is the "x" for the number (tango3, tango4, etc.) Assuming it's the later based on usage throughout the patch, I think it'd be better to just use "tango" throughout instead of tangox. Also a MAINTAINERS file entry is appropriate for this new platform support (as scripts/checkpatch.pl should have told you.) Kevin
Kevin Hilman <khilman@kernel.org> writes: > Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > >> Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" >> platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the >> Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). >> >> Support for older MIPS-based platforms can be found elsewhere: >> https://github.com/mansr/linux-tangox >> >> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >> --- >> arch/arm/Kconfig | 2 ++ >> arch/arm/Makefile | 1 + >> arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ >> arch/arm/mach-tangox/Makefile | 2 ++ >> arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ >> arch/arm/mach-tangox/smc.S | 9 +++++++++ >> arch/arm/mach-tangox/smc.h | 5 +++++ > > Potential bike-shed fodder, but, a dumb question: is the family name > actually "tangox" or is the "x" for the number (tango3, tango4, etc.) > > Assuming it's the later based on usage throughout the patch, I think > it'd be better to just use "tango" throughout instead of tangox. The x indeed stands for a number. I have no idea what tango1 was or if it ever existed. Tango2 (SMP863x) and tango3 (SMP86[457]x) were MIPS based. Tango4 is ARM based (mostly, the SMP8910 is MIPS) but otherwise very similar to tango3. Since we don't know what tango5 will look like, mach-tango4 might be more suitable here. If tango5 turns out to be sufficiently similar, there's no harm from adding support for that to the mach-tango4 code (just look at mach-omap2). Most of the drivers support both tango3 and tango4, but apparently some changes are planned for tango5.
Måns Rullgård <mans@mansr.com> writes: > Kevin Hilman <khilman@kernel.org> writes: > >> Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: >> >>> Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" >>> platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the >>> Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). >>> >>> Support for older MIPS-based platforms can be found elsewhere: >>> https://github.com/mansr/linux-tangox >>> >>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >>> --- >>> arch/arm/Kconfig | 2 ++ >>> arch/arm/Makefile | 1 + >>> arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ >>> arch/arm/mach-tangox/Makefile | 2 ++ >>> arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ >>> arch/arm/mach-tangox/smc.S | 9 +++++++++ >>> arch/arm/mach-tangox/smc.h | 5 +++++ >> >> Potential bike-shed fodder, but, a dumb question: is the family name >> actually "tangox" or is the "x" for the number (tango3, tango4, etc.) >> >> Assuming it's the later based on usage throughout the patch, I think >> it'd be better to just use "tango" throughout instead of tangox. > > The x indeed stands for a number. I have no idea what tango1 was or if > it ever existed. Tango2 (SMP863x) and tango3 (SMP86[457]x) were MIPS > based. Tango4 is ARM based (mostly, the SMP8910 is MIPS) but otherwise > very similar to tango3. Thanks for the clarification. > Since we don't know what tango5 will look like, > mach-tango4 might be more suitable here. If tango5 turns out to be > sufficiently similar, there's no harm from adding support for that to > the mach-tango4 code (just look at mach-omap2). Well, mach-omap2 leads to enough confusion that I don't think we need to use that as a model. ;) IMO, mach-tango is a better starting point. > Most of the drivers support both tango3 and tango4, but apparently some > changes are planned for tango5. Thanks for the clarification, Kevin
Kevin Hilman wrote: > Marc Gonzalez wrote: > >> Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" >> platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the >> Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). >> >> Support for older MIPS-based platforms can be found elsewhere: >> https://github.com/mansr/linux-tangox >> >> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >> --- >> arch/arm/Kconfig | 2 ++ >> arch/arm/Makefile | 1 + >> arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ >> arch/arm/mach-tangox/Makefile | 2 ++ >> arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ >> arch/arm/mach-tangox/smc.S | 9 +++++++++ >> arch/arm/mach-tangox/smc.h | 5 +++++ > > Potential bike-shed fodder, but, a dumb question: is the family name > actually "tangox" or is the "x" for the number (tango3, tango4, etc.) > > Assuming it's the later based on usage throughout the patch, I think > it'd be better to just use "tango" throughout instead of tangox. I should just change tangox to tango everywhere? This port supports tango4. I will submit a tango5 port in 2016. Does that change anything? > Also a MAINTAINERS file entry is appropriate for this new platform > support (as scripts/checkpatch.pl should have told you.) Thanks for pointing that out. I'll send a v10. Are these the only issues in your opinion? Regards.
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > Kevin Hilman wrote: > >> Marc Gonzalez wrote: >> >>> Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" >>> platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the >>> Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). >>> >>> Support for older MIPS-based platforms can be found elsewhere: >>> https://github.com/mansr/linux-tangox >>> >>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> >>> --- >>> arch/arm/Kconfig | 2 ++ >>> arch/arm/Makefile | 1 + >>> arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ >>> arch/arm/mach-tangox/Makefile | 2 ++ >>> arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ >>> arch/arm/mach-tangox/smc.S | 9 +++++++++ >>> arch/arm/mach-tangox/smc.h | 5 +++++ >> >> Potential bike-shed fodder, but, a dumb question: is the family name >> actually "tangox" or is the "x" for the number (tango3, tango4, etc.) >> >> Assuming it's the later based on usage throughout the patch, I think >> it'd be better to just use "tango" throughout instead of tangox. > > I should just change tangox to tango everywhere? IMO, yes. > This port supports tango4. I will submit a tango5 port in 2016. > Does that change anything? Probably not. I'm assuming it's an SoC in the same family, so the goal should be to support both from the same mach dir. Most of the "real" support should end up in drivers/* and DT descriptions, so the mach directory should stay very small. >> Also a MAINTAINERS file entry is appropriate for this new platform >> support (as scripts/checkpatch.pl should have told you.) > > Thanks for pointing that out. I'll send a v10. > Are these the only issues in your opinion? I have a couple comments, I'll reply separately. Kevin
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" > platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the > Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). > > Support for older MIPS-based platforms can be found elsewhere: > https://github.com/mansr/linux-tangox > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> [...] > +static void tango_l2c_write(unsigned long val, unsigned int reg) > +{ > + pr_debug("%s: reg=0x%x val=0x%lx\n", __func__, reg, val); leftover debugging aid? > + if (reg == L2X0_CTRL) > + tango_set_l2_control(val); > +} > + [...] > diff --git a/arch/arm/mach-tangox/smc.S b/arch/arm/mach-tangox/smc.S > new file mode 100644 > index 000000000000..5d932ce3c1bd > --- /dev/null > +++ b/arch/arm/mach-tangox/smc.S > @@ -0,0 +1,9 @@ > +#include <linux/linkage.h> > + > +ENTRY(tango_smc) > + push {lr} > + mov ip, r1 > + dsb /* This barrier is probably unnecessary */ Then remove it? > + smc #0 > + pop {pc} > +ENDPROC(tango_smc) Otherwise looks pretty simple and straight forward to me. FWIW, one of the benefits of starting with the small/minimum set and adding as you go is that it's much easier on reviewers. Kevin
On 19/11/2015 20:49, Kevin Hilman wrote: > Marc Gonzalez wrote: > >> +static void tango_l2c_write(unsigned long val, unsigned int reg) >> +{ >> + pr_debug("%s: reg=0x%x val=0x%lx\n", __func__, reg, val); > > leftover debugging aid? I'll remove it. (For my education, we're not supposed to use any pr_debug calls?) >> +ENTRY(tango_smc) >> + push {lr} >> + mov ip, r1 >> + dsb /* This barrier is probably unnecessary */ > > Then remove it? This was discussed in v8. It's probably cargo cult from OMAP, but the performance hit is negligible, and I don't have time to properly analyze the code path. I just wanted to add the comment in case someone copied my code. Regards.
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes: > On 19/11/2015 20:49, Kevin Hilman wrote: > >> Marc Gonzalez wrote: >> >>> +static void tango_l2c_write(unsigned long val, unsigned int reg) >>> +{ >>> + pr_debug("%s: reg=0x%x val=0x%lx\n", __func__, reg, val); >> >> leftover debugging aid? > > I'll remove it. > > (For my education, we're not supposed to use any pr_debug calls?) pr_debug() are fine to leave if you want them, but I assumed it was just a leftover as it didn't seem generally useful. >>> +ENTRY(tango_smc) >>> + push {lr} >>> + mov ip, r1 >>> + dsb /* This barrier is probably unnecessary */ >> >> Then remove it? > > This was discussed in v8. It's probably cargo cult from OMAP, > but the performance hit is negligible, and I don't have time > to properly analyze the code path. I just wanted to add the > comment in case someone copied my code. Sure, Kevin
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 774dc59650c5..d8f0c31f521f 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/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig new file mode 100644 index 000000000000..cf814d7336f3 --- /dev/null +++ b/arch/arm/mach-tangox/Kconfig @@ -0,0 +1,12 @@ +config ARCH_TANGOX + bool "Sigma Designs Tango4 (SMP87xx)" if ARCH_MULTI_V7 + # Cortex-A9 MPCore r3p0, PL310 r3p2 + select ARCH_HAS_HOLES_MEMORYMODEL + select ARM_ERRATA_754322 + select ARM_ERRATA_764369 if SMP + select ARM_ERRATA_775420 + select ARM_GIC + select CLKSRC_TANGO_XTAL + 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..0d7e2b5976e3 --- /dev/null +++ b/arch/arm/mach-tangox/Makefile @@ -0,0 +1,2 @@ +asflags-y += -mcpu=cortex-a9 +obj-y += setup.o smc.o diff --git a/arch/arm/mach-tangox/setup.c b/arch/arm/mach-tangox/setup.c new file mode 100644 index 000000000000..09b7540df14b --- /dev/null +++ b/arch/arm/mach-tangox/setup.c @@ -0,0 +1,32 @@ +#include <linux/smp.h> +#include <asm/mach/arch.h> +#include <asm/hardware/cache-l2x0.h> +#include "smc.h" + +static int tango4_boot_secondary(unsigned int cpu, struct task_struct *idle) +{ + tango_set_aux_boot_addr(virt_to_phys(secondary_startup)); + tango_start_aux_core(cpu); + return 0; +} + +static struct smp_operations tango4_smp_ops __initdata = { + .smp_boot_secondary = tango4_boot_secondary, +}; + +CPU_METHOD_OF_DECLARE(tango4_smp, "sigma,tango4-smp", &tango4_smp_ops); + +static void tango_l2c_write(unsigned long val, unsigned int reg) +{ + pr_debug("%s: reg=0x%x val=0x%lx\n", __func__, reg, val); + if (reg == L2X0_CTRL) + tango_set_l2_control(val); +} + +static const char *tango_dt_compat[] = { "sigma,tango4", NULL }; + +DT_MACHINE_START(TANGO_DT, "Sigma Tango DT") + .dt_compat = tango_dt_compat, + .l2c_aux_mask = ~0, + .l2c_write_sec = tango_l2c_write, +MACHINE_END diff --git a/arch/arm/mach-tangox/smc.S b/arch/arm/mach-tangox/smc.S new file mode 100644 index 000000000000..5d932ce3c1bd --- /dev/null +++ b/arch/arm/mach-tangox/smc.S @@ -0,0 +1,9 @@ +#include <linux/linkage.h> + +ENTRY(tango_smc) + push {lr} + mov ip, r1 + dsb /* This barrier is probably unnecessary */ + smc #0 + pop {pc} +ENDPROC(tango_smc) diff --git a/arch/arm/mach-tangox/smc.h b/arch/arm/mach-tangox/smc.h new file mode 100644 index 000000000000..7a4af35cc390 --- /dev/null +++ b/arch/arm/mach-tangox/smc.h @@ -0,0 +1,5 @@ +extern int tango_smc(unsigned int val, unsigned int service); + +#define tango_set_l2_control(val) tango_smc(val, 0x102) +#define tango_start_aux_core(val) tango_smc(val, 0x104) +#define tango_set_aux_boot_addr(val) tango_smc((unsigned int)val, 0x105)
Add support for Sigma Designs ARM-based Tango4 "Secure Media Processor" platforms (i.e. smp8734, smp8756, smp8758, smp8759) built around the Cortex-A9 MPCore r3p0 (all dual-core, except the 8756). Support for older MIPS-based platforms can be found elsewhere: https://github.com/mansr/linux-tangox Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com> --- arch/arm/Kconfig | 2 ++ arch/arm/Makefile | 1 + arch/arm/mach-tangox/Kconfig | 12 ++++++++++++ arch/arm/mach-tangox/Makefile | 2 ++ arch/arm/mach-tangox/setup.c | 32 ++++++++++++++++++++++++++++++++ arch/arm/mach-tangox/smc.S | 9 +++++++++ arch/arm/mach-tangox/smc.h | 5 +++++ 7 files changed, 63 insertions(+) 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 create mode 100644 arch/arm/mach-tangox/smc.S create mode 100644 arch/arm/mach-tangox/smc.h