diff mbox

Steps to submit a new arch/arm port

Message ID 56016780.5080104@free.fr (mailing list archive)
State New, archived
Headers show

Commit Message

Mason Sept. 22, 2015, 2:36 p.m. UTC
On 21/09/2015 17:49, Arnd Bergmann wrote:

> On Monday 21 September 2015 17:00:08 Mason wrote:
> 
>> I've been working on an arch/arm port for some time. I've removed
>> a lot of non-essential code, and currently, what I have is:
>>
>> $ git diff --stat v4.2 my4.2
>>  Makefile                                               |    4 +-
>>  arch/arm/Kconfig                                       |   26 ++
>>  arch/arm/Makefile                                      |    1 +
>>  arch/arm/boot/dts/Makefile                             |    1 +
>>  arch/arm/boot/dts/tango.dts                            |   96 ++++++++
>>  arch/arm/kernel/smp_twd.c                              |    3 +-
>>  arch/arm/mach-tangox/Kconfig                           |   57 +++++
>>  arch/arm/mach-tangox/Makefile                          |   10 +
>>  arch/arm/mach-tangox/Makefile.boot                     |    3 +
>>  arch/arm/mach-tangox/clock-tangox.c                    |  134 ++++++++++
>>  arch/arm/mach-tangox/io.c                              |   18 ++
>>  arch/arm/mach-tangox/setup.c                           |   22 ++
>>  arch/arm/tools/mach-types                              |    1 +
>>  drivers/irqchip/Makefile                               |    1 +
>>  drivers/irqchip/irq-tangox.c                           |  234 ++++++++++++++++++
>>  drivers/net/ethernet/Kconfig                           |    1 +
>>  drivers/net/ethernet/Makefile                          |    1 +
>>  drivers/net/ethernet/sigmadesigns/Kconfig              |    7 +
>>  drivers/net/ethernet/sigmadesigns/Makefile             |    5 +
>>  drivers/net/ethernet/sigmadesigns/tangox/Kconfig       |   21 ++
>>  drivers/net/ethernet/sigmadesigns/tangox/Makefile      |    5 +
>>  drivers/net/ethernet/sigmadesigns/tangox/tangox_enet.c | 1158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/net/ethernet/sigmadesigns/tangox/tangox_enet.h |  257 +++++++++++++++++++
>>  drivers/tty/serial/8250/8250_core.c                    |    8 +-
>>  drivers/tty/serial/of_serial.c                         |    2 +-
>>  25 files changed, 2066 insertions(+), 10 deletions(-)
>>
>> (The two drivers (irqchip and ethernet) are from Mans Rullgard's tree.)
>>
>> TODO
>> Convert the clock registration code to device tree
>> Add PHY ISR to ethernet driver
>>
>>
>> Could you provide some pointers/links and guidance detailing the
>> steps required to submit a new port under arch/arm?
>> (With the current requirements: DT, ARCH_MULTIPLATFORM, etc)
> 
> A few things to be aware of:
> 
> - As you are probably aware, please split the series into multiple patches,
>   doing one thing at a time. A lot of the patches don't have dependencies
>   on one another and can just get merged as soon as they are ready

I'm supposed to use git-format-patch, right?

> [snip advice for drivers]
> - no mach-types changes please

The new way is DT_MACHINE_START?

> - Makefile.boot should not be needed

I'll try removing it altogether.

> - the clock support should probably go to drivers/clk, unless this is
>   a clocksource driver, which should go to drivers/clocksource. Add a
>   DT binding spec. If you lack DT support, don't bother sending it,
>   but send the rest anyway.

I'll take a look at Mans' DT clock code, but I want to simplify it.

> - merge io.c and setup.c into one file

Done.

> - whatever you need in smp_twd.c will have to get merged by Russell,
>   the other patches go through the arm-soc tree.

One change is a temp wart for getting twd_clk until I have proper DT.

The other change is to remove CLOCK_EVT_FEAT_C3STOP for my board.
(We discussed this in May.)

Thanks for your guidance. Just to get the ball rolling, here's
the full platform patch minus drivers.

Open question:

1) arch/arm/Kconfig vs arch/arm/mach-tangox/Kconfig
What goes in the first? What goes in the second?


Regards

Comments

Arnd Bergmann Sept. 22, 2015, 2:51 p.m. UTC | #1
On Tuesday 22 September 2015 16:36:48 Mason wrote:
> On 21/09/2015 17:49, Arnd Bergmann wrote:
> > 
> > - As you are probably aware, please split the series into multiple patches,
> >   doing one thing at a time. A lot of the patches don't have dependencies
> >   on one another and can just get merged as soon as they are ready
> 
> I'm supposed to use git-format-patch, right?

Yes, and follow Documentation/SubmittingPatches

> > [snip advice for drivers]
> > - no mach-types changes please
> 
> The new way is DT_MACHINE_START?

Right.

> > - whatever you need in smp_twd.c will have to get merged by Russell,
> >   the other patches go through the arm-soc tree.
> 
> One change is a temp wart for getting twd_clk until I have proper DT.
> 
> The other change is to remove CLOCK_EVT_FEAT_C3STOP for my board.
> (We discussed this in May.)
> 
> Thanks for your guidance. Just to get the ball rolling, here's
> the full platform patch minus drivers.
> 
> Open question:
> 
> 1) arch/arm/Kconfig vs arch/arm/mach-tangox/Kconfig
> What goes in the first? What goes in the second?

The first only gets a single line to include the second.

 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 1c5021002fe4..06002e552afd 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -758,6 +758,21 @@ config ARCH_OMAP1
>  	help
>  	  Support for older TI OMAP1 (omap7xx, omap15xx or omap16xx)
>  
> +config ARCH_TANGOX
> +	bool "Sigma Designs Tango"
> +	select CLKDEV_LOOKUP
> +	select HAVE_CLK
> +	select HAVE_SMP
> +	select ARCH_REQUIRE_GPIOLIB
> +	select ARCH_HAS_CPUFREQ
> +	select CLKSRC_MMIO
> +	select GENERIC_CLOCKEVENTS
> +	select GENERIC_IRQ_CHIP
> +	select ARCH_HAS_HOLES_MEMORYMODEL
> +	select SPARSE_IRQ
> +	help
> +	  Support for Sigma Designs Tango platform.
> +
>  endchoice

Move this to the platform Kconfig file. Also, drop all the
'select' statements that are implied by ARCH_MULTIPLATFORM
and ARCH_MULTI_V7

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 246473a244f6..0ac4ed7c267e 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1,5 +1,6 @@
>  ifeq ($(CONFIG_OF),y)
>  
> +dtb-$(CONFIG_ARCH_TANGOX) += tango.dtb
>  dtb-$(CONFIG_ARCH_ALPINE) += \
>  	alpine-db.dtb

Keep this sorted alphabetically.

> diff --git a/arch/arm/boot/dts/tango.dts b/arch/arm/boot/dts/tango.dts
> new file mode 100644
> index 000000000000..de3bf7ccc760
> --- /dev/null
> +++ b/arch/arm/boot/dts/tango.dts

name this after the actual board you use, put everything that is not
specific to the board into a tango4.dtsi file.

> +	cpublock: cpublock {
> +		compatible = "simple-bus";
> +		reg = <0x60000 0x10000>;

A "simple-bus" does not have a 'reg' property normally. If you
have to program some of the registers, give it a different
compatible string that the driver for this bus can match.

> +		ranges = <0x0 0x60000 0x10000>;
> +		interrupt-parent = <&irqintc>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		intc: intc@e000 {
> +			compatible = "sigma,tango-intc";
> +			reg = <0xe000 0x1000>;
> +			ranges = <0x0 0xe000 0x1000>;
> +			interrupt-parent = <&gic>;
> +			interrupt-controller;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			irqintc: irq@000 {
> +				reg = <0x000 0x100>;
> +				interrupt-parent = <&gic>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 2 4>;
> +				label = "IRQ";
> +			};

You can probably drop the label.

> +			fiqintc: fiq@100 {
> +				reg = <0x100 0x100>;
> +				interrupt-parent = <&gic>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 3 4>;
> +				label = "FIQ";
> +			};
> +
> +			iiqintc: iiq@300 {
> +				reg = <0x300 0x100>;
> +				interrupt-parent = <&gic>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <0 4 4>;
> +				label = "IIQ";
> +			};
> +		};
> +	};
> +
> +};


> diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> new file mode 100644
> index 000000000000..aba4eef9227c
> --- /dev/null
> +++ b/arch/arm/mach-tangox/Kconfig
> @@ -0,0 +1,48 @@
> +if ARCH_TANGOX
> +
> +config UNCOMPRESS_INCLUDE
> +	string
> +	default "debug/uncompress.h"

Better do this like everyone else.

> +config ARM_L1_CACHE_SHIFT
> +	int
> +	default 5

This conflicts with the other definition of the same symbol.

> +config MACH_TANGOX_87XX
> +	bool "Sigma Designs TANGOX 87XX Board"
> +	default y
> +	depends on ARCH_TANGOX
> +	select MIGHT_HAVE_CACHE_L2X0
> +	select CPU_V7
> +	select ARM_GIC
> +	select VFP
> +	select SMP
> +	select LOCAL_TIMERS if SMP
> +	select HAVE_ARM_TWD if SMP
> +	select HAVE_ARM_SCU if SMP
> +	select PL310_ERRATA_588369
> +	select PL310_ERRATA_727915
> +	select ARM_ERRATA_754322
> +	select ARM_ERRATA_775420
> +	select ARCH_HAS_OPP
> +	select PM_OPP if PM
> +	select USB_ARCH_HAS_EHCI if USB_SUPPORT
> +	select ARM_CPU_SUSPEND if PM
> +	select CPU_USE_DOMAINS if MMU
> +	select COMMON_CLK
> +	select TANGOX

