Skip to contentSkip to conversation

ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

PATCH New Magnus Damm 3 participants 14 messages Jul 9, 2013

Subsystems (6)
Device tree bindings Arm (32-bit) Arm sub-architectures SuperH +2 more subsystems
lore
Share & tools
lore

Magnus Damm

author maintainer
#0 lore raw

cover ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

[PATCH 01/04] ARM: shmobile: Only build clocks when COMMON_CLK=n [PATCH 02/04] ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI [PATCH 03/04] ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI [PATCH 04/04] ARM: shmobile: Allow ARCH_SHMOBILE_MULTI timer configuration

This patch set extends mach-shmobile to allow a subset of SoCs and boards to be built for ARCH_MULTIPLATFORM. With this series applied the EMEV2 SoC and the KZM9D board can be built in two ways - either as good old ARCH_SHMOBILE or new ARCH_SHMOBILE_MULTI. The latter requires common clock framework for proper run time operation.

The goal is to over time move all DT reference implementations from ARCH_SHMOBILE to ARCH_SHMOBILE_MULTI. Until CCF is available for a certain SoC it is to me acceptable for a DT reference board to allow build in both ARCH_SHMOBILE and ARCH_SHMOBILE_MULTI. The reason for this is to allow incremental development and easy testing during development of CCF for a certain SoC. The KZM9D board is now in this state while it is waiting for common clocks to become ready.

In the near future a prototype hack will be posted that comments out the clock framework bits on EMEV2 and will allow boot of KZM9D in the case of ARCH_SHMOBILE_MULTI even though CCF is not yet available.

Signed-off-by Magnus Damm <[email protected]>

---

Built on top of renesas-next-20130704v2 and [PATCH] ARM: shmobile: KZM9D DT reference implementation

 arch/arm/Makefile                    |    1 
 arch/arm/boot/dts/Makefile           |    1 
 arch/arm/mach-shmobile/Kconfig       |   52 ++++++++++++++++++++++++++++++----
 arch/arm/mach-shmobile/Makefile      |   32 +++++++++++++++-----
 arch/arm/mach-shmobile/setup-emev2.c |    5 +++
 5 files changed, 77 insertions(+), 14 deletions(-)
author maintainer
#1 lore raw

PATCH 1/4 ARM: shmobile: Only build clocks when COMMON_CLK=n

From: Magnus Damm <[email protected]>

Move shared clock.c file and per-SoC clock-xxx.c files to only build when CONFIG_COMMON_CLK != y.

The regular ARCH_SHMOBILE case with legacy SH clock framework will build just as before with this patch applied, however the case when COMMON_CLK=y will exclude all the clock files.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/mach-shmobile/Makefile |   30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)
arch/arm/mach-shmobile/Makefile +21 -9
@@ -3,17 +3,28 @@
 #
 
 # Common objects
-obj-y				:= timer.o console.o clock.o
+obj-y				:= timer.o console.o
 
 # CPU objects
-obj-$(CONFIG_ARCH_SH7372)	+= setup-sh7372.o clock-sh7372.o intc-sh7372.o
-obj-$(CONFIG_ARCH_SH73A0)	+= setup-sh73a0.o clock-sh73a0.o intc-sh73a0.o
-obj-$(CONFIG_ARCH_R8A73A4)	+= setup-r8a73a4.o clock-r8a73a4.o
-obj-$(CONFIG_ARCH_R8A7740)	+= setup-r8a7740.o clock-r8a7740.o intc-r8a7740.o
-obj-$(CONFIG_ARCH_R8A7778)	+= setup-r8a7778.o clock-r8a7778.o
-obj-$(CONFIG_ARCH_R8A7779)	+= setup-r8a7779.o clock-r8a7779.o intc-r8a7779.o
-obj-$(CONFIG_ARCH_R8A7790)	+= setup-r8a7790.o clock-r8a7790.o
-obj-$(CONFIG_ARCH_EMEV2)	+= setup-emev2.o clock-emev2.o
+obj-$(CONFIG_ARCH_SH7372)	+= setup-sh7372.o intc-sh7372.o
+obj-$(CONFIG_ARCH_SH73A0)	+= setup-sh73a0.o intc-sh73a0.o
+obj-$(CONFIG_ARCH_R8A73A4)	+= setup-r8a73a4.o
+obj-$(CONFIG_ARCH_R8A7740)	+= setup-r8a7740.o intc-r8a7740.o
+obj-$(CONFIG_ARCH_R8A7778)	+= setup-r8a7778.o
+obj-$(CONFIG_ARCH_R8A7779)	+= setup-r8a7779.o intc-r8a7779.o
+obj-$(CONFIG_ARCH_R8A7790)	+= setup-r8a7790.o
+obj-$(CONFIG_ARCH_EMEV2)	+= setup-emev2.o
+
+# Clock objects
+clk-y				:= clock.o
+clk-$(CONFIG_ARCH_SH7372)	+= clock-sh7372.o
+clk-$(CONFIG_ARCH_SH73A0)	+= clock-sh73a0.o
+clk-$(CONFIG_ARCH_R8A73A4)	+= clock-r8a73a4.o
+clk-$(CONFIG_ARCH_R8A7740)	+= clock-r8a7740.o
+clk-$(CONFIG_ARCH_R8A7778)	+= clock-r8a7778.o
+clk-$(CONFIG_ARCH_R8A7779)	+= clock-r8a7779.o
+clk-$(CONFIG_ARCH_R8A7790)	+= clock-r8a7790.o
+clk-$(CONFIG_ARCH_EMEV2)	+= clock-emev2.o
 
 # SMP objects
 smp-y				:= platsmp.o headsmp.o

