[PATCH 0/4] Unrelated fixes found while DT:ing clocks

[PATCH 0/4] Unrelated fixes found while DT:ing clocks

[Lee Jones]

.. only sent as a patch-set for convenience.

[PATCH 1/4] mfd: dbx500-prcmu: Use correct names for clock management registers

[Lee Jones]

... as stipulated by the Hardware Specification document.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/dbx500-prcmu-regs.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
index d14836e..7cc32a8 100644
--- a/drivers/mfd/dbx500-prcmu-regs.h
+++ b/drivers/mfd/dbx500-prcmu-regs.h
@@ -16,8 +16,8 @@
 #define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
 
 #define PRCM_ACLK_MGT		(0x004)
-#define PRCM_SVACLK_MGT		(0x008)
-#define PRCM_SIACLK_MGT		(0x00C)
+#define PRCM_SVAMMCSPCLK_MGT	(0x008)
+#define PRCM_SIAMMDSPCLK_MGT	(0x00C)
 #define PRCM_SGACLK_MGT		(0x014)
 #define PRCM_UARTCLK_MGT	(0x018)
 #define PRCM_MSP02CLK_MGT	(0x01C)
-- 
1.7.10.4

[PATCH 1/4] mfd: dbx500-prcmu: Use correct names for clock management registers

[Linus Walleij]

On Thu, Jun 6, 2013 at 1:21 PM, Lee Jones <lee.jones@linaro.org> wrote:

> ... as stipulated by the Hardware Specification document.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/4] mfd: dbx500-prcmu: Correctly reorder PRCMU clock identifiers

[Lee Jones]