Most of these are already implied by teh generic options.

> +void __init tangox_timer_init(void)
> +{
> +	int err;
> +
> +	clkgen_base = ioremap(CLKGEN_BASE, 0x100);

Remove all hardcoded physical memory addresses.

> +	if (clkgen_base == NULL) return;
> +
> +	register_current_timer_delay(&delay_timer);
> +	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
> +
> +	err = clocksource_register_hz(&tangox_xtal, XTAL_FREQ);
> +	if (err) pi_alert("Failed to register tangox_xtal clocksource!\n");
> +
> +	tangox_clock_tree_register();
> +
> +	of_clk_init(NULL);
> +	clocksource_of_init();
> +}

CLK_OF_DECLARE() for the clk

CLOCKSOURCE_OF_DECLARE() for the clocksource/clockevent

Make these two drivers.

> +static struct map_desc tango_map_desc[] __initdata = {
> +	{
> +		.virtual	= TANGO_IO_START,
> +		.pfn		=__phys_to_pfn(0),
> +		.length		= TANGO_IO_SIZE,
> +		.type 		= MT_DEVICE,
> +	},
> +};
> +
> +static void __init tango_map_io(void)
> +{
> +	iotable_init(tango_map_desc, ARRAY_SIZE(tango_map_desc));
> +}

What is this for? Most platforms don't need this.

> +extern void __init tangox_timer_init(void);
> +//extern struct smp_operations tangox_smp_ops;

Never put 'extern' declarations into a .c file.

> +static const char *tango_dt_compat[] = { "sigma,tango4-soc", NULL };
> +
> +MACHINE_START(TANGOX_87XX, "TANGOX_87XX")
> +	.atag_offset	= 0x100,
> +	.init_time	= tangox_timer_init,
> +	.map_io		= tango_map_io,
> +	//.smp		= &tangox_smp_ops,
> +	//.restart	= tango_restart, /*** REQUIRED FOR CLEAN REBOOT ***/
> +	.dt_compat	= tango_dt_compat,
> +MACHINE_END

All except the dt_compat can be removed here.

	Arnd
Russell King - ARM Linux Sept. 22, 2015, 2:56 p.m. UTC | #2
On Tue, Sep 22, 2015 at 04:51:48PM +0200, Arnd Bergmann wrote:
> On Tuesday 22 September 2015 16:36:48 Mason wrote:
> > +config ARCH_TANGOX
> > +	bool "Sigma Designs Tango"
> > +	select CLKDEV_LOOKUP
> > +	select HAVE_CLK
> > +	select HAVE_SMP
> > +	select ARCH_REQUIRE_GPIOLIB
> > +	select ARCH_HAS_CPUFREQ
> > +	select CLKSRC_MMIO
> > +	select GENERIC_CLOCKEVENTS
> > +	select GENERIC_IRQ_CHIP
> > +	select ARCH_HAS_HOLES_MEMORYMODEL
> > +	select SPARSE_IRQ
> > +	help
> > +	  Support for Sigma Designs Tango platform.
> > +
> >  endchoice
> 
> Move this to the platform Kconfig file. Also, drop all the
> 'select' statements that are implied by ARCH_MULTIPLATFORM
> and ARCH_MULTI_V7

And any remaining select statements should be sorted alphanumerically,
and that goes for everywhere, especially when introducing a new select
statement.

> > diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
> > new file mode 100644
> > index 000000000000..aba4eef9227c
> > --- /dev/null
> > +++ b/arch/arm/mach-tangox/Kconfig
> > @@ -0,0 +1,48 @@
> > +if ARCH_TANGOX
> > +
> > +config UNCOMPRESS_INCLUDE
> > +	string
> > +	default "debug/uncompress.h"
> 
> Better do this like everyone else.
> 
> > +config ARM_L1_CACHE_SHIFT
> > +	int
> > +	default 5
> 
> This conflicts with the other definition of the same symbol.

Indeed - I really don't like that Kconfig doesn't error out over this
kind of thing.
Mans Rullgard Sept. 22, 2015, 3:48 p.m. UTC | #3
Mason <slash.tmp@free.fr> writes:

>> - the clock support should probably go to drivers/clk, unless this is
>>   a clocksource driver, which should go to drivers/clocksource. Add a
>>   DT binding spec. If you lack DT support, don't bother sending it,
>>   but send the rest anyway.
>
> I'll take a look at Mans' DT clock code, but I want to simplify it.

I wrote the clock driver using some incomplete documentation for 865x,
additional details gleaned from ancient vendor kernel source, plus trial
and error on an 8643 chip.  If I could have done it any simpler, I would
have.  I don't have any information on the 87xx series, so it's possible
the clock tree is completely different there.
Mason Sept. 22, 2015, 3:54 p.m. UTC | #4
On 22/09/2015 16:51, Arnd Bergmann wrote:
> On Tuesday 22 September 2015 16:36:48 Mason wrote:
>> On 21/09/2015 17:49, Arnd Bergmann wrote:
>>>
>>> - As you are probably aware, please split the series into multiple patches,
>>>   doing one thing at a time. A lot of the patches don't have dependencies
>>>   on one another and can just get merged as soon as they are ready
>>
>> I'm supposed to use git-format-patch, right?
> 
> Yes, and follow Documentation/SubmittingPatches
> 
>>> [snip advice for drivers]
>>> - no mach-types changes please
>>
>> The new way is DT_MACHINE_START?
> 
> Right.
> 
>>> - whatever you need in smp_twd.c will have to get merged by Russell,
>>>   the other patches go through the arm-soc tree.
>>
>> One change is a temp wart for getting twd_clk until I have proper DT.
>>
>> The other change is to remove CLOCK_EVT_FEAT_C3STOP for my board.
>> (We discussed this in May.)
>>
>> Thanks for your guidance. Just to get the ball rolling, here's
>> the full platform patch minus drivers.
>>
>> Open question:
>>
>> 1) arch/arm/Kconfig vs arch/arm/mach-tangox/Kconfig
>> What goes in the first? What goes in the second?
> 
> The first only gets a single line to include the second.

Oh... So all the platforms that define stuff in arch/arm/Kconfig
are just legacy at this point?

>> arch/arm/Kconfig
> 
> Move this to the platform Kconfig file. Also, drop all the
> 'select' statements that are implied by ARCH_MULTIPLATFORM
> and ARCH_MULTI_V7

OK.

There is still a way to generate a kernel that supports only
one board though, right? (To minimize storage required.)

>> arch/arm/boot/dts/Makefile
> 
> Keep this sorted alphabetically.

Right, sorry for the warts, some of this stuff was just randomly thrown
together in my attempts of getting it to run. I am aware of the
sorting requirements, and will follow them, of course.

>> arch/arm/boot/dts/tango.dts
> 
> name this after the actual board you use, put everything that is not
> specific to the board into a tango4.dtsi file.

Nothing in tango.dts is specific to the board. It's all in the SoC,
as far as I can tell.

> [snip comments on DT description]
>> arch/arm/mach-tangox/Kconfig
>> +if ARCH_TANGOX
>> +
>> +config UNCOMPRESS_INCLUDE
>> +	string
>> +	default "debug/uncompress.h"
> 
> Better do this like everyone else.

Do you mean I should provide an empty mach/uncompress.h ?

>> +config ARM_L1_CACHE_SHIFT
>> +	int
>> +	default 5
> 
> This conflicts with the other definition of the same symbol.

I asked about this a long time ago. Maybe I didn't understand
Russell's answer?

http://thread.gmane.org/gmane.linux.ports.arm.kernel/402968

How do I force ARM_L1_CACHE_SHIFT to 5 for my platform?
It saves ~6% of the .data section size.
(Not worth it?)

>> +config MACH_TANGOX_87XX
>> +	bool "Sigma Designs TANGOX 87XX Board"
>> +	default y
>> +	depends on ARCH_TANGOX
>> +	select MIGHT_HAVE_CACHE_L2X0
>> +	select CPU_V7
>> +	select ARM_GIC
>> +	select VFP
>> +	select SMP
>> +	select LOCAL_TIMERS if SMP
>> +	select HAVE_ARM_TWD if SMP
>> +	select HAVE_ARM_SCU if SMP
>> +	select PL310_ERRATA_588369
>> +	select PL310_ERRATA_727915
>> +	select ARM_ERRATA_754322
>> +	select ARM_ERRATA_775420
>> +	select ARCH_HAS_OPP
>> +	select PM_OPP if PM
>> +	select USB_ARCH_HAS_EHCI if USB_SUPPORT
>> +	select ARM_CPU_SUSPEND if PM
>> +	select CPU_USE_DOMAINS if MMU
>> +	select COMMON_CLK
>> +	select TANGOX
> 
> Most of these are already implied by teh generic options.

Is there a simple way to remove what is unnecessary and not more?