@@ -53,3 +64,4 @@ obj-$(CONFIG_MACH_KZM9G_REFERENCE)	+= bo
 
 # Framework support
 obj-$(CONFIG_SMP)		+= $(smp-y)
+obj-y				+= $(clk-y$(CONFIG_COMMON_CLK))
maintainer
#2 lore raw

PATCH 1/4 ARM: shmobile: Only build clocks when COMMON_CLK=n

Quoted 1 line · Magnus Damm

+obj-y += $(clk-y$(CONFIG_COMMON_CLK))

It took me a bit to understand the Makefile trick you are using here. I assume other readers will have the same problem. How about just doing

+ifndef CONFIG_COMMON_CLK

 obj-$(CONFIG_ARCH_SH7372)      += setup-sh7372.o intc-sh7372.o
 obj-$(CONFIG_ARCH_SH73A0)      += setup-sh73a0.o intc-sh73a0.o
 obj-$(CONFIG_ARCH_R8A73A4)     += setup-r8a73a4.o
 obj-$(CONFIG_ARCH_R8A7740)     += setup-r8a7740.o intc-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7778)     += setup-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)     += setup-r8a7779.o intc-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)     += setup-r8a7790.o
 obj-$(CONFIG_ARCH_EMEV2)       += setup-emev2.o

+endif

Arnd

#3 lore raw

PATCH 1/4 ARM: shmobile: Only build clocks when COMMON_CLK=n

Hi Arnd and Magnus,

Quoted 7 lines · On Tuesday 09 July 2013 09:43:07 Arnd Bergmann wrote:
Quoted 1 line · Magnus Damm

+obj-y += $(clk-y$(CONFIG_COMMON_CLK))

It took me a bit to understand the Makefile trick you are using here. I assume other readers will have the same problem. How about just doing

Review

Quoted 10 lines
(quoted diff) +2 -0
+ifndef CONFIG_COMMON_CLK
 obj-$(CONFIG_ARCH_SH7372)      += setup-sh7372.o intc-sh7372.o
 obj-$(CONFIG_ARCH_SH73A0)      += setup-sh73a0.o intc-sh73a0.o
 obj-$(CONFIG_ARCH_R8A73A4)     += setup-r8a73a4.o
 obj-$(CONFIG_ARCH_R8A7740)     += setup-r8a7740.o intc-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7778)     += setup-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)     += setup-r8a7779.o intc-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)     += setup-r8a7790.o
 obj-$(CONFIG_ARCH_EMEV2)       += setup-emev2.o
+endif

I was about to mention the same, so I think that would be a good idea.

Signature · 3 lines
Regards,

Laurent Pinchart
author maintainer
#4 lore raw

PATCH 1/4 ARM: shmobile: Only build clocks when COMMON_CLK=n

Hi Laurent and Arnd,

Quoted 10 lines · Laurent Pinchart

Hi Arnd and Magnus,

Quoted 7 lines · On Tuesday 09 July 2013 09:43:07 Arnd Bergmann wrote:
Quoted 1 line · Magnus Damm

+obj-y += $(clk-y$(CONFIG_COMMON_CLK))

It took me a bit to understand the Makefile trick you are using here. I assume other readers will have the same problem. How about just doing

Quoted 12 lines

Review