... as stipulated by the Hardware Specification document.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/mfd/dbx500-prcmu.h |  110 ++++++++++++++++++++------------------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h
index 689e6a0..b6a61d3 100644
--- a/include/linux/mfd/dbx500-prcmu.h
+++ b/include/linux/mfd/dbx500-prcmu.h
@@ -97,58 +97,64 @@ enum prcmu_wakeup_index {
 /*
  * Clock identifiers.
  */
-enum prcmu_clock {
-	PRCMU_SGACLK,
-	PRCMU_UARTCLK,
-	PRCMU_MSP02CLK,
-	PRCMU_MSP1CLK,
-	PRCMU_I2CCLK,
-	PRCMU_SDMMCCLK,
-	PRCMU_SPARE1CLK,
-	PRCMU_SLIMCLK,
-	PRCMU_PER1CLK,
-	PRCMU_PER2CLK,
-	PRCMU_PER3CLK,
-	PRCMU_PER5CLK,
-	PRCMU_PER6CLK,
-	PRCMU_PER7CLK,
-	PRCMU_LCDCLK,
-	PRCMU_BMLCLK,
-	PRCMU_HSITXCLK,
-	PRCMU_HSIRXCLK,
-	PRCMU_HDMICLK,
-	PRCMU_APEATCLK,
-	PRCMU_APETRACECLK,
-	PRCMU_MCDECLK,
-	PRCMU_IPI2CCLK,
-	PRCMU_DSIALTCLK,
-	PRCMU_DMACLK,
-	PRCMU_B2R2CLK,
-	PRCMU_TVCLK,
-	PRCMU_SSPCLK,
-	PRCMU_RNGCLK,
-	PRCMU_UICCCLK,
-	PRCMU_PWMCLK,
-	PRCMU_IRDACLK,
-	PRCMU_IRRCCLK,
-	PRCMU_SIACLK,
-	PRCMU_SVACLK,
-	PRCMU_ACLK,
-	PRCMU_NUM_REG_CLOCKS,
-	PRCMU_SYSCLK = PRCMU_NUM_REG_CLOCKS,
-	PRCMU_CDCLK,
-	PRCMU_TIMCLK,
-	PRCMU_PLLSOC0,
-	PRCMU_PLLSOC1,
-	PRCMU_ARMSS,
-	PRCMU_PLLDDR,
-	PRCMU_PLLDSI,
-	PRCMU_DSI0CLK,
-	PRCMU_DSI1CLK,
-	PRCMU_DSI0ESCCLK,
-	PRCMU_DSI1ESCCLK,
-	PRCMU_DSI2ESCCLK,
-};
+
+#define ARMCLK			0
+#define PRCMU_ACLK		1
+#define PRCMU_SVAMMCSPCLK 	2
+#define PRCMU_SIACLK 		3
+#define PRCMU_SGACLK 		4
+#define PRCMU_UARTCLK 		5
+#define PRCMU_MSP02CLK 		6
+#define PRCMU_MSP1CLK 		7
+#define PRCMU_I2CCLK 		8
+#define PRCMU_SDMMCCLK 		9
+#define PRCMU_SLIMCLK 		10
+#define PRCMU_PER1CLK 		11
+#define PRCMU_PER2CLK 		12
+#define PRCMU_PER3CLK 		13
+#define PRCMU_PER5CLK 		14
+#define PRCMU_PER6CLK 		15
+#define PRCMU_PER7CLK 		16
+#define PRCMU_LCDCLK 		17
+#define PRCMU_BMLCLK 		18
+#define PRCMU_HSITXCLK 		19
+#define PRCMU_HSIRXCLK 		20
+#define PRCMU_HDMICLK		21
+#define PRCMU_APEATCLK 		22
+#define PRCMU_APETRACECLK 	23
+#define PRCMU_MCDECLK  	 	24
+#define PRCMU_IPI2CCLK  	25
+#define PRCMU_DSIALTCLK  	26
+#define PRCMU_DMACLK  	 	27
+#define PRCMU_B2R2CLK  	 	28
+#define PRCMU_TVCLK  	 	29
+#define SPARE_UNIPROCLK  	30
+#define PRCMU_SSPCLK  	 	31
+#define PRCMU_RNGCLK  	 	32
+#define PRCMU_UICCCLK  	 	33
+#define SPARE1CLK  	 	34
+#define SPARE2CLK  	 	35
+
+#define PRCMU_NUM_REG_CLOCKS  	39
+
+#define PRCMU_RTCCLK  	 	PRCMU_NUM_REG_CLOCKS
+#define PRCMU_SYSCLK  	 	40
+#define PRCMU_CDCLK  	 	41
+#define PRCMU_TIMCLK  	 	42
+#define PRCMU_PLLSOC0  	 	43
+#define PRCMU_PLLSOC1  	 	44
+#define PRCMU_ARMSS  	 	45
+#define PRCMU_PLLDDR  	 	46
+
+/* DSI Clocks */
+#define PRCMU_PLLDSI  	 	47
+#define PRCMU_DSI0CLK 	  	48
+#define PRCMU_DSI1CLK  	 	49
+#define PRCMU_DSI0ESCCLK  	50
+#define PRCMU_DSI1ESCCLK  	51
+#define PRCMU_DSI2ESCCLK  	52
+
+#define PRCMU_NUM_CLKS  	53
 
 /**
  * enum prcmu_wdog_id - PRCMU watchdog IDs
-- 
1.7.10.4

[PATCH 2/4] mfd: dbx500-prcmu: Correctly reorder PRCMU clock identifiers

[Linus Walleij]

On Thu, Jun 6, 2013 at 1:21 PM, Lee Jones <lee.jones@linaro.org> wrote:

> ... as stipulated by the Hardware Specification document.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I think you need Ulf (as ux500 clock author) and Loic (as PRCMU
developer) to have a look at this.

> -enum prcmu_clock {
> -       PRCMU_SGACLK,
(...)
> +#define ARMCLK                 0
> +#define PRCMU_ACLK             1
(...)

It seems the enum and the defines want to do two different things?

The first is a kernel-internal representation of the clocks, whereas
the latter is a HW-centric representation for a certain PRCMU
variant (I guess?)

Does one actually exclude the other?

Yours,
Linus Walleij

[PATCH 2/4] mfd: dbx500-prcmu: Correctly reorder PRCMU clock identifiers

[Lee Jones]

On Fri, 07 Jun 2013, Linus Walleij wrote:

> On Thu, Jun 6, 2013 at 1:21 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > ... as stipulated by the Hardware Specification document.
> >
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> I think you need Ulf (as ux500 clock author) and Loic (as PRCMU
> developer) to have a look at this.
> 
> > -enum prcmu_clock {
> > -       PRCMU_SGACLK,
> (...)
> > +#define ARMCLK                 0
> > +#define PRCMU_ACLK             1
> (...)
> 
> It seems the enum and the defines want to do two different things?
> 
> The first is a kernel-internal representation of the clocks, whereas
> the latter is a HW-centric representation for a certain PRCMU
> variant (I guess?)
> 
> Does one actually exclude the other?

>From what I can tell, they're non-ordered values which are used as
read-ins to the clk_mgt array. So long as they are unique I'm not sure
the order matters. The current order seems 'made up', unless I'm
mistaken.

I agree that Ulf should take a look and confirm/deny my findings.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/4] mfd: dbx500-prcmu: Correctly reorder PRCMU clock identifiers

[Linus Walleij]

On Fri, Jun 7, 2013 at 4:20 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 07 Jun 2013, Linus Walleij wrote:
>
>> > -enum prcmu_clock {
>> > -       PRCMU_SGACLK,
>> (...)
>> > +#define ARMCLK                 0
>> > +#define PRCMU_ACLK             1
>> (...)
>>
>> It seems the enum and the defines want to do two different things?
>>
>> The first is a kernel-internal representation of the clocks, whereas
>> the latter is a HW-centric representation for a certain PRCMU
>> variant (I guess?)
>>
>> Does one actually exclude the other?
>
> From what I can tell, they're non-ordered values which are used as
> read-ins to the clk_mgt array. So long as they are unique I'm not sure
> the order matters. The current order seems 'made up', unless I'm
> mistaken.

Hm, I suspect it might be something to do with the enumeration
inside the PRCMU firmware, but I don't really know. In that case it
could affect the message marshalling, but let's call out the experts...

Yours,
Linus Walleij

[PATCH 3/4] ARM: ux500: Fix trivial whitespace/tabbing issue

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/dbx5x0.dtsi |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/dbx5x0.dtsi b/arch/arm/boot/dts/dbx5x0.dtsi
index ed5655d..13ebe20 100644
--- a/arch/arm/boot/dts/dbx5x0.dtsi
+++ b/arch/arm/boot/dts/dbx5x0.dtsi
@@ -297,7 +297,7 @@
 					     <22 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-names = "IRQ_HOTMON_LOW", "IRQ_HOTMON_HIGH";
 				status = "disabled";
-			 };
+			};
 
 			db8500-prcmu-regulators {
 				compatible = "stericsson,db8500-prcmu-regulator";
-- 
1.7.10.4

[PATCH 3/4] ARM: ux500: Fix trivial whitespace/tabbing issue

[Linus Walleij]

On Thu, Jun 6, 2013 at 1:21 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Applied to my ux500-devicetree branch.

Yours,
Linus Walleij

[PATCH 4/4] ARM: ux500: Remove call to u8500_of_init_devices() - again

[Lee Jones]

Somehow, probably due to a merge error the call to u8500_of_init_devices()
has re-appeared in next, causing this error:

arch/arm/mach-ux500/cpu-db8500.c: In function ?u8500_init_machine?:
arch/arm/mach-ux500/cpu-db8500.c:268:2: error: implicit declaration of     \
  function ?u8500_of_init_devices? [-Werror=implicit-function-declaration]
arch/arm/mach-ux500/cpu-db8500.c:268:9: warning: assignment makes pointer  \
  from integer without a cast [enabled by default]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/cpu-db8500.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
index d8ee616..f391d08 100644
--- a/arch/arm/mach-ux500/cpu-db8500.c
+++ b/arch/arm/mach-ux500/cpu-db8500.c
@@ -264,9 +264,6 @@ static void __init u8500_init_machine(void)
 	else if (of_machine_is_compatible("st-ericsson,ccu9540")) {}
 		/* TODO: Add pinmaps for ccu9540 board. */
 
-	/* TODO: Export SoC, USB, cpu-freq and DMA40 */
-	parent = u8500_of_init_devices();
-
 	/* automatically probe child nodes of dbx5x0 devices */
 	if (of_machine_is_compatible("st-ericsson,u8540"))
 		of_platform_populate(NULL, u8500_local_bus_nodes,
-- 
1.7.10.4

[PATCH 4/4] ARM: ux500: Remove call to u8500_of_init_devices() - again

[Arnd Bergmann]

On Thursday 06 June 2013, Lee Jones wrote:
> Somehow, probably due to a merge error the call to u8500_of_init_devices()
> has re-appeared in next, causing this error:
> 
> arch/arm/mach-ux500/cpu-db8500.c: In function ?u8500_init_machine?:
> arch/arm/mach-ux500/cpu-db8500.c:268:2: error: implicit declaration of     \
>   function ?u8500_of_init_devices? [-Werror=implicit-function-declaration]
> arch/arm/mach-ux500/cpu-db8500.c:268:9: warning: assignment makes pointer  \
>   from integer without a cast [enabled by default]
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/mach-ux500/cpu-db8500.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
> index d8ee616..f391d08 100644
> --- a/arch/arm/mach-ux500/cpu-db8500.c
> +++ b/arch/arm/mach-ux500/cpu-db8500.c
> @@ -264,9 +264,6 @@ static void __init u8500_init_machine(void)
>  	else if (of_machine_is_compatible("st-ericsson,ccu9540")) {}
>  		/* TODO: Add pinmaps for ccu9540 board. */
>  
> -	/* TODO: Export SoC, USB, cpu-freq and DMA40 */
> -	parent = u8500_of_init_devices();
> -
>  	/* automatically probe child nodes of dbx5x0 devices */
>  	if (of_machine_is_compatible("st-ericsson,u8540"))
>  		of_platform_populate(NULL, u8500_local_bus_nodes,
> -- 
> 1.7.10.4
> 
> 

In the code I'm looking at, the definition for that function is

/* TODO: Once all pieces are DT:ed, remove completely. */
static struct device * __init u8500_of_init_devices(void)
{
        struct device *parent = db8500_soc_device_init();
        db8500_add_usb(parent, usb_db8500_dma_cfg, usb_db8500_dma_cfg);
        return parent;
}

I'm not sure about the db8500_add_usb() call, but I think at least
db8500_soc_device_init() is required to get the root device for all
other devices.

	Arnd

[PATCH 4/4] ARM: ux500: Remove call to u8500_of_init_devices() - again

[Lee Jones]

On Thu, 06 Jun 2013, Arnd Bergmann wrote:

> On Thursday 06 June 2013, Lee Jones wrote:
> > Somehow, probably due to a merge error the call to u8500_of_init_devices()
> > has re-appeared in next, causing this error:
> > 
> > arch/arm/mach-ux500/cpu-db8500.c: In function ?u8500_init_machine?:
> > arch/arm/mach-ux500/cpu-db8500.c:268:2: error: implicit declaration of     \
> >   function ?u8500_of_init_devices? [-Werror=implicit-function-declaration]
> > arch/arm/mach-ux500/cpu-db8500.c:268:9: warning: assignment makes pointer  \
> >   from integer without a cast [enabled by default]
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/mach-ux500/cpu-db8500.c |    3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-ux500/cpu-db8500.c b/arch/arm/mach-ux500/cpu-db8500.c
> > index d8ee616..f391d08 100644
> > --- a/arch/arm/mach-ux500/cpu-db8500.c
> > +++ b/arch/arm/mach-ux500/cpu-db8500.c
> > @@ -264,9 +264,6 @@ static void __init u8500_init_machine(void)
> >  	else if (of_machine_is_compatible("st-ericsson,ccu9540")) {}
> >  		/* TODO: Add pinmaps for ccu9540 board. */
> >  
> > -	/* TODO: Export SoC, USB, cpu-freq and DMA40 */
> > -	parent = u8500_of_init_devices();
> > -
> >  	/* automatically probe child nodes of dbx5x0 devices */
> >  	if (of_machine_is_compatible("st-ericsson,u8540"))
> >  		of_platform_populate(NULL, u8500_local_bus_nodes,
> 
> In the code I'm looking at, the definition for that function is
> 
> /* TODO: Once all pieces are DT:ed, remove completely. */
> static struct device * __init u8500_of_init_devices(void)
> {
>         struct device *parent = db8500_soc_device_init();
>         db8500_add_usb(parent, usb_db8500_dma_cfg, usb_db8500_dma_cfg);
>         return parent;
> }
> 
> I'm not sure about the db8500_add_usb() call, but I think at least
> db8500_soc_device_init() is required to get the root device for all
> other devices.

Right:

https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-stericsson.git/commit/arch/arm/mach-ux500/cpu-db8500.c?h=next&id=1e74043a29cdec67279d6e4f494dd1674a1b2365

For some reason (I suspect a merge error), u8500_of_init_devices
reappeared.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] ARM: ux500: Remove call to u8500_of_init_devices() - again

[Arnd Bergmann]

On Thursday 06 June 2013, Lee Jones wrote:
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-stericsson.git/commit/arch/arm/mach-ux500/cpu-db8500.c?h=next&id=1e74043a29cdec67279d6e4f494dd1674a1b2365
> 
> For some reason (I suspect a merge error), u8500_of_init_devices
> reappeared.
> 

>From what I can see, that match you cited has never made it into the mainline kernel.

	Arnd

[PATCH 4/4] ARM: ux500: Remove call to u8500_of_init_devices() - again

[Linus Walleij]

On Thu, Jun 6, 2013 at 1:21 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Somehow, probably due to a merge error the call to u8500_of_init_devices()
> has re-appeared in next, causing this error:
>
> arch/arm/mach-ux500/cpu-db8500.c: In function ?u8500_init_machine?:
> arch/arm/mach-ux500/cpu-db8500.c:268:2: error: implicit declaration of     \
>   function ?u8500_of_init_devices? [-Werror=implicit-function-declaration]
> arch/arm/mach-ux500/cpu-db8500.c:268:9: warning: assignment makes pointer  \
>   from integer without a cast [enabled by default]
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Looking closer this seems like it's fixing a merge error that appeared in
my linux-stericsson mixdown branch "next" at one point, and which was
fixed three days ago.

The ARM SoC maintainers may need to watch out for that merge error
when pullin in my 5 pull requests sent recently though, that is
essentially the basic merge conflict between the DMA40 and
general devicetree work.

Yours,
Linus Walleij