>> +void __init tangox_timer_init(void)
>> +{
>> +	int err;
>> +
>> +	clkgen_base = ioremap(CLKGEN_BASE, 0x100);
> 
> Remove all hardcoded physical memory addresses.

You mean I should read CLKGEN_BASE through DT?
(Converting the clock code to DT is a TODO.)

>> +	if (clkgen_base == NULL) return;
>> +
>> +	register_current_timer_delay(&delay_timer);
>> +	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
>> +
>> +	err = clocksource_register_hz(&tangox_xtal, XTAL_FREQ);
>> +	if (err) pi_alert("Failed to register tangox_xtal clocksource!\n");
>> +
>> +	tangox_clock_tree_register();
>> +
>> +	of_clk_init(NULL);
>> +	clocksource_of_init();
>> +}
> 
> CLK_OF_DECLARE() for the clk
> 
> CLOCKSOURCE_OF_DECLARE() for the clocksource/clockevent

I'm sorry I don't understand what you mean here.
I will look at Mans' implementation.

> Make these two drivers.
> 
>> +static struct map_desc tango_map_desc[] __initdata = {
>> +	{
>> +		.virtual	= TANGO_IO_START,
>> +		.pfn		=__phys_to_pfn(0),
>> +		.length		= TANGO_IO_SIZE,
>> +		.type 		= MT_DEVICE,
>> +	},
>> +};
>> +
>> +static void __init tango_map_io(void)
>> +{
>> +	iotable_init(tango_map_desc, ARRAY_SIZE(tango_map_desc));
>> +}
> 
> What is this for? Most platforms don't need this.

IIUC, the idea was to optimize TLB entries by having a single
16MB super-section mapping. If every driver maps his little
500-byte area, the MMU code has to create lots of second
level mappings.

We might as well create one large mapping at init, no?

This logic slightly breaks down since AFAICT Linux doesn't
take advantage of super-sections.

>> +extern void __init tangox_timer_init(void);
>> +//extern struct smp_operations tangox_smp_ops;
> 
> Never put 'extern' declarations into a .c file.

Sorry for the quick and dirty code.

>> +static const char *tango_dt_compat[] = { "sigma,tango4-soc", NULL };
>> +
>> +MACHINE_START(TANGOX_87XX, "TANGOX_87XX")
>> +	.atag_offset	= 0x100,
>> +	.init_time	= tangox_timer_init,
>> +	.map_io		= tango_map_io,
>> +	//.smp		= &tangox_smp_ops,
>> +	//.restart	= tango_restart, /*** REQUIRED FOR CLEAN REBOOT ***/
>> +	.dt_compat	= tango_dt_compat,
>> +MACHINE_END
> 
> All except the dt_compat can be removed here.

I left .smp and .restart in comments because I need those, AFAIU?
Also I'm using the APPEND_DT_TO_UIMAGE option. I don't need the
atag_offset info?

Thanks for your comments, you've given me much to think about,
and much to fix.

Regards.
Russell King - ARM Linux Sept. 22, 2015, 4:29 p.m. UTC | #5
On Tue, Sep 22, 2015 at 05:54:04PM +0200, Mason wrote:
> On 22/09/2015 16:51, Arnd Bergmann wrote:
> > On Tuesday 22 September 2015 16:36:48 Mason wrote:
> >> On 21/09/2015 17:49, Arnd Bergmann wrote:
> >> +config ARM_L1_CACHE_SHIFT
> >> +	int
> >> +	default 5
> > 
> > This conflicts with the other definition of the same symbol.
> 
> I asked about this a long time ago. Maybe I didn't understand
> Russell's answer?
> 
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/402968
> 
> How do I force ARM_L1_CACHE_SHIFT to 5 for my platform?
> It saves ~6% of the .data section size.
> (Not worth it?)

It's really not worth the complexity in Kconfig to make it work - we
would need some way to detect a configuration where _only_ your
platform is enabled, and the statement for that is likely to be very
big, and very difficult to maintain into the future.

Sorry.
Arnd Bergmann Sept. 22, 2015, 7:15 p.m. UTC | #6
On Tuesday 22 September 2015 17:54:04 Mason wrote:
> On 22/09/2015 16:51, Arnd Bergmann wrote:
> > On Tuesday 22 September 2015 16:36:48 Mason wrote:
> >> On 21/09/2015 17:49, Arnd Bergmann wrote:
> >>>
> >>> - As you are probably aware, please split the series into multiple patches,
> >>>   doing one thing at a time. A lot of the patches don't have dependencies
> >>>   on one another and can just get merged as soon as they are ready
> >>
> >> I'm supposed to use git-format-patch, right?
> > 
> > Yes, and follow Documentation/SubmittingPatches
> > 
> >>> [snip advice for drivers]
> >>> - no mach-types changes please
> >>
> >> The new way is DT_MACHINE_START?
> > 
> > Right.
> > 
> >>> - whatever you need in smp_twd.c will have to get merged by Russell,
> >>>   the other patches go through the arm-soc tree.
> >>
> >> One change is a temp wart for getting twd_clk until I have proper DT.
> >>
> >> The other change is to remove CLOCK_EVT_FEAT_C3STOP for my board.
> >> (We discussed this in May.)
> >>
> >> Thanks for your guidance. Just to get the ball rolling, here's
> >> the full platform patch minus drivers.
> >>
> >> Open question:
> >>
> >> 1) arch/arm/Kconfig vs arch/arm/mach-tangox/Kconfig
> >> What goes in the first? What goes in the second?
> > 
> > The first only gets a single line to include the second.
> 
> Oh... So all the platforms that define stuff in arch/arm/Kconfig
> are just legacy at this point?

To a certain degree yes. I have patches for all ARMv6/ARMv7 platforms
that are not converted yet, and we are not accepting new platforms
outside of ARCH_MULTIPLATFORM.

> > 
> > name this after the actual board you use, put everything that is not
> > specific to the board into a tango4.dtsi file.
> 
> Nothing in tango.dts is specific to the board. It's all in the SoC,
> as far as I can tell.

You can probably just rename the file then, and have a board specific
dts file with just the /chosen and /aliases nodes.

> > [snip comments on DT description]
> >> arch/arm/mach-tangox/Kconfig
> >> +if ARCH_TANGOX
> >> +
> >> +config UNCOMPRESS_INCLUDE
> >> +	string
> >> +	default "debug/uncompress.h"
> > 
> > Better do this like everyone else.
> 
> Do you mean I should provide an empty mach/uncompress.h ?

No, add a debug/*.S file for your uart and do the configuration in
arch/arm/Kconfig.debug.

The UNCOMPRESS_INCLUDE symbol is defined in arch/arm/Kconfig.debug,
and you should not duplicate that.

> >> +config MACH_TANGOX_87XX
> >> +	bool "Sigma Designs TANGOX 87XX Board"
> >> +	default y
> >> +	depends on ARCH_TANGOX
> >> +	select MIGHT_HAVE_CACHE_L2X0
> >> +	select CPU_V7
> >> +	select ARM_GIC
> >> +	select VFP
> >> +	select SMP
> >> +	select LOCAL_TIMERS if SMP
> >> +	select HAVE_ARM_TWD if SMP
> >> +	select HAVE_ARM_SCU if SMP
> >> +	select PL310_ERRATA_588369
> >> +	select PL310_ERRATA_727915
> >> +	select ARM_ERRATA_754322
> >> +	select ARM_ERRATA_775420
> >> +	select ARCH_HAS_OPP
> >> +	select PM_OPP if PM
> >> +	select USB_ARCH_HAS_EHCI if USB_SUPPORT
> >> +	select ARM_CPU_SUSPEND if PM
> >> +	select CPU_USE_DOMAINS if MMU
> >> +	select COMMON_CLK
> >> +	select TANGOX
> > 
> > Most of these are already implied by teh generic options.
> 
> Is there a simple way to remove what is unnecessary and not more?

Look at ARCH_MULTIPLATFORM and ARCH_MULTI_V7 as a start.

> >> +void __init tangox_timer_init(void)
> >> +{
> >> +	int err;
> >> +
> >> +	clkgen_base = ioremap(CLKGEN_BASE, 0x100);
> > 
> > Remove all hardcoded physical memory addresses.
> 
> You mean I should read CLKGEN_BASE through DT?
> (Converting the clock code to DT is a TODO.)

Yes, until that is done, we are not merging this file.

> > 
> >> +static struct map_desc tango_map_desc[] __initdata = {
> >> +	{
> >> +		.virtual	= TANGO_IO_START,
> >> +		.pfn		=__phys_to_pfn(0),
> >> +		.length		= TANGO_IO_SIZE,
> >> +		.type 		= MT_DEVICE,
> >> +	},
> >> +};
> >> +
> >> +static void __init tango_map_io(void)
> >> +{
> >> +	iotable_init(tango_map_desc, ARRAY_SIZE(tango_map_desc));
> >> +}
> > 
> > What is this for? Most platforms don't need this.
> 
> IIUC, the idea was to optimize TLB entries by having a single
> 16MB super-section mapping. If every driver maps his little
> 500-byte area, the MMU code has to create lots of second
> level mappings.
> 
> We might as well create one large mapping at init, no?

Right, but if you do that, please add a comment for it, ideally
showing how much you gained from doing this in terms of vmalloc
space or real-life performance.

It's not wrong to have a mapping like this, just annoying enough
that we don't want to do it without a good reason.

> >> +static const char *tango_dt_compat[] = { "sigma,tango4-soc", NULL };
> >> +
> >> +MACHINE_START(TANGOX_87XX, "TANGOX_87XX")
> >> +	.atag_offset	= 0x100,
> >> +	.init_time	= tangox_timer_init,
> >> +	.map_io		= tango_map_io,
> >> +	//.smp		= &tangox_smp_ops,
> >> +	//.restart	= tango_restart, /*** REQUIRED FOR CLEAN REBOOT ***/
> >> +	.dt_compat	= tango_dt_compat,
> >> +MACHINE_END
> > 
> > All except the dt_compat can be removed here.
> 
> I left .smp and .restart in comments because I need those, AFAIU?