Quoted 10 lines
(quoted diff) +2 -0
+ifndef CONFIG_COMMON_CLK
 obj-$(CONFIG_ARCH_SH7372)      += setup-sh7372.o intc-sh7372.o
 obj-$(CONFIG_ARCH_SH73A0)      += setup-sh73a0.o intc-sh73a0.o
 obj-$(CONFIG_ARCH_R8A73A4)     += setup-r8a73a4.o
 obj-$(CONFIG_ARCH_R8A7740)     += setup-r8a7740.o intc-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7778)     += setup-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)     += setup-r8a7779.o intc-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)     += setup-r8a7790.o
 obj-$(CONFIG_ARCH_EMEV2)       += setup-emev2.o
+endif

I was about to mention the same, so I think that would be a good idea.

Sure, that may be a more common way to handle this. =)

To be clear, I assume you wanted that ifndef to cover the clock-xxx.c files instead of setup-xxx.c. So I will do that in next version.

Thanks for your comments!

Cheers,

/ magnus

maintainer
#5 lore raw

PATCH 1/4 ARM: shmobile: Only build clocks when COMMON_CLK=n

Quoted 14 lines · Magnus Damm
Quoted 13 lines

Review

Quoted 11 lines
(quoted diff) +2 -0
+ifndef CONFIG_COMMON_CLK
 obj-$(CONFIG_ARCH_SH7372)      += setup-sh7372.o intc-sh7372.o
 obj-$(CONFIG_ARCH_SH73A0)      += setup-sh73a0.o intc-sh73a0.o
 obj-$(CONFIG_ARCH_R8A73A4)     += setup-r8a73a4.o
 obj-$(CONFIG_ARCH_R8A7740)     += setup-r8a7740.o intc-r8a7740.o
 obj-$(CONFIG_ARCH_R8A7778)     += setup-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)     += setup-r8a7779.o intc-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)     += setup-r8a7790.o
 obj-$(CONFIG_ARCH_EMEV2)       += setup-emev2.o
+endif

I was about to mention the same, so I think that would be a good idea.

Quoted 5 lines

Sure, that may be a more common way to handle this. =)

To be clear, I assume you wanted that ifndef to cover the clock-xxx.c files instead of setup-xxx.c. So I will do that in next version.

Yes, that's right, sorry for screwing up the example.

Arnd

author maintainer
#6 lore raw

PATCH 2/4 ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Add ARCH_SHMOBILE_MULTI to mach-shmobile that can be used to enable ARCH_MULTIPLATFORM on selected SoCs and boards.

The headers stay under arch/arm/mach-shmobile/include/mach/ for now, they can and will be migrated independently over time.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/Makefile               |    1 +
 arch/arm/mach-shmobile/Kconfig  |   21 +++++++++++++++++++++
 arch/arm/mach-shmobile/Makefile |    2 ++
 3 files changed, 24 insertions(+)
arch/arm/Makefile +1 -0
@@ -181,6 +181,7 @@ machine-$(CONFIG_ARCH_EXYNOS)		+= exynos
 machine-$(CONFIG_ARCH_SA1100)		+= sa1100
 machine-$(CONFIG_ARCH_SHARK)		+= shark
 machine-$(CONFIG_ARCH_SHMOBILE) 	+= shmobile
+machine-$(CONFIG_ARCH_SHMOBILE_MULTI) 	+= shmobile
 machine-$(CONFIG_ARCH_TEGRA)		+= tegra
 machine-$(CONFIG_ARCH_U300)		+= u300
 machine-$(CONFIG_ARCH_U8500)		+= ux500
arch/arm/mach-shmobile/Kconfig +21 -0
@@ -1,3 +1,24 @@
+config ARCH_SHMOBILE_MULTI
+	bool "SH-Mobile Series" if ARCH_MULTI_V7
+	depends on MMU
+	select CPU_V7
+	select GENERIC_CLOCKEVENTS
+	select HAVE_ARM_SCU if SMP
+	select HAVE_ARM_TWD if LOCAL_TIMERS
+	select HAVE_SMP
+	select ARM_GIC
+	select MIGHT_HAVE_CACHE_L2X0
+	select NO_IOPORT
+	select PINCTRL
+	select ARCH_REQUIRE_GPIOLIB
+	select CLKDEV_LOOKUP
+
+if ARCH_SHMOBILE_MULTI
+
+comment "SH-Mobile System Type"
+
+endif
+
 if ARCH_SHMOBILE
 
 comment "SH-Mobile System Type"
arch/arm/mach-shmobile/Makefile +2 -0
@@ -2,6 +2,8 @@
 # Makefile for the linux kernel.
 #
 
+ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/arch/arm/mach-shmobile/include
+
 # Common objects
 obj-y				:= timer.o console.o
author maintainer
#7 lore raw