There are other ways to do those too, see CPU_METHOD_OF_DECLARE()
and register_restart_handler().

> Also I'm using the APPEND_DT_TO_UIMAGE option. I don't need the
> atag_offset info?

No, the atag_offset is only use for native booting without a DT
(old-style board files), which you don't do.

	Arnd
Mason Sept. 23, 2015, 8:49 a.m. UTC | #7
On 22/09/2015 18:29, Russell King - ARM Linux wrote:
> On Tue, Sep 22, 2015 at 05:54:04PM +0200, Mason wrote:
>> On 22/09/2015 16:51, Arnd Bergmann wrote:
>>> On Tuesday 22 September 2015 16:36:48 Mason wrote:
>>>> On 21/09/2015 17:49, Arnd Bergmann wrote:
>>>> +config ARM_L1_CACHE_SHIFT
>>>> +	int
>>>> +	default 5
>>>
>>> This conflicts with the other definition of the same symbol.
>>
>> I asked about this a long time ago. Maybe I didn't understand
>> Russell's answer?
>>
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/402968
>>
>> How do I force ARM_L1_CACHE_SHIFT to 5 for my platform?
>> It saves ~6% of the .data section size.
>> (Not worth it?)
> 
> It's really not worth the complexity in Kconfig to make it work - we
> would need some way to detect a configuration where _only_ your
> platform is enabled, and the statement for that is likely to be very
> big, and very difficult to maintain into the future.

Doh! I keep missing the ramifications of the ARCH_MULTIPLATFORM setting.

How about overriding ARM_L1_CACHE_SHIFT *ONLY* when not building an
ARCH_MULTIPLATFORM kernel?

In my platform Kconfig, something along the lines of

config ARM_L1_CACHE_SHIFT
	int
	default 5 if !ARCH_MULTIPLATFORM

Regards.
Russell King - ARM Linux Sept. 23, 2015, 9:13 a.m. UTC | #8
On Wed, Sep 23, 2015 at 10:49:25AM +0200, Mason wrote:
> In my platform Kconfig, something along the lines of
> 
> config ARM_L1_CACHE_SHIFT
> 	int
> 	default 5 if !ARCH_MULTIPLATFORM

How would that be useful to you?  Arnd is saying that you will always
need ARCH_MULTIPLATFORM selected to build your platform.

This symbol doesn't automatically turn off just because you only have
one platform enabled.
Mason Sept. 23, 2015, 9:21 a.m. UTC | #9
On 23/09/2015 11:13, Russell King - ARM Linux wrote:
> On Wed, Sep 23, 2015 at 10:49:25AM +0200, Mason wrote:
>> In my platform Kconfig, something along the lines of
>>
>> config ARM_L1_CACHE_SHIFT
>> 	int
>> 	default 5 if !ARCH_MULTIPLATFORM
> 
> How would that be useful to you?  Arnd is saying that you will always
> need ARCH_MULTIPLATFORM selected to build your platform.
> 
> This symbol doesn't automatically turn off just because you only have
> one platform enabled.

I must be missing the big picture for ARCH_MULTIPLATFORM.

I thought modern ports /had/ to support it, but that it could
be disabled locally to build a single platform kernel?

Regards.
Mason Sept. 23, 2015, 9:26 a.m. UTC | #10
On 22/09/2015 21:15, Arnd Bergmann wrote:
> On Tuesday 22 September 2015 17:54:04 Mason wrote:
>> On 22/09/2015 16:51, Arnd Bergmann wrote:
>>> On Tuesday 22 September 2015 16:36:48 Mason wrote:
>>>
>>>> arch/arm/mach-tangox/Kconfig
>>>> +if ARCH_TANGOX
>>>> +
>>>> +config UNCOMPRESS_INCLUDE
>>>> +	string
>>>> +	default "debug/uncompress.h"
>>>
>>> Better do this like everyone else.
>>
>> Do you mean I should provide an empty mach/uncompress.h ?
> 
> No, add a debug/*.S file for your uart and do the configuration in
> arch/arm/Kconfig.debug.
> 
> The UNCOMPRESS_INCLUDE symbol is defined in arch/arm/Kconfig.debug,
> and you should not duplicate that.

config UNCOMPRESS_INCLUDE
	string
	default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \
					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
					ARCH_SHMOBILE_LEGACY
	default "mach/uncompress.h"


I'm going for the smallest possible port, I don't have a debug/tango.S
I didn't define DEBUG_UNCOMPRESS in my .config, and I just wanted to
get this NOP definition from debug/uncompress.h

	static inline void putc(int c) {}

Are you saying I cannot do it like that?

If ARCH_MULTIPLATFORM is defined, then debug/uncompress.h will be
included, just like I want.

If ARCH_MULTIPLATFORM is not defined, can I do this in my port:

config UNCOMPRESS_INCLUDE
	string
	default "debug/uncompress.h" if !ARCH_MULTIPLATFORM

or add my platform to the list on arch/arm/Kconfig.debug?

Regards.
Russell King - ARM Linux Sept. 23, 2015, 9:26 a.m. UTC | #11
On Wed, Sep 23, 2015 at 11:21:15AM +0200, Mason wrote:
> On 23/09/2015 11:13, Russell King - ARM Linux wrote:
> > On Wed, Sep 23, 2015 at 10:49:25AM +0200, Mason wrote:
> >> In my platform Kconfig, something along the lines of
> >>
> >> config ARM_L1_CACHE_SHIFT
> >> 	int
> >> 	default 5 if !ARCH_MULTIPLATFORM
> > 
> > How would that be useful to you?  Arnd is saying that you will always
> > need ARCH_MULTIPLATFORM selected to build your platform.
> > 
> > This symbol doesn't automatically turn off just because you only have
> > one platform enabled.
> 
> I must be missing the big picture for ARCH_MULTIPLATFORM.
> 
> I thought modern ports /had/ to support it, but that it could
> be disabled locally to build a single platform kernel?

ARCH_MULTIPLATFORM is purely a Kconfig construct to allow separation of
platforms which support being built together, and those which don't.
It provides a certain basic level of support when enabled (such as DT,
CCF, etc.)  It remains enabled even if you only want to build for one
platform in the multi-platform group.
Arnd Bergmann Sept. 23, 2015, 9:34 a.m. UTC | #12
On Wednesday 23 September 2015 11:26:33 Mason wrote:
> On 22/09/2015 21:15, Arnd Bergmann wrote:
> > On Tuesday 22 September 2015 17:54:04 Mason wrote:
> >> On 22/09/2015 16:51, Arnd Bergmann wrote:
> >>> On Tuesday 22 September 2015 16:36:48 Mason wrote:
> >>>
> >>>> arch/arm/mach-tangox/Kconfig
> >>>> +if ARCH_TANGOX
> >>>> +
> >>>> +config UNCOMPRESS_INCLUDE
> >>>> +	string
> >>>> +	default "debug/uncompress.h"
> >>>
> >>> Better do this like everyone else.
> >>
> >> Do you mean I should provide an empty mach/uncompress.h ?
> > 
> > No, add a debug/*.S file for your uart and do the configuration in
> > arch/arm/Kconfig.debug.
> > 
> > The UNCOMPRESS_INCLUDE symbol is defined in arch/arm/Kconfig.debug,
> > and you should not duplicate that.
> 
> config UNCOMPRESS_INCLUDE
> 	string
> 	default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \
> 					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
> 					ARCH_SHMOBILE_LEGACY
> 	default "mach/uncompress.h"
> 
> 
> I'm going for the smallest possible port, I don't have a debug/tango.S
> I didn't define DEBUG_UNCOMPRESS in my .config, and I just wanted to
> get this NOP definition from debug/uncompress.h
> 
> 	static inline void putc(int c) {}
> 
> Are you saying I cannot do it like that?
> 
> If ARCH_MULTIPLATFORM is defined, then debug/uncompress.h will be
> included, just like I want.
> 
> If ARCH_MULTIPLATFORM is not defined, can I do this in my port:
> 
> config UNCOMPRESS_INCLUDE
> 	string
> 	default "debug/uncompress.h" if !ARCH_MULTIPLATFORM
> 
> or add my platform to the list on arch/arm/Kconfig.debug?

Just use ARCH_MULTIPLATFORM.

	Arnd
Mason Sept. 25, 2015, 1:06 p.m. UTC | #13
On 22/09/2015 16:51, Arnd Bergmann wrote:

> Move this to the platform Kconfig file. Also, drop all the
> 'select' statements that are implied by ARCH_MULTIPLATFORM
> and ARCH_MULTI_V7

I've trimmed my platform Kconfig down to:

config ARCH_TANGO4
	bool "Sigma Designs Tango4 (SMP87xx)"
	default y
	select ARCH_HAS_HOLES_MEMORYMODEL
	select ARM_ERRATA_754322
	select ARM_ERRATA_764369
	select ARM_GIC
	select CACHE_L2X0
	select CLKSRC_MMIO
	select GENERIC_IRQ_CHIP
	select HAVE_ARM_SCU
	select HAVE_ARM_TWD
	select NEON
	select SMP


Didn't find much documentation on ARCH_HAS_HOLES_MEMORYMODEL.
What is it used for?

http://cateee.net/lkddb/web-lkddb/ARCH_HAS_HOLES_MEMORYMODEL.html

Is it OK to select CACHE_L2X0, NEON, and SMP?

Regards.
Arnd Bergmann Sept. 25, 2015, 1:17 p.m. UTC | #14
On Friday 25 September 2015 15:06:32 Mason wrote:
> On 22/09/2015 16:51, Arnd Bergmann wrote:
> 
> > Move this to the platform Kconfig file. Also, drop all the
> > 'select' statements that are implied by ARCH_MULTIPLATFORM
> > and ARCH_MULTI_V7
> 
> I've trimmed my platform Kconfig down to:
> 
> config ARCH_TANGO4
>         bool "Sigma Designs Tango4 (SMP87xx)"
>         default y

drop the default

add
	depends on CPU_MULTI_V7

>         select ARCH_HAS_HOLES_MEMORYMODEL
>         select ARM_ERRATA_754322
>         select ARM_ERRATA_764369

	add 'if SMP'

>         select ARM_GIC
>         select CACHE_L2X0

remove CACHE_L2X0, we want to be able to turn this off.

>         select CLKSRC_MMIO
>         select GENERIC_IRQ_CHIP
>         select HAVE_ARM_SCU
>         select HAVE_ARM_TWD

	'if SMP'

>         select NEON
>         select SMP

These should be user-selectable as well, so drop the 'select'
and make sure the kernel builds with them turned off.
 
> 
> Didn't find much documentation on ARCH_HAS_HOLES_MEMORYMODEL.
> What is it used for?

You need this if the RAM is not physically contiguous, e.g. 256MB at one
address and another 256MB somewhere else.

> Is it OK to select CACHE_L2X0, NEON, and SMP?

no

	Arnd
Mason Sept. 25, 2015, 1:35 p.m. UTC | #15
On 25/09/2015 15:17, Arnd Bergmann wrote:
> On Friday 25 September 2015 15:06:32 Mason wrote:
>> On 22/09/2015 16:51, Arnd Bergmann wrote:
>>
>>> Move this to the platform Kconfig file. Also, drop all the
>>> 'select' statements that are implied by ARCH_MULTIPLATFORM
>>> and ARCH_MULTI_V7
>>
>> I've trimmed my platform Kconfig down to:
>>
>> config ARCH_TANGO4
>>         bool "Sigma Designs Tango4 (SMP87xx)"
>>         default y
> 
> drop the default
> 
> add
> 	depends on CPU_MULTI_V7

I've done this higher up.

menuconfig ARCH_TANGOX
	bool "Sigma Designs Tango" if ARCH_MULTI_V7
	help
	  something useful

if ARCH_TANGOX
config ARCH_TANGO4
...
endif

So 'default y' enables ARCH_TANGO4 only if ARCH_TANGOX is selected,
right?

>>         select ARCH_HAS_HOLES_MEMORYMODEL
>>         select ARM_ERRATA_754322
>>         select ARM_ERRATA_764369
> 
> 	add 'if SMP'
> 
>>         select ARM_GIC
>>         select CACHE_L2X0
> 
> remove CACHE_L2X0, we want to be able to turn this off.

Some people run with L2 disabled? That's a strange thing to do.

>>         select CLKSRC_MMIO
>>         select GENERIC_IRQ_CHIP
>>         select HAVE_ARM_SCU
>>         select HAVE_ARM_TWD
> 
> 	'if SMP'
> 
>>         select NEON
>>         select SMP
> 
> These should be user-selectable as well, so drop the 'select'
> and make sure the kernel builds with them turned off.

It will build, but it won't run! My port uses the TWD for clockevents,
and smp_twd.c is only compiled if HAVE_ARM_TWD. So I must set SMP.

>> Didn't find much documentation on ARCH_HAS_HOLES_MEMORYMODEL.
>> What is it used for?
> 
> You need this if the RAM is not physically contiguous, e.g. 256MB at one
> address and another 256MB somewhere else.
> 
>> Is it OK to select CACHE_L2X0, NEON, and SMP?
> 
> no

Then I have a major problem... my port requires SMP.

Regards.
Arnd Bergmann Sept. 25, 2015, 2:11 p.m. UTC | #16
On Friday 25 September 2015 15:35:36 Mason wrote:
> On 25/09/2015 15:17, Arnd Bergmann wrote:
> > On Friday 25 September 2015 15:06:32 Mason wrote:
> >> On 22/09/2015 16:51, Arnd Bergmann wrote:
> >>
> >>> Move this to the platform Kconfig file. Also, drop all the
> >>> 'select' statements that are implied by ARCH_MULTIPLATFORM
> >>> and ARCH_MULTI_V7
> >>
> >> I've trimmed my platform Kconfig down to:
> >>
> >> config ARCH_TANGO4
> >>         bool "Sigma Designs Tango4 (SMP87xx)"
> >>         default y
> > 
> > drop the default
> > 
> > add
> > 	depends on CPU_MULTI_V7
> 
> I've done this higher up.
> 
> menuconfig ARCH_TANGOX
> 	bool "Sigma Designs Tango" if ARCH_MULTI_V7
> 	help
> 	  something useful
> 
> if ARCH_TANGOX
> config ARCH_TANGO4
> ...
> endif
> 
> So 'default y' enables ARCH_TANGO4 only if ARCH_TANGOX is selected,
> right?

You should really only have one of the two.

> >>         select ARCH_HAS_HOLES_MEMORYMODEL
> >>         select ARM_ERRATA_754322
> >>         select ARM_ERRATA_764369
> > 
> > 	add 'if SMP'
> > 
> >>         select ARM_GIC
> >>         select CACHE_L2X0
> > 
> > remove CACHE_L2X0, we want to be able to turn this off.
> 
> Some people run with L2 disabled? That's a strange thing to do.

I can help for testing. Most importantly, we currently allow that
configuration today, and it's not up to you to forbid other platforms
from doing it.

If you can prove that we never need to disable that option, send
another patch to make it unconditional.

> >>         select CLKSRC_MMIO
> >>         select GENERIC_IRQ_CHIP
> >>         select HAVE_ARM_SCU
> >>         select HAVE_ARM_TWD
> > 
> > 	'if SMP'
> > 
> >>         select NEON
> >>         select SMP
> > 
> > These should be user-selectable as well, so drop the 'select'
> > and make sure the kernel builds with them turned off.
> 
> It will build, but it won't run! My port uses the TWD for clockevents,
> and smp_twd.c is only compiled if HAVE_ARM_TWD. So I must set SMP.

I think this has come up before and should be fixed. Could you
send a patch that allows using TWD in uniprocessor configurations?

> >> Didn't find much documentation on ARCH_HAS_HOLES_MEMORYMODEL.
> >> What is it used for?
> > 
> > You need this if the RAM is not physically contiguous, e.g. 256MB at one
> > address and another 256MB somewhere else.
> > 
> >> Is it OK to select CACHE_L2X0, NEON, and SMP?
> > 
> > no
> 
> Then I have a major problem... my port requires SMP.

That would be very unusual.

	Arnd
Mason Sept. 25, 2015, 3:28 p.m. UTC | #17
On 25/09/2015 16:11, Arnd Bergmann wrote:
> On Friday 25 September 2015 15:35:36 Mason wrote:
>> On 25/09/2015 15:17, Arnd Bergmann wrote:
>>> On Friday 25 September 2015 15:06:32 Mason wrote:
>>>
>>>> I've trimmed my platform Kconfig down to:
>>>>
>>>> config ARCH_TANGO4
>>>>         bool "Sigma Designs Tango4 (SMP87xx)"
>>>>         default y
>>>
>>> drop the default
>>>
>>> add
>>> 	depends on CPU_MULTI_V7
>>
>> I've done this higher up.
>>
>> menuconfig ARCH_TANGOX
>> 	bool "Sigma Designs Tango" if ARCH_MULTI_V7
>> 	help
>> 	  something useful
>>
>> if ARCH_TANGOX
>> config ARCH_TANGO4
>> ...
>> endif
>>
>> So 'default y' enables ARCH_TANGO4 only if ARCH_TANGOX is selected,
>> right?
> 
> You should really only have one of the two.

I plan to add ARCH_TANGO5 in a few weeks. So I copied the layout used
by Samsung for EXYNOSx and TI for OMAPx.

>>>>         select ARCH_HAS_HOLES_MEMORYMODEL
>>>>         select ARM_ERRATA_754322
>>>>         select ARM_ERRATA_764369
>>>
>>> 	add 'if SMP'
>>>
>>>>         select ARM_GIC
>>>>         select CACHE_L2X0
>>>
>>> remove CACHE_L2X0, we want to be able to turn this off.
>>
>> Some people run with L2 disabled? That's a strange thing to do.
> 
> I can help for testing. Most importantly, we currently allow that
> configuration today, and it's not up to you to forbid other platforms
> from doing it.

As far as I can tell, I'm running with L2 disabled anyway, since
Linux is running in non-secure mode, and I didn't provide the ABI
to talk to the secure OS.

> If you can prove that we never need to disable that option, send
> another patch to make it unconditional.

CACHE_L2X0 defaults to y. That's good enough for me.

>>>>         select CLKSRC_MMIO
>>>>         select GENERIC_IRQ_CHIP
>>>>         select HAVE_ARM_SCU
>>>>         select HAVE_ARM_TWD
>>>
>>> 	'if SMP'
>>>
>>>>         select NEON
>>>>         select SMP
>>>
>>> These should be user-selectable as well, so drop the 'select'
>>> and make sure the kernel builds with them turned off.

About NEON. I had 'select VFP' but I see that CPU_V7 enables VFPv3 and VFP.
Isn't NEON required to support user-space programs using NEON instructions?
arch/arm/kernel/entry-armv.S

>> It will build, but it won't run! My port uses the TWD for clockevents,
>> and smp_twd.c is only compiled if HAVE_ARM_TWD. So I must set SMP.
> 
> I think this has come up before and should be fixed. Could you
> send a patch that allows using TWD in uniprocessor configurations?

This was discussed here:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/389931/focus=15737

I can re-post the same patch.

Regards.
Mason Sept. 25, 2015, 3:33 p.m. UTC | #18
On 25/09/2015 17:28, Mason wrote:

> About NEON. I had 'select VFP' but I see that CPU_V7 enables VFPv3 and VFP.
> Isn't NEON required to support user-space programs using NEON instructions?
> arch/arm/kernel/entry-armv.S

I'm taking cues from exynos and omap2, are these good references?

config ARCH_OMAP2PLUS_TYPICAL (default y) selects NEON if CPU_V7.
Can I do something similar? (My platform is only CPU_V7.)

Regards.
Arnd Bergmann Sept. 25, 2015, 3:49 p.m. UTC | #19
On Friday 25 September 2015 17:33:13 Mason wrote:
> On 25/09/2015 17:28, Mason wrote:
> 
> > About NEON. I had 'select VFP' but I see that CPU_V7 enables VFPv3 and VFP.
> > Isn't NEON required to support user-space programs using NEON instructions?
> > arch/arm/kernel/entry-armv.S
> 
> I'm taking cues from exynos and omap2, are these good references?
> 
> config ARCH_OMAP2PLUS_TYPICAL (default y) selects NEON if CPU_V7.
> Can I do something similar? (My platform is only CPU_V7.)

No, you should leave it turned off and just enable that option in the
defconfig. There is no need to prevent anyone from running a kernel
with NEON disabled if they have matching user space.

	Arnd
Arnd Bergmann Sept. 25, 2015, 3:52 p.m. UTC | #20
On Friday 25 September 2015 17:28:54 Mason wrote:
> On 25/09/2015 16:11, Arnd Bergmann wrote:
> > On Friday 25 September 2015 15:35:36 Mason wrote:
> >>
> >> So 'default y' enables ARCH_TANGO4 only if ARCH_TANGOX is selected,
> >> right?
> > 
> > You should really only have one of the two.
> 
> I plan to add ARCH_TANGO5 in a few weeks. So I copied the layout used
> by Samsung for EXYNOSx and TI for OMAPx.
 
Better start simple, I wouldn't expect there to be too much code
that is specific to just one of them to warrant a top-level Kconfig
option like on OMAP.


> >> It will build, but it won't run! My port uses the TWD for clockevents,
> >> and smp_twd.c is only compiled if HAVE_ARM_TWD. So I must set SMP.
> > 
> > I think this has come up before and should be fixed. Could you
> > send a patch that allows using TWD in uniprocessor configurations?
> 
> This was discussed here:
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/389931/focus=15737
> 
> I can re-post the same patch.

Yes, please include that patch in the series.

	Arnd
Mason Sept. 28, 2015, 1:48 p.m. UTC | #21
On 22/09/2015 16:51, Arnd Bergmann wrote:

> On Tuesday 22 September 2015 16:36:48 Mason wrote:
> 
>> +void __init tangox_timer_init(void)
>> +{
>> +	int err;
>> +
>> +	clkgen_base = ioremap(CLKGEN_BASE, 0x100);
> 
> Remove all hardcoded physical memory addresses.
> 
>> +	if (clkgen_base == NULL) return;
>> +
>> +	register_current_timer_delay(&delay_timer);
>> +	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
>> +
>> +	err = clocksource_register_hz(&tangox_xtal, XTAL_FREQ);
>> +	if (err) pi_alert("Failed to register tangox_xtal clocksource!\n");
>> +
>> +	tangox_clock_tree_register();
>> +
>> +	of_clk_init(NULL);
>> +	clocksource_of_init();
>> +}
> 
> CLK_OF_DECLARE() for the clk
> 
> CLOCKSOURCE_OF_DECLARE() for the clocksource/clockevent
> 
> Make these two drivers.

Hmmm, about that (splitting clock-tango.c in two drivers)

The "legacy" order of calls was:

  register_current_timer_delay
  sched_clock_register
  clocksource_register_hz
  /* Should the above 3 be called in a different order? */