PATCH 3/4 ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Enable build of EMEV2 and KZM9D DT Reference in case of ARCH_MULTIPLATFORM and ARCH_SHMOBILE_MULTI.

A weak symbol covers the case when the clock-emev2.c file is not built when COMMON_CLK=y.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/boot/dts/Makefile           |    1 +
 arch/arm/mach-shmobile/Kconfig       |   15 +++++++++++++++
 arch/arm/mach-shmobile/setup-emev2.c |    5 +++++
 3 files changed, 21 insertions(+)
arch/arm/boot/dts/Makefile +1 -0
@@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_SHMOBILE) += emev2-kzm
 	sh73a0-kzm9g-reference.dtb \
 	r8a73a4-ape6evm.dtb \
 	sh7372-mackerel.dtb
+dtb-$(CONFIG_ARCH_SHMOBILE_MULTI) += emev2-kzm9d-reference.dtb
 dtb-$(CONFIG_ARCH_SOCFPGA) += socfpga_cyclone5.dtb \
 	socfpga_vt.dtb
 dtb-$(CONFIG_ARCH_SPEAR13XX) += spear1310-evb.dtb \
arch/arm/mach-shmobile/Kconfig +15 -0
@@ -17,6 +17,21 @@ if ARCH_SHMOBILE_MULTI
 
 comment "SH-Mobile System Type"
 
+config ARCH_EMEV2
+	bool "Emma Mobile EV2"
+
+comment "SH-Mobile Board Type"
+
+config MACH_KZM9D_REFERENCE
+	bool "KZM9D board - Reference Device Tree Implementation"
+	depends on ARCH_EMEV2
+	select REGULATOR_FIXED_VOLTAGE if REGULATOR
+	---help---
+	   Use reference implementation of KZM9D board support
+	   which makes a greater use of device tree at the expense
+	   of not supporting a number of devices.
+
+	   This is intended to aid developers
 endif
 
 if ARCH_SHMOBILE
arch/arm/mach-shmobile/setup-emev2.c +5 -0
@@ -190,6 +190,11 @@ void __init emev2_add_standard_devices(v
 	emev2_register_pmu();
 }
 