then

  tangox_clock_tree_register

then

  of_clk_init
  clocksource_of_init
  /* To invoke the smp_twd OF init, after the clock tree is registered */


I moved the first three to a separate clocksource driver:

static void __init tango_timer_init(struct device_node *np)
{
	clkgen_base = of_iomap(np, 0);
	register_current_timer_delay(&delay_timer);
	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
	clocksource_register_hz(&tango_xtal, XTAL_FREQ);
}
CLOCKSOURCE_OF_DECLARE(tango, "sigma,tango-xtal", tango_timer_init);

But tango_timer_init() is not being called...

I updated my device tree with

	tango-xtal@10000 {
		compatible = "sigma,tango-xtal";
		reg = <0x10000 0x100>;
	};

Shouldn't the kernel call tango_timer_init?

Regards.
Mans Rullgard Sept. 28, 2015, 2:43 p.m. UTC | #22
Mason <slash.tmp@free.fr> writes:

> On 22/09/2015 16:51, Arnd Bergmann wrote:
>
>> On Tuesday 22 September 2015 16:36:48 Mason wrote:
>> 
>>> +void __init tangox_timer_init(void)
>>> +{
>>> +	int err;
>>> +
>>> +	clkgen_base = ioremap(CLKGEN_BASE, 0x100);
>> 
>> Remove all hardcoded physical memory addresses.
>> 
>>> +	if (clkgen_base == NULL) return;
>>> +
>>> +	register_current_timer_delay(&delay_timer);
>>> +	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
>>> +
>>> +	err = clocksource_register_hz(&tangox_xtal, XTAL_FREQ);
>>> +	if (err) pi_alert("Failed to register tangox_xtal clocksource!\n");
>>> +
>>> +	tangox_clock_tree_register();
>>> +
>>> +	of_clk_init(NULL);
>>> +	clocksource_of_init();
>>> +}
>> 
>> CLK_OF_DECLARE() for the clk
>> 
>> CLOCKSOURCE_OF_DECLARE() for the clocksource/clockevent
>> 
>> Make these two drivers.
>
> Hmmm, about that (splitting clock-tango.c in two drivers)
>
> The "legacy" order of calls was:
>
>   register_current_timer_delay
>   sched_clock_register
>   clocksource_register_hz
>   /* Should the above 3 be called in a different order? */
>
> then
>
>   tangox_clock_tree_register
>
> then
>
>   of_clk_init
>   clocksource_of_init
>   /* To invoke the smp_twd OF init, after the clock tree is registered */
>
> I moved the first three to a separate clocksource driver:
>
> static void __init tango_timer_init(struct device_node *np)
> {
> 	clkgen_base = of_iomap(np, 0);
> 	register_current_timer_delay(&delay_timer);
> 	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
> 	clocksource_register_hz(&tango_xtal, XTAL_FREQ);
> }
> CLOCKSOURCE_OF_DECLARE(tango, "sigma,tango-xtal", tango_timer_init);
>
> But tango_timer_init() is not being called...
>
> I updated my device tree with
>
> 	tango-xtal@10000 {
> 		compatible = "sigma,tango-xtal";
> 		reg = <0x10000 0x100>;
> 	};
>
> Shouldn't the kernel call tango_timer_init?

Why don't you use the drivers from my tree?  It has drivers for
everything clock related in tango3 and shouldn't need much tweaking to
work with tango4 as well.
Mason Sept. 28, 2015, 4:32 p.m. UTC | #23
On 28/09/2015 15:48, Mason wrote:

> But tango_timer_init() is not being called...

Doh! I was using the wrong DTB...

By the way, the command I use to generate uImage feels wrong.
Is there a better way?

$ make dtbs ; make -j2 zImage ; cat arch/arm/boot/zImage arch/arm/boot/dts/tango4.dtb >XXX && mv XXX arch/arm/boot/zImage ; make uImage LOADADDR=0x80008000


My clocksource driver is fairly trivial:

#include <linux/of_address.h>	/* of_iomap			*/
#include <linux/sched_clock.h>	/* sched_clock_register		*/
#include <linux/clocksource.h>	/* clocksource_register_hz	*/
#include <linux/delay.h>	/* register_current_timer_delay	*/

#define XTAL_FREQ		27000000 /* Hz */

static void __iomem *xtal_in_cnt;

static unsigned long read_xtal_counter(void)
{
	return readl_relaxed(xtal_in_cnt);
}

static u64 read_sched_clock(void)
{
	return read_xtal_counter();
}

static cycle_t read_clocksource(struct clocksource *cs)
{
	return read_xtal_counter();
}

static struct clocksource tango_xtal = {
	.name	= "tango_xtal",
	.rating	= 300,
	.read	= read_clocksource,
	.mask	= CLOCKSOURCE_MASK(32),
	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
};

static struct delay_timer delay_timer = { read_xtal_counter, XTAL_FREQ };

static void __init tango_timer_init(struct device_node *np)
{
	xtal_in_cnt = of_iomap(np, 0);
	register_current_timer_delay(&delay_timer);
	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
	clocksource_register_hz(&tango_xtal, XTAL_FREQ);
}

CLOCKSOURCE_OF_DECLARE(tango, "sigma,tango-xtal", tango_timer_init);


Regards.
Russell King - ARM Linux Sept. 28, 2015, 5:29 p.m. UTC | #24
On Mon, Sep 28, 2015 at 06:32:05PM +0200, Mason wrote:
> On 28/09/2015 15:48, Mason wrote:
> 
> > But tango_timer_init() is not being called...
> 
> Doh! I was using the wrong DTB...
> 
> By the way, the command I use to generate uImage feels wrong.
> Is there a better way?
> 
> $ make dtbs ; make -j2 zImage ; cat arch/arm/boot/zImage arch/arm/boot/dts/tango4.dtb >XXX && mv XXX arch/arm/boot/zImage ; make uImage LOADADDR=0x80008000

bash$ cat > build.sh
#!/bin/sh
function emake
{
  LANG=C make -j2 CROSS_COMPILE=arm-linux- ARCH=arm "$@"
}

load=0x80008000

emake zImage dtbs
v=$(emake -s kernelrelease)
cat arch/arm/boot/zImage arch/arm/boot/dts/tango4.dtb zImage.tmp
mkimage -A arm -O linux -C none -T kernel \
	-a $load -e $load -n "Linux $v (dt)" -d zImage.tmp uImage
^d
bash$ chmod 755 build.sh
bash$ ./build.sh #<--- build kernel to uImage like this

Even better (and this is what I do) is you can append to that script
a command to copy it to your TFTP server, install modules, tar them
up, copy the modules to the target, and have the target automatically
reboot after the kernel build has finished (provided certain options
are given to the script.)

For a fair number of my active development boards, my kernel build and
boot process is just:

$ kbuild -r -m imx6 cbi4 ssh:hbi2ex hbi1

to build and boot an imx6 kernel on three imx6 machines, or just:

$ kbuild armada38x

to build and copy the armada38x kernel to the tftp server.

This is why people who think that copying kernels to SD cards is somehow
easier than TFTP boot - TFTP based booting is fully automated, no need
to constantly plug and unplug SD cards, wearing out their sockets.  You
can't get any simpler than running a script, walking away to make a
coffee, coming back and having your newly built kernel running on a
whole suite of platforms without any user intervention - complete with
updated modules on the target systems. ;)
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1c5021002fe4..06002e552afd 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -758,6 +758,21 @@  config ARCH_OMAP1
 	help
 	  Support for older TI OMAP1 (omap7xx, omap15xx or omap16xx)
 
+config ARCH_TANGOX
+	bool "Sigma Designs Tango"
+	select CLKDEV_LOOKUP
+	select HAVE_CLK
+	select HAVE_SMP
+	select ARCH_REQUIRE_GPIOLIB
+	select ARCH_HAS_CPUFREQ
+	select CLKSRC_MMIO
+	select GENERIC_CLOCKEVENTS
+	select GENERIC_IRQ_CHIP
+	select ARCH_HAS_HOLES_MEMORYMODEL
+	select SPARSE_IRQ
+	help
+	  Support for Sigma Designs Tango platform.
+
 endchoice
 
 menu "Multiple platform selection"
@@ -896,6 +911,8 @@  source "arch/arm/mach-omap1/Kconfig"
 
 source "arch/arm/mach-omap2/Kconfig"
 
+source "arch/arm/mach-tangox/Kconfig"
+
 source "arch/arm/mach-orion5x/Kconfig"
 
 source "arch/arm/mach-picoxcell/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 7451b447cc2d..54a4453857ad 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..0ac4ed7c267e 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1,5 +1,6 @@ 
 ifeq ($(CONFIG_OF),y)
 
+dtb-$(CONFIG_ARCH_TANGOX) += tango.dtb
 dtb-$(CONFIG_ARCH_ALPINE) += \
 	alpine-db.dtb
 dtb-$(CONFIG_MACH_ASM9260) += \
diff --git a/arch/arm/boot/dts/tango.dts b/arch/arm/boot/dts/tango.dts
new file mode 100644
index 000000000000..de3bf7ccc760
--- /dev/null
+++ b/arch/arm/boot/dts/tango.dts
@@ -0,0 +1,96 @@ 
+/dts-v1/;
+
+/ {
+	compatible = "sigma,tango4-soc";
+
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	gic: interrupt-controller@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>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		interrupt-parent = <&irqintc>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		uart0: uart@10700 {
+			compatible = "ns16550a";
+			reg = <0x10700 0x100>;
+			clock-frequency = <7372800>;
+			reg-shift = <2>;
+			no-loopback-test;
+		};
+
+		eth0: emac@26000 {
+			compatible = "sigma,smp8640-emac";
+			reg = <0x26000 0x800>;
+			interrupt-parent = <&irqintc>;
+			interrupts = <38 4>;
+			mac-address = [ 00 16 e8 02 08 42 ];
+			phy-connection-type = "rgmii";
+			max-speed = <1000>;
+		};
+	};
+
+	cpublock: cpublock {
+		compatible = "simple-bus";
+		reg = <0x60000 0x10000>;
+		ranges = <0x0 0x60000 0x10000>;
+		interrupt-parent = <&irqintc>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		intc: intc@e000 {
+			compatible = "sigma,tango-intc";
+			reg = <0xe000 0x1000>;
+			ranges = <0x0 0xe000 0x1000>;
+			interrupt-parent = <&gic>;
+			interrupt-controller;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			irqintc: irq@000 {
+				reg = <0x000 0x100>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 2 4>;
+				label = "IRQ";
+			};
+
+			fiqintc: fiq@100 {
+				reg = <0x100 0x100>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 3 4>;
+				label = "FIQ";
+			};
+
+			iiqintc: iiq@300 {
+				reg = <0x300 0x100>;
+				interrupt-parent = <&gic>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+				interrupts = <0 4 4>;
+				label = "IIQ";
+			};
+		};
+	};
+
+};
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c
index 172c6a05d27f..29235f7b02db 100644
--- a/arch/arm/kernel/smp_twd.c
+++ b/arch/arm/kernel/smp_twd.c
@@ -34,6 +34,7 @@  static unsigned long twd_timer_rate;
 static DEFINE_PER_CPU(bool, percpu_setup_called);
 
 static struct clock_event_device __percpu *twd_evt;