+void __init __weak emev2_clock_init(void)
+{
+	/* in case legacy clocks are disabled then do nothing */
+}
+
 void __init emev2_init_delay(void)
 {
 	shmobile_setup_delay(533, 1, 3); /* Cortex-A9 @ 533MHz */
maintainer
#8 lore raw

PATCH 3/4 ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI

arch/arm/mach-shmobile/setup-emev2.c void __init emev2_add_standard_devices(v · line 196

Quoted 4 lines · Magnus Damm
arch/arm/mach-shmobile/setup-emev2.c +4 -0
+void __init __weak emev2_clock_init(void)
+{
+       /* in case legacy clocks are disabled then do nothing */
+}

Nitpicking:

I don't like the __weak symbols too much, in general I prefer doing either

	if (IS_ENABLED(CONFIG_SHMOBILE_CLOCKS)
		emev2_clock_init()

or using an inline alternative in the header file.

Arnd

author maintainer
#9 lore raw

PATCH 3/4 ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI

Hi Arnd,

Quoted 8 lines · Arnd Bergmann

arch/arm/mach-shmobile/setup-emev2.c void __init emev2_add_standard_devices(v · line 196

Quoted 4 lines · Magnus Damm
arch/arm/mach-shmobile/setup-emev2.c +4 -0
+void __init __weak emev2_clock_init(void)
+{
+       /* in case legacy clocks are disabled then do nothing */
+}

Nitpicking:

Quoted 7 lines

I don't like the __weak symbols too much, in general I prefer doing either

        if (IS_ENABLED(CONFIG_SHMOBILE_CLOCKS)
                emev2_clock_init()

or using an inline alternative in the header file.

Sure, good idea. I plan on using the following if you don't mind:

         if (!IS_ENABLED(CONFIG_COMMON_CLK)
                 emev2_clock_init()

Cheers,

/ magnus

maintainer
#10 lore raw

PATCH 3/4 ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI

Quoted 5 lines · Magnus Damm

Sure, good idea. I plan on using the following if you don't mind:

         if (!IS_ENABLED(CONFIG_COMMON_CLK)
                 emev2_clock_init()

Yes, looks good.

Arnd

author maintainer
#11 lore raw

PATCH 4/4 ARM: shmobile: Allow ARCH_SHMOBILE_MULTI timer configuration

In reply to Magnus Damm

From: Magnus Damm <[email protected]>

Move the timer configuration bits to allow use in
case of ARCH_SHMOBILE or ARCH_SHMOBILE_MULTI.

The timers all make use of the regular driver model so they are safe to enable for ARCH_MULTIPLATFORM.

Signed-off-by Magnus Damm <[email protected]>

---

 arch/arm/mach-shmobile/Kconfig |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
arch/arm/mach-shmobile/Kconfig +11 -5
@@ -30,6 +30,8 @@ config MACH_KZM9D_REFERENCE
 	   of not supporting a number of devices.
 
 	   This is intended to aid developers
+
+comment "SH-Mobile System Configuration"
 endif
 
 if ARCH_SHMOBILE

@@ -243,6 +245,15 @@ config CPU_HAS_INTEVT
         bool
 	default y
 
+config SH_CLK_CPG
+	bool
+
+source "drivers/sh/Kconfig"
+
+endif
+
+if ARCH_SHMOBILE || ARCH_SHMOBILE_MULTI
+
 menu "Timer and clock configuration"
 
 config SHMOBILE_TIMER_HZ

@@ -277,9 +288,4 @@ config EM_TIMER_STI
 
 endmenu
 
-config SH_CLK_CPG
-	bool
-
-source "drivers/sh/Kconfig"
-
 endif
maintainer
#12 lore raw

cover ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

In reply to Magnus Damm
Quoted 21 lines · Magnus Damm

ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

[PATCH 01/04] ARM: shmobile: Only build clocks when COMMON_CLK=n [PATCH 02/04] ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI [PATCH 03/04] ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI [PATCH 04/04] ARM: shmobile: Allow ARCH_SHMOBILE_MULTI timer configuration

This patch set extends mach-shmobile to allow a subset of SoCs and boards to be built for ARCH_MULTIPLATFORM. With this series applied the EMEV2 SoC and the KZM9D board can be built in two ways - either as good old ARCH_SHMOBILE or new ARCH_SHMOBILE_MULTI. The latter requires common clock framework for proper run time operation.

The goal is to over time move all DT reference implementations from ARCH_SHMOBILE to ARCH_SHMOBILE_MULTI. Until CCF is available for a certain SoC it is to me acceptable for a DT reference board to allow build in both ARCH_SHMOBILE and ARCH_SHMOBILE_MULTI. The reason for this is to allow incremental development and easy testing during development of CCF for a certain SoC. The KZM9D board is now in this state while it is waiting for common clocks to become ready.

Quoted 6 lines

In the near future a prototype hack will be posted that comments out the clock framework bits on EMEV2 and will allow boot of KZM9D in the case of ARCH_SHMOBILE_MULTI even though CCF is not yet available.

Signed-off-by Magnus Damm <[email protected]>

Great stuff, very glad you got this far!

I have two small style comments, nothing important.

Arnd

author maintainer
#13 lore raw

cover ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

Quoted 22 lines · Arnd Bergmann
Quoted 21 lines · Magnus Damm

ARM: shmobile: ARCH_MULTIPLATFORM for EMEV2 and KZM9D

[PATCH 01/04] ARM: shmobile: Only build clocks when COMMON_CLK=n [PATCH 02/04] ARM: shmobile: Introduce ARCH_SHMOBILE_MULTI [PATCH 03/04] ARM: shmobile: Add EMEV2 and KZM9D to ARCH_SHMOBILE_MULTI [PATCH 04/04] ARM: shmobile: Allow ARCH_SHMOBILE_MULTI timer configuration

This patch set extends mach-shmobile to allow a subset of SoCs and boards to be built for ARCH_MULTIPLATFORM. With this series applied the EMEV2 SoC and the KZM9D board can be built in two ways - either as good old ARCH_SHMOBILE or new ARCH_SHMOBILE_MULTI. The latter requires common clock framework for proper run time operation.

The goal is to over time move all DT reference implementations from ARCH_SHMOBILE to ARCH_SHMOBILE_MULTI. Until CCF is available for a certain SoC it is to me acceptable for a DT reference board to allow build in both ARCH_SHMOBILE and ARCH_SHMOBILE_MULTI. The reason for this is to allow incremental development and easy testing during development of CCF for a certain SoC. The KZM9D board is now in this state while it is waiting for common clocks to become ready.

Quoted 8 lines
Quoted 6 lines

In the near future a prototype hack will be posted that comments out the clock framework bits on EMEV2 and will allow boot of KZM9D in the case of ARCH_SHMOBILE_MULTI even though CCF is not yet available.

Signed-off-by Magnus Damm <[email protected]>

Great stuff, very glad you got this far!

Thanks, I'm happy to hear that!

Quoted 1 line

I have two small style comments, nothing important.

Sure, I will fix up and resend. Thanks for your feedback!

/ magnus