+static int feat_c3stop;
 static int twd_ppi;
 
 static void twd_set_mode(enum clock_event_mode mode,
@@ -245,7 +246,8 @@  static void twd_get_clock(struct device_node *np)
 	int err;
 
 	if (np)
-		twd_clk = of_clk_get(np, 0);
+		//twd_clk = of_clk_get(np, 0);
+		twd_clk = clk_get_sys("smp_twd", NULL);
 	else
 		twd_clk = clk_get_sys("smp_twd", NULL);
 
@@ -294,7 +296,7 @@  static void twd_timer_setup(void)
 
 	clk->name = "local_timer";
 	clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
-			CLOCK_EVT_FEAT_C3STOP;
+			feat_c3stop;
 	clk->rating = 350;
 	clk->set_mode = twd_set_mode;
 	clk->set_next_event = twd_set_next_event;
@@ -346,6 +348,8 @@  static int __init twd_local_timer_common_register(struct device_node *np)
 		goto out_irq;
 
 	twd_get_clock(np);
+	if (!of_property_read_bool(np, "twd_never_stops"))
+		feat_c3stop = CLOCK_EVT_FEAT_C3STOP;
 
 	/*
 	 * Immediately configure the timer on the boot CPU, unless we need
diff --git a/arch/arm/mach-tangox/Kconfig b/arch/arm/mach-tangox/Kconfig
new file mode 100644
index 000000000000..aba4eef9227c
--- /dev/null
+++ b/arch/arm/mach-tangox/Kconfig
@@ -0,0 +1,48 @@ 
+if ARCH_TANGOX
+
+config UNCOMPRESS_INCLUDE
+	string
+	default "debug/uncompress.h"
+
+config ARM_L1_CACHE_SHIFT
+	int
+	default 5
+
+config TANGOX
+	bool
+
+comment "Sigma Designs Tangox options"
+
+menu "Sigma Designs TANGOX Specific Features"
+
+config MACH_TANGOX_87XX
+	bool "Sigma Designs TANGOX 87XX Board"
+	default y
+	depends on ARCH_TANGOX
+	select MIGHT_HAVE_CACHE_L2X0
+	select CPU_V7
+	select ARM_GIC
+	select VFP
+	select SMP
+	select LOCAL_TIMERS if SMP
+	select HAVE_ARM_TWD if SMP
+	select HAVE_ARM_SCU if SMP
+	select PL310_ERRATA_588369
+	select PL310_ERRATA_727915
+	select ARM_ERRATA_754322
+	select ARM_ERRATA_775420
+	select ARCH_HAS_OPP
+	select PM_OPP if PM
+	select USB_ARCH_HAS_EHCI if USB_SUPPORT
+	select ARM_CPU_SUSPEND if PM
+	select CPU_USE_DOMAINS if MMU
+	select COMMON_CLK
+	select TANGOX
+
+config TANGOX_SMC
+	bool "Enable TANGOX secure monitor call operations"
+	default y
+
+endmenu
+
+endif
diff --git a/arch/arm/mach-tangox/Makefile b/arch/arm/mach-tangox/Makefile
new file mode 100644
index 000000000000..cc7f91b05577
--- /dev/null
+++ b/arch/arm/mach-tangox/Makefile
@@ -0,0 +1,7 @@ 
+obj-y += setup.o
+obj-y += clock-tangox.o
+##obj-$(CONFIG_CACHE_L2X0)	+= l2x0.o
+##obj-$(CONFIG_CPU_FREQ)		+= cpufreq.o
+
+##plus_sec := $(call as-instr,.arch_extension sec,+sec)
+##AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)
diff --git a/arch/arm/mach-tangox/Makefile.boot b/arch/arm/mach-tangox/Makefile.boot
new file mode 100644
index 000000000000..b03e562acc60
--- /dev/null
+++ b/arch/arm/mach-tangox/Makefile.boot
@@ -0,0 +1,3 @@ 
+  zreladdr-y		+= 0x80008000
+params_phys-y		:= 0x80000100
+initrd_phys-y		:= 0x80800000
diff --git a/arch/arm/mach-tangox/clock-tangox.c b/arch/arm/mach-tangox/clock-tangox.c
new file mode 100644
index 000000000000..505782329c92
--- /dev/null
+++ b/arch/arm/mach-tangox/clock-tangox.c
@@ -0,0 +1,118 @@ 
+#include <linux/clocksource.h>	/* clocksource_register_khz	*/
+#include <linux/sched_clock.h>	/* sched_clock_register		*/
+#include <linux/clk-provider.h>	/* clk_register_fixed_rate	*/
+#include <linux/clkdev.h>	/* clk_register_clkdev		*/
+#include <linux/delay.h>	/* register_current_timer_delay	*/
+
+#define FAST_RAMP		1
+#define XTAL_FREQ		27000000 /* in Hz */
+#define CLKGEN_BASE		0x10000
+#define SYS_clkgen0_pll		(clkgen_base + 0x00)
+#define SYS_cpuclk_div_ctrl	(clkgen_base + 0x24)
+#define SYS_xtal_in_cnt		(clkgen_base + 0x48)
+
+static void __iomem *clkgen_base;
+
+static unsigned long read_xtal_counter(void)
+{
+	return readl_relaxed(SYS_xtal_in_cnt);
+}
+
+static u64 read_sched_clock(void)
+{
+	return read_xtal_counter();
+}
+
+static cycle_t read_clocksource(struct clocksource *cs)
+{
+	return read_xtal_counter();
+}
+
+static struct clocksource tangox_xtal = {
+	.name	= "tangox_xtal",
+	.rating	= 300,
+	.read	= read_clocksource,
+	.mask	= CLOCKSOURCE_MASK(32),
+	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static struct delay_timer delay_timer = { read_xtal_counter, XTAL_FREQ };
+
+#define pi_alert(format, ...) do {				\
+	static char fmt[] __initdata = KERN_ALERT format;	\
+	printk(fmt, ## __VA_ARGS__);				\
+} while (0)
+
+#define REG(name, ...) union name { struct { u32 __VA_ARGS__; }; u32 val; }
+
+REG(SYS_clkgen_pll, N:7, :6, K:3, M:3, :5, Isel:3, :3, T:1, B:1);
+/*
+ * CG0, CG1, CG2, CG3 PLL Control:
+ * -------------------------------
+ *
+ * |    Byte 3     |    Byte 2     |    Byte 1     |    Byte 0     |
+ * |3 3 2 2 2 2 2 2|2 2 2 2 1 1 1 1|1 1 1 1 1 1    |               |
+ * |1 0 9 8 7 6 5 4|3 2 1 0 9 8 7 6|5 4 3 2 1 0 9 8|7 6 5 4 3 2 1 0|
+ * |-|-|-----|-----|---------|-----|-----|---------|-|-------------|
+ * |B|T|xxxxx|Isel |xxxxxxxxx|  M  |  K  |xxxxxxxxx|x|      N      |
+ * |-|-|-----|-----|---------|-----|-----|---------|-|-------------|
+ *
+ * These registers are used to configure the PLL parameters:
+ *
+ * Bits  6 to  0: N[6:0]. Default = 29
+ * Bits 15 to 13: K[2:0]. Default = 1
+ * Bit  18 to 16: M[2:0]. Default = 0
+ * Bits 26 to 24: Isel[2:0] (PLL Input Select). Default = 1
+ * Bits 30      : T (PLL Test). Default = 0
+ * Bits 31      : B (PLL Bypass). Default = 0
+ *
+ * PLL0 : Out = In * (N+1) / (M+1) / 2^K
+ * PLL1 : Same as PLL0
+ * PLL2 : Same as PLL0
+ * Default values : All PLLs configured to output 405MHz.
+ */
+static void __init tangox_clock_tree_register(void)
+{
+	struct clk *clk;
+	unsigned int mul, div;
+	union SYS_clkgen_pll pll;
+
+	pll.val = readl_relaxed(SYS_clkgen0_pll);
+	mul = pll.N + 1; div = (pll.M + 1) << pll.K;
+	if (pll.Isel != 1) pi_alert("PLL0 source is not XTAL_IN!\n");
+
+	clk = clk_register_fixed_rate(0, "XTAL", 0, CLK_IS_ROOT, XTAL_FREQ);
+	if (!clk) pi_alert("Failed to register %s clk!\n", "XTAL");
+
+	clk = clk_register_fixed_factor(0, "PLL0", "XTAL", 0, mul, div);
+	if (!clk) pi_alert("Failed to register %s clk!\n", "PLL0");
+
+	clk = clk_register_divider(0, "CPU_CLK", "PLL0", 0, SYS_cpuclk_div_ctrl, 8, 8, CLK_DIVIDER_ONE_BASED, 0);
+	if (!clk) pi_alert("Failed to register %s clk!\n", "CPU_CLK");
+	clk_register_clkdev(clk, NULL, "cpu_clk");
+
+	clk = clk_register_fixed_factor(0, "PERIPHCLK", "CPU_CLK", 0, 1, 2);
+	if (!clk) pi_alert("Failed to register %s clk!\n", "PERIPHCLK");
+	clk_register_clkdev(clk, NULL, "smp_twd");
+
+	writel_relaxed(FAST_RAMP << 21 | 1 << 8, SYS_cpuclk_div_ctrl);
+}
+
+void __init tangox_timer_init(void)
+{
+	int err;
+
+	clkgen_base = ioremap(CLKGEN_BASE, 0x100);
+	if (clkgen_base == NULL) return;
+
+	register_current_timer_delay(&delay_timer);
+	sched_clock_register(read_sched_clock, 32, XTAL_FREQ);
+
+	err = clocksource_register_hz(&tangox_xtal, XTAL_FREQ);
+	if (err) pi_alert("Failed to register tangox_xtal clocksource!\n");
+
+	tangox_clock_tree_register();
+
+	of_clk_init(NULL);
+	clocksource_of_init();
+}
diff --git a/arch/arm/mach-tangox/setup.c b/arch/arm/mach-tangox/setup.c
new file mode 100644
index 000000000000..1c694e35558d
--- /dev/null
+++ b/arch/arm/mach-tangox/setup.c
@@ -0,0 +1,34 @@ 
+#include <asm/mach/map.h>	// iotable_init
+#include <asm/mach/arch.h>	// MACHINE_START
+#include <asm/mach-types.h>	// TANGOX_87XX
+
+#define TANGO_IO_START	0xf0000000
+#define TANGO_IO_SIZE	SZ_16M
+
+static struct map_desc tango_map_desc[] __initdata = {
+	{
+		.virtual	= TANGO_IO_START,
+		.pfn		=__phys_to_pfn(0),
+		.length		= TANGO_IO_SIZE,
+		.type 		= MT_DEVICE,
+	},
+};
+
+static void __init tango_map_io(void)
+{
+	iotable_init(tango_map_desc, ARRAY_SIZE(tango_map_desc));
+}
+
+extern void __init tangox_timer_init(void);
+//extern struct smp_operations tangox_smp_ops;
+
+static const char *tango_dt_compat[] = { "sigma,tango4-soc", NULL };
+
+MACHINE_START(TANGOX_87XX, "TANGOX_87XX")
+	.atag_offset	= 0x100,
+	.init_time	= tangox_timer_init,
+	.map_io		= tango_map_io,
+	//.smp		= &tangox_smp_ops,
+	//.restart	= tango_restart, /*** REQUIRED FOR CLEAN REBOOT ***/
+	.dt_compat	= tango_dt_compat,
+MACHINE_END
diff --git a/arch/arm/tools/mach-types b/arch/arm/tools/mach-types
index 2ed1b8a922ed..5ffb975d68ea 100644
--- a/arch/arm/tools/mach-types
+++ b/arch/arm/tools/mach-types
@@ -554,6 +554,7 @@  smdk4412		MACH_SMDK4412		SMDK4412		3765
 marzen			MACH_MARZEN		MARZEN			3790
 krome			MACH_KROME		KROME			3797
 armadillo800eva		MACH_ARMADILLO800EVA	ARMADILLO800EVA		3863
+tangox_87xx		MACH_TANGOX_87XX	TANGOX_87XX		3892
 mx53_umobo		MACH_MX53_UMOBO		MX53_UMOBO		3927
 mt4			MACH_MT4		MT4			3981
 u8520			MACH_U8520		U8520			3990