ep93xx: Move SoC private bits to core

The PHYS_BASE defines in arch/arm/mach-ep93xx/include/mach-ep93xx-regs.h are only used in the SoC code. Move the defines to a local header file.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/adssphere.c                |    1 +
 arch/arm/mach-ep93xx/edb93xx.c                  |    2 +
 arch/arm/mach-ep93xx/gesbc9312.c                |    1 +
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |   34 ---------------
 arch/arm/mach-ep93xx/micro9.c                   |    1 +
 arch/arm/mach-ep93xx/simone.c                   |    2 +
 arch/arm/mach-ep93xx/snappercl15.c              |    2 +
 arch/arm/mach-ep93xx/soc.h                      |   50 +++++++++++++++++++++++
 arch/arm/mach-ep93xx/ts72xx.c                   |    2 +
 arch/arm/mach-ep93xx/vision_ep9307.c            |    2 +
 10 files changed, 63 insertions(+), 34 deletions(-)  create mode 100644 arch/arm/mach-ep93xx/soc.h

Quoted 18 lines · Ryan Mallon

The PHYS_BASE defines in arch/arm/mach-ep93xx/include/mach-ep93xx-regs.h are only used in the SoC code. Move the defines to a local header file.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/adssphere.c                |    1 +
 arch/arm/mach-ep93xx/edb93xx.c                  |    2 +
 arch/arm/mach-ep93xx/gesbc9312.c                |    1 +
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |   34 ---------------
 arch/arm/mach-ep93xx/micro9.c                   |    1 +
 arch/arm/mach-ep93xx/simone.c                   |    2 +
 arch/arm/mach-ep93xx/snappercl15.c              |    2 +
 arch/arm/mach-ep93xx/soc.h                      |   50 +++++++++++++++++++++++
 arch/arm/mach-ep93xx/ts72xx.c                   |    2 +
 arch/arm/mach-ep93xx/vision_ep9307.c            |    2 +
 10 files changed, 63 insertions(+), 34 deletions(-)  create mode 100644 arch/arm/mach-ep93xx/soc.h

<snip>

arch/arm/mach-ep93xx/edb93xx.c line 46

Quoted 9 lines

arch/arm/mach-ep93xx/edb93xx.c +2 -0

@@ -42,6 +42,8 @@
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 
+#include "soc.h"
+

Nitpick... Unnecessary whitespace...

<snip>

Quoted 7 lines

arch/arm/mach-ep93xx/ts72xx.c +0 -0

@@ -27,6 +27,8 @@
 #include <asm/mach/map.h>
 #include <asm/mach/arch.h>
 

arch/arm/mach-ep93xx/ts72xx.c line 31

Quoted 2 lines

arch/arm/mach-ep93xx/ts72xx.c +2 -0

+#include "soc.h"
+

Nitpick... Unnecessary whitespace...

Other than that... Looks ok.

Acked-by H Hartley Sweeten <[email protected]>

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |    8 --------
 arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h |   10 ++++++++++
 2 files changed, 10 insertions(+), 8 deletions(-)

Move the AHB/APB peripheral defines to local SoC header since they are only needed by the core SoC code. The UART defines are not
moved because they are used by the mach/uncompress.h header.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c                     |    1 +
 arch/arm/mach-ep93xx/dma.c                      |    2 +
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |   57 +----------------------
 arch/arm/mach-ep93xx/soc.h                      |   54 +++++++++++++++++++++
 4 files changed, 58 insertions(+), 56 deletions(-)

Move the pinmux setting of the EP93xx GPIOs to the core code. This removes the need for the GPIO driver to have access to the system controller registers.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Grant Likely <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c |    7 +++++++
 drivers/gpio/gpio-ep93xx.c  |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

arch/arm/mach-ep93xx/core.c +0 -0

index dcb10c2..978c5a6 100644

arch/arm/mach-ep93xx/core.c +0 -0

 	/* Get the GPIO working early, other devices need it */
 	platform_device_register(&ep93xx_gpio_device);
 

drivers/gpio/gpio-ep93xx.c +0 -7

index 1c0fc37..4ca5642 100644
@@ -378,13 +378,6 @@ static int __devinit ep93xx_gpio_probe(struct platform_device *pdev)
 	}
 	ep93xx_gpio->mmio_base = mmio;
 
-	/* Default all ports to GPIO */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
-			       EP93XX_SYSCON_DEVCFG_GONK |
-			       EP93XX_SYSCON_DEVCFG_EONIDE |
-			       EP93XX_SYSCON_DEVCFG_GONIDE |
-			       EP93XX_SYSCON_DEVCFG_HONIDE);
-
 	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++) {
 		struct bgpio_chip *bgc = &ep93xx_gpio->bgc[i];
 		struct ep93xx_gpio_bank *bank = &ep93xx_gpio_banks[i];

Signature · 1 line

1.7.0.4

Quoted 21 lines · Ryan Mallon

Move the pinmux setting of the EP93xx GPIOs to the core code. This removes the need for the GPIO driver to have access to the system controller registers.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Grant Likely <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c |    7 +++++++
 drivers/gpio/gpio-ep93xx.c  |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

arch/arm/mach-ep93xx/core.c +0 -0

index dcb10c2..978c5a6 100644
@@ -896,6 +896,13 @@ void __init ep93xx_init_devices(void)
 	/* Disallow access to MaverickCrunch initially */
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_CPENA);
 

arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 905

Quoted 7 lines

arch/arm/mach-ep93xx/core.c +7 -0

+	/* Default all ports to GPIO */
+	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
+			       EP93XX_SYSCON_DEVCFG_GONK |
+			       EP93XX_SYSCON_DEVCFG_EONIDE |
+			       EP93XX_SYSCON_DEVCFG_GONIDE |
+			       EP93XX_SYSCON_DEVCFG_HONIDE);
+

Doesn't this now change the behaviour? Now you unconditionally mux the pins whereas before they were only muxed if we had GPIO driver enabled.

Quoted 23 lines

arch/arm/mach-ep93xx/core.c +0 -0

 	/* Get the GPIO working early, other devices need it */
 	platform_device_register(&ep93xx_gpio_device);
 

drivers/gpio/gpio-ep93xx.c +0 -7

index 1c0fc37..4ca5642 100644
@@ -378,13 +378,6 @@ static int __devinit ep93xx_gpio_probe(struct platform_device *pdev)
 	}
 	ep93xx_gpio->mmio_base = mmio;
 
-	/* Default all ports to GPIO */
-	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
-			       EP93XX_SYSCON_DEVCFG_GONK |
-			       EP93XX_SYSCON_DEVCFG_EONIDE |
-			       EP93XX_SYSCON_DEVCFG_GONIDE |
-			       EP93XX_SYSCON_DEVCFG_HONIDE);
-
 	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++) {
 		struct bgpio_chip *bgc = &ep93xx_gpio->bgc[i];
 		struct ep93xx_gpio_bank *bank = &ep93xx_gpio_banks[i];

Signature · 1 line

1.7.0.4

Quoted 22 lines · Mika Westerberg

Quoted 21 lines · Ryan Mallon

Move the pinmux setting of the EP93xx GPIOs to the core code. This removes the need for the GPIO driver to have access to the system controller registers.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Grant Likely <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c |    7 +++++++
 drivers/gpio/gpio-ep93xx.c  |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

arch/arm/mach-ep93xx/core.c +0 -0

index dcb10c2..978c5a6 100644
@@ -896,6 +896,13 @@ void __init ep93xx_init_devices(void)
 	/* Disallow access to MaverickCrunch initially */
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_CPENA);
 

Quoted 9 lines

arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 905

Quoted 7 lines

arch/arm/mach-ep93xx/core.c +7 -0

+	/* Default all ports to GPIO */
+	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
+			       EP93XX_SYSCON_DEVCFG_GONK |
+			       EP93XX_SYSCON_DEVCFG_EONIDE |
+			       EP93XX_SYSCON_DEVCFG_GONIDE |
+			       EP93XX_SYSCON_DEVCFG_HONIDE);
+

Doesn't this now change the behaviour? Now you unconditionally mux the pins whereas before they were only muxed if we had GPIO driver enabled.

True, but I think the previous behaviour was broken anyway. If the gpio driver was installed after any driver which has muxed gpio pins for other purposes then it will override those settings. All of the ep93xx drivers will mux the pins they need at probe time. IIRC, before the gpio driver was moved out of arch/arm/mach-ep93xx we muxed all the pins unconditionally anyway.

Quoted 22 lines · Mika Westerberg

Quoted 21 lines · Ryan Mallon

Move the pinmux setting of the EP93xx GPIOs to the core code. This removes the need for the GPIO driver to have access to the system controller registers.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Grant Likely <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c |    7 +++++++
 drivers/gpio/gpio-ep93xx.c  |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

(quoted diff) +0 -0

b/arch/arm/mach-ep93xx/core.c index dcb10c2..978c5a6 100644

arch/arm/mach-ep93xx/core.c +0 -0

@@ -896,6 +896,13 @@ void __init ep93xx_init_devices(void)
 	/* Disallow access to MaverickCrunch initially */
 	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_CPENA);
 

Quoted 9 lines

arch/arm/mach-ep93xx/core.c void __init ep93xx_init_devices(void) · line 905

Quoted 7 lines

arch/arm/mach-ep93xx/core.c +7 -0

+	/* Default all ports to GPIO */
+	ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_KEYS |
+			       EP93XX_SYSCON_DEVCFG_GONK |
+			       EP93XX_SYSCON_DEVCFG_EONIDE |
+			       EP93XX_SYSCON_DEVCFG_GONIDE |
+			       EP93XX_SYSCON_DEVCFG_HONIDE);
+

Doesn't this now change the behaviour? Now you unconditionally mux the pins whereas before they were only muxed if we had GPIO driver enabled.

True, but I think the previous behaviour was broken anyway. If the gpio driver was installed after any driver which has muxed gpio pins for other purposes then it will override those settings. All of the ep93xx drivers will mux the pins they need at probe time. IIRC, before the gpio driver was moved out of arch/arm/mach-ep93xx we muxed all the pins unconditionally anyway.

I can update the changelog to reflect the changed behaviour, but I don't think it will cause problems for anyone. Will see if Hartley has any comments.

Move the arch_reset function from include/mach/system.h to core.c. This removes the need for the EP93XX_SYSCON_DEVCFG defines to
be exported in the include/mach headers.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c                |   18 ++++++++++++++++++
 arch/arm/mach-ep93xx/include/mach/system.h |   14 +-------------
 2 files changed, 19 insertions(+), 13 deletions(-)

Both the Snapper CL15 and EDB93xx audio drivers set the same audio configuration in ep93xx_i2s_acquire. Remove the arguments to ep93xx_i2s_acquire so that the audio drivers no longer need the EP93XX_SYSCON defines exported.

Quoted 4 lines · Ryan Mallon

Both the Snapper CL15 and EDB93xx audio drivers set the same audio configuration in ep93xx_i2s_acquire. Remove the arguments to ep93xx_i2s_acquire so that the audio drivers no longer need the EP93XX_SYSCON defines exported.

What if there are other boards out there which use a different configuration?

We can either add a second function, like ep93xx_i2s_acquire_on_ssp, or we can create a set of defines for the various options needed. I just want to avoid exposing the system controller registers/flags directly. The ep93xx chip is now obsolete, so I don't expect new boards to appear, and these two boards have been the only ones with audio support for a couple of years now, so this seemed to be the best solution.

Both the Snapper CL15 and EDB93xx audio drivers set the same audio configuration in ep93xx_i2s_acquire. Remove the arguments
to ep93xx_i2s_acquire so that the audio drivers no longer need
the EP93XX_SYSCON defines exported.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c                  |   19 ++++---------------
 arch/arm/mach-ep93xx/include/mach/platform.h |    2 +-
 sound/soc/ep93xx/edb93xx.c                   |    4 +---
 sound/soc/ep93xx/snappercl15.c               |    4 +---
 4 files changed, 7 insertions(+), 22 deletions(-)

arch/arm/mach-ep93xx/core.c +3 -14

@@ -836,23 +836,12 @@ void __init ep93xx_register_i2s(void)
 #define EP93XX_I2SCLKDIV_MASK		(EP93XX_SYSCON_I2SCLKDIV_ORIDE | \
 					 EP93XX_SYSCON_I2SCLKDIV_SPOL)
 
-int ep93xx_i2s_acquire(unsigned i2s_pins, unsigned i2s_config)
+int ep93xx_i2s_acquire(void)
 {
 	unsigned val;
 
-	/* Sanity check */
-	if (i2s_pins & ~EP93XX_SYSCON_DEVCFG_I2S_MASK)
-		return -EINVAL;
-	if (i2s_config & ~EP93XX_I2SCLKDIV_MASK)
-		return -EINVAL;
-
-	/* Must have only one of I2SONSSP/I2SONAC97 set */
-	if ((i2s_pins & EP93XX_SYSCON_DEVCFG_I2SONSSP) ==
-	    (i2s_pins & EP93XX_SYSCON_DEVCFG_I2SONAC97))
-		return -EINVAL;
-
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2S_MASK);
-	ep93xx_devcfg_set_bits(i2s_pins);
+	ep93xx_devcfg_set_clear(EP93XX_SYSCON_DEVCFG_I2SONAC97,
+			EP93XX_SYSCON_DEVCFG_I2S_MASK);
 
 	/*
 	 * This is potentially racy with the clock api for i2s_mclk, sclk and @@ -862,7 +851,7 @@ int ep93xx_i2s_acquire(unsigned i2s_pins, unsigned i2s_config)

arch/arm/mach-ep93xx/core.c +1 -1

 	 */
 	val = __raw_readl(EP93XX_SYSCON_I2SCLKDIV);
 	val &= ~EP93XX_I2SCLKDIV_MASK;
-	val |= i2s_config;
+	val |= EP93XX_SYSCON_I2SCLKDIV_ORIDE | EP93XX_SYSCON_I2SCLKDIV_SPOL;

	ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);

sound/soc/ep93xx/edb93xx.c +1 -3

@@ -94,9 +94,7 @@ static int __devinit edb93xx_probe(struct platform_device *pdev)
 	struct snd_soc_card *card = &snd_soc_edb93xx;
 	int ret;
 
-	ret = ep93xx_i2s_acquire(EP93XX_SYSCON_DEVCFG_I2SONAC97,
-				 EP93XX_SYSCON_I2SCLKDIV_ORIDE |
-				 EP93XX_SYSCON_I2SCLKDIV_SPOL);
+	ret = ep93xx_i2s_acquire();
 	if (ret)
 		return ret;
 

Quoted 45 lines · Ryan Mallon

Both the Snapper CL15 and EDB93xx audio drivers set the same audio configuration in ep93xx_i2s_acquire. Remove the arguments
to ep93xx_i2s_acquire so that the audio drivers no longer need
the EP93XX_SYSCON defines exported.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Liam Girdwood <[email protected]>

Cc Mark Brown <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/core.c                  |   19 ++++---------------
 arch/arm/mach-ep93xx/include/mach/platform.h |    2 +-
 sound/soc/ep93xx/edb93xx.c                   |    4 +---
 sound/soc/ep93xx/snappercl15.c               |    4 +---
 4 files changed, 7 insertions(+), 22 deletions(-)

arch/arm/mach-ep93xx/core.c +3 -14

@@ -836,23 +836,12 @@ void __init ep93xx_register_i2s(void)
 #define EP93XX_I2SCLKDIV_MASK		(EP93XX_SYSCON_I2SCLKDIV_ORIDE | \
 					 EP93XX_SYSCON_I2SCLKDIV_SPOL)
 
-int ep93xx_i2s_acquire(unsigned i2s_pins, unsigned i2s_config)
+int ep93xx_i2s_acquire(void)
 {
 	unsigned val;
 
-	/* Sanity check */
-	if (i2s_pins & ~EP93XX_SYSCON_DEVCFG_I2S_MASK)
-		return -EINVAL;
-	if (i2s_config & ~EP93XX_I2SCLKDIV_MASK)
-		return -EINVAL;
-
-	/* Must have only one of I2SONSSP/I2SONAC97 set */
-	if ((i2s_pins & EP93XX_SYSCON_DEVCFG_I2SONSSP) ==
-	    (i2s_pins & EP93XX_SYSCON_DEVCFG_I2SONAC97))
-		return -EINVAL;
-
-	ep93xx_devcfg_clear_bits(EP93XX_SYSCON_DEVCFG_I2S_MASK);
-	ep93xx_devcfg_set_bits(i2s_pins);
+	ep93xx_devcfg_set_clear(EP93XX_SYSCON_DEVCFG_I2SONAC97,
+			EP93XX_SYSCON_DEVCFG_I2S_MASK);
 

Quoted 8 lines

arch/arm/mach-ep93xx/core.c +0 -0

 	/*
 	 * This is potentially racy with the clock api for i2s_mclk, sclk and @@ -862,7 +851,7 @@ int ep93xx_i2s_acquire(unsigned i2s_pins, unsigned i2s_config)

arch/arm/mach-ep93xx/core.c +1 -1

 	 */
 	val = __raw_readl(EP93XX_SYSCON_I2SCLKDIV);
 	val &= ~EP93XX_I2SCLKDIV_MASK;
-	val |= i2s_config;
+	val |= EP93XX_SYSCON_I2SCLKDIV_ORIDE | EP93XX_SYSCON_I2SCLKDIV_SPOL;

	ep93xx_syscon_swlocked_write(val, EP93XX_SYSCON_I2SCLKDIV);

Nitpick... You clear then set the same bits here.

Quoted 21 lines

sound/soc/ep93xx/edb93xx.c +1 -3

@@ -94,9 +94,7 @@ static int __devinit edb93xx_probe(struct platform_device *pdev)
 	struct snd_soc_card *card = &snd_soc_edb93xx;
 	int ret;
 
-	ret = ep93xx_i2s_acquire(EP93XX_SYSCON_DEVCFG_I2SONAC97,
-				 EP93XX_SYSCON_I2SCLKDIV_ORIDE |
-				 EP93XX_SYSCON_I2SCLKDIV_SPOL);
+	ret = ep93xx_i2s_acquire();
 	if (ret)
 		return ret;
 

sound/soc/ep93xx/snappercl15.c +0 -0

@@ -110,9 +110,7 @@ static int __devinit snappercl15_probe(struct platform_device *pdev)
 	struct snd_soc_card *card = &snd_soc_snappercl15;
 	int ret;
 

sound/soc/ep93xx/snappercl15.c static int __devinit snappercl15_probe(struct platform_device *pdev) · line 116

Quoted 7 lines

sound/soc/ep93xx/snappercl15.c +1 -3

-	ret = ep93xx_i2s_acquire(EP93XX_SYSCON_DEVCFG_I2SONAC97,
-				 EP93XX_SYSCON_I2SCLKDIV_ORIDE |
-				 EP93XX_SYSCON_I2SCLKDIV_SPOL);
+	ret = ep93xx_i2s_acquire();
 	if (ret)
 		return ret;
 

Maybe the ep93xx_i2s_acquire() function should be renamed to something like ep93xx_i2s_ac97_acquire() so that the i2s configuration is understood. This should address Mark Brown's issue.

Regardless, looks good.

Acked-by H Hartley Sweeten <[email protected]>

Quoted 3 lines · H Hartley Sweeten

Maybe the ep93xx_i2s_acquire() function should be renamed to something like ep93xx_i2s_ac97_acquire() so that the i2s configuration is understood. This should address Mark Brown's issue.

Mark has already picked this patch up. I can probably modify it and get him to grab the new one, but the reason I didn't change the name is because there have been only two users of this function for a couple of years now, and the ep93xx is obsolete, so its unlikely that we are going to need a new function anyway.

The syscon access functions are no longer used outside of the core EP93xx code. Move their definitions into the SoC code.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/clock.c                 |    1 +
 arch/arm/mach-ep93xx/core.c                  |    2 --
 arch/arm/mach-ep93xx/include/mach/platform.h |   14 --------------
 arch/arm/mach-ep93xx/soc.h                   |   14 ++++++++++++++
 4 files changed, 15 insertions(+), 16 deletions(-)

arch/arm/mach-ep93xx/soc.h +6 -0

@@ -13,6 +13,20 @@
 #ifndef _EP93XX_SOC_H
 #define _EP93XX_SOC_H
 
+/* EP93xx System Controller software locked register write */ void 
+ep93xx_syscon_swlocked_write(unsigned int val, void __iomem *reg); void 
+ep93xx_devcfg_set_clear(unsigned int set_bits, unsigned int 
+clear_bits);
+
+static inline void ep93xx_devcfg_set_bits(unsigned int bits) {

Quoted 6 lines · Ryan Mallon

The syscon access functions are no longer used outside of the core EP93xx code. Move their definitions into the SoC code.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

Quoted 6 lines

---
 arch/arm/mach-ep93xx/clock.c                 |    1 +
 arch/arm/mach-ep93xx/core.c                  |    2 --
 arch/arm/mach-ep93xx/include/mach/platform.h |   14 --------------
 arch/arm/mach-ep93xx/soc.h                   |   14 ++++++++++++++
 4 files changed, 15 insertions(+), 16 deletions(-)

It looks like part of this patch is word wrapped, or it could just be me...

<snip>

Quoted 13 lines

arch/arm/mach-ep93xx/soc.h +6 -0

@@ -13,6 +13,20 @@
 #ifndef _EP93XX_SOC_H
 #define _EP93XX_SOC_H
 
+/* EP93xx System Controller software locked register write */ void 
+ep93xx_syscon_swlocked_write(unsigned int val, void __iomem *reg); void 
+ep93xx_devcfg_set_clear(unsigned int set_bits, unsigned int 
+clear_bits);
+
+static inline void ep93xx_devcfg_set_bits(unsigned int bits) {

arch/arm/mach-ep93xx/soc.h line 31

Quoted 10 lines

arch/arm/mach-ep93xx/soc.h +7 -0

+	ep93xx_devcfg_set_clear(bits, 0x00);
+}
+
+static inline void ep93xx_devcfg_clear_bits(unsigned int bits) {
+	ep93xx_devcfg_set_clear(0x00, bits);
+}
+
 /*
  * EP93xx Physical Memory Map:
  *

I would prefer these functions to be after all the #define's. But, your call...

Acked-by H Hartley Sweeten <[email protected]>

The EP93xx watchdog driver is the only user of this define, move it there.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Wim Van Sebroeck <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |    2 --
 drivers/watchdog/ep93xx_wdt.c                   |    1 +
 2 files changed, 1 insertions(+), 2 deletions(-)

arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h +0 -2

index 387f745..225790d 100644
@@ -127,7 +127,5 @@
 #define EP93XX_SYSCON_SYSCFG_LCSN1	(1<<0)
 #define EP93XX_SYSCON_SWLOCK		EP93XX_SYSCON_REG(0xc0)
 
-#define EP93XX_WATCHDOG_BASE		EP93XX_APB_IOMEM(0x00140000)
-
 
 #endif

Quoted 27 lines · Ryan Mallon

The EP93xx watchdog driver is the only user of this define, move it there.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Wim Van Sebroeck <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |    2 --
 drivers/watchdog/ep93xx_wdt.c                   |    1 +
 2 files changed, 1 insertions(+), 2 deletions(-)

arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h +0 -2

index 387f745..225790d 100644
@@ -127,7 +127,5 @@
 #define EP93XX_SYSCON_SYSCFG_LCSN1	(1<<0)
 #define EP93XX_SYSCON_SWLOCK		EP93XX_SYSCON_REG(0xc0)
 
-#define EP93XX_WATCHDOG_BASE		EP93XX_APB_IOMEM(0x00140000)
-
 
 #endif

drivers/watchdog/ep93xx_wdt.c static unsigned long boot_status; · line 55

Quoted 7 lines

drivers/watchdog/ep93xx_wdt.c +1 -0

@@ -49,6 +49,7 @@ static unsigned long boot_status;
 #define WDT_IN_USE		0
 #define WDT_OK_TO_CLOSE		1
 
+#define EP93XX_WATCHDOG_BASE	EP93XX_APB_IOMEM(0x00140000)
 #define EP93XX_WDT_REG(x)	(EP93XX_WATCHDOG_BASE + (x))
 #define EP93XX_WDT_WATCHDOG	EP93XX_WDT_REG(0x00)
 #define EP93XX_WDT_WDSTATUS	EP93XX_WDT_REG(0x04)

Ryan,

I have a patch to convert the watchdog driver to a platform device and use the WatchDog Timer Driver Core. This patch will end up moving the base define to soc.h instead. But, for now, this looks good.

Quoted 3 lines · H Hartley Sweeten

I have a patch to convert the watchdog driver to a platform device and use the WatchDog Timer Driver Core. This patch will end up moving the base define to soc.h instead. But, for now, this looks good.

Is your patch ready to go? I'm in no rush to get these patches in, and I can add it to this series if you like, which would get rid of the need for this patch and would also mean that we don't need the include of <mach/ep93xx-regs.h> in the watchdog driver in the final patch.

The crunch code in arch/arm/kernel is specific to the EP93xx. Move it to the mach-ep93xx directory. This removes the need for the EP93XX_SYSCON defines to be exported to arch/arm/kernel.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Russell King <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/kernel/Makefile                       |    3 ---
 arch/arm/mach-ep93xx/Makefile                  |    3 +++
 arch/arm/{kernel => mach-ep93xx}/crunch-bits.S |    0
 arch/arm/{kernel => mach-ep93xx}/crunch.c      |    2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)  rename arch/arm/{kernel => mach-ep93xx}/crunch-bits.S (100%)  rename arch/arm/{kernel => mach-ep93xx}/crunch.c (99%)

arch/arm/kernel/Makefile +0 -3

@@ -62,9 +62,6 @@ obj-$(CONFIG_SWP_EMULATE)	+= swp_emulate.o
 CFLAGS_swp_emulate.o		:= -Wa,-march=armv7-a
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 
-obj-$(CONFIG_CRUNCH)		+= crunch.o crunch-bits.o
-AFLAGS_crunch-bits.o		:= -Wa,-mcpu=ep9312
-
 obj-$(CONFIG_CPU_XSCALE)	+= xscale-cp0.o
 obj-$(CONFIG_CPU_XSC3)		+= xscale-cp0.o
 obj-$(CONFIG_CPU_MOHAWK)	+= xscale-cp0.o

arch/arm/mach-ep93xx/Makefile +0 -0

 obj-$(CONFIG_MACH_ADSSPHERE)	+= adssphere.o
 obj-$(CONFIG_MACH_EDB93XX)	+= edb93xx.o
 obj-$(CONFIG_MACH_GESBC9312)	+= gesbc9312.o

arch/arm/mach-ep93xx/crunch-bits.S renamed from arch/arm/kernel/crunch-bits.S rename to arch/arm/mach-ep93xx/crunch-bits.S +0 -0

similarity index 100%
rename from arch/arm/kernel/crunch-bits.S rename to arch/arm/mach-ep93xx/crunch-bits.S

Quoted 35 lines · Ryan Mallon

The crunch code in arch/arm/kernel is specific to the EP93xx. Move it to the mach-ep93xx directory. This removes the need for the EP93XX_SYSCON defines to be exported to arch/arm/kernel.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Russell King <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/kernel/Makefile                       |    3 ---
 arch/arm/mach-ep93xx/Makefile                  |    3 +++
 arch/arm/{kernel => mach-ep93xx}/crunch-bits.S |    0
 arch/arm/{kernel => mach-ep93xx}/crunch.c      |    2 ++
 4 files changed, 5 insertions(+), 3 deletions(-)  rename arch/arm/{kernel => mach-ep93xx}/crunch-bits.S (100%)  rename arch/arm/{kernel => mach-ep93xx}/crunch.c (99%)

arch/arm/kernel/Makefile +0 -3

@@ -62,9 +62,6 @@ obj-$(CONFIG_SWP_EMULATE)	+= swp_emulate.o
 CFLAGS_swp_emulate.o		:= -Wa,-march=armv7-a
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 
-obj-$(CONFIG_CRUNCH)		+= crunch.o crunch-bits.o
-AFLAGS_crunch-bits.o		:= -Wa,-mcpu=ep9312
-
 obj-$(CONFIG_CPU_XSCALE)	+= xscale-cp0.o
 obj-$(CONFIG_CPU_XSC3)		+= xscale-cp0.o
 obj-$(CONFIG_CPU_MOHAWK)	+= xscale-cp0.o

arch/arm/mach-ep93xx/Makefile +0 -0

@@ -8,6 +8,9 @@ obj-			:=
 
 obj-$(CONFIG_EP93XX_DMA)	+= dma.o
 

arch/arm/mach-ep93xx/Makefile obj- := · line 13

Quoted 3 lines

arch/arm/mach-ep93xx/Makefile +3 -0

+obj-$(CONFIG_CRUNCH)		+= crunch.o crunch-bits.o
+AFLAGS_crunch-bits.o		:= -Wa,-mcpu=ep9312
+

Nitpick... Care to align the core stuff above this while you are touching this file?

Also, does the compile order matter here? Should the crunch stuff come before the dma driver?

Quoted 13 lines

arch/arm/mach-ep93xx/Makefile +0 -0

 obj-$(CONFIG_MACH_ADSSPHERE)	+= adssphere.o
 obj-$(CONFIG_MACH_EDB93XX)	+= edb93xx.o
 obj-$(CONFIG_MACH_GESBC9312)	+= gesbc9312.o

arch/arm/mach-ep93xx/crunch-bits.S renamed from arch/arm/kernel/crunch-bits.S rename to arch/arm/mach-ep93xx/crunch-bits.S +0 -0

similarity index 100%
rename from arch/arm/kernel/crunch-bits.S rename to arch/arm/mach-ep93xx/crunch-bits.S

arch/arm/mach-ep93xx/crunch.c renamed from arch/arm/kernel/crunch.c +0 -0

@@ -19,6 +19,8 @@
 #include <mach/ep93xx-regs.h>
 #include <asm/thread_notify.h>
 

arch/arm/mach-ep93xx/crunch.c line 26

Quoted 5 lines

arch/arm/mach-ep93xx/crunch.c renamed from arch/arm/kernel/crunch.c +2 -0

+#include "soc.h"
+
 struct crunch_state *crunch_owner;
 
 void crunch_task_release(struct thread_info *thread)

While you are moving this, is there anything you can do about the sparse warning regarding the crunch_owner variable?

Regardless, looks good.

Acked-by H Hartley Sweeten <[email protected]>

I was hoping that Russell would make some comment here about whether moving this file out of arch/arm/kernel is okay to do.

I assume it had
some reason to be there in the first place. I have only done build testing so far since I don't have any ep93xx hardware to test on, and have never actually used crunch anyway.

The EP93XX_SYSCON defines are now no longer needed outside of the EP93xx SoC core code, so they can be moved to a private header.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |   90 -----------------------
 arch/arm/mach-ep93xx/soc.h                      |   90 +++++++++++++++++++++++
 2 files changed, 90 insertions(+), 90 deletions(-)

Remove the include of ep93xx-regs.h from files which no longer need it, notably include/mach/hardware.h. An explict include of ep93xx-regs.h
is needed in the EP93xx watchdog driver for the EP93XX_APB_IOMEM macro.

Cc Hartley Sweeten <[email protected]>

Cc Mika Westerberg <[email protected]>

Signed-off-by Ryan Mallon <[email protected]>

---
 arch/arm/mach-ep93xx/crunch.c                |    2 +-
 arch/arm/mach-ep93xx/include/mach/hardware.h |    1 -
 arch/arm/mach-ep93xx/soc.h                   |    2 ++
 drivers/watchdog/ep93xx_wdt.c                |    1 +
 4 files changed, 4 insertions(+), 2 deletions(-)

This patch series is an effort to move the ep93xx SoC specific code out of drivers and include/mach into arch/arm/mach-ep93xx. This work involves the following changes:

- Create a new private header called soc.h to replace most of mach/include/ep93xx-regs.h
- Move the Maverick crunch code from arch/arm/kernel to mach-ep93xx
- Move all system controller access to the ep93xx core code

There are a small handful of defines left in ep93xx-regs.h which are used by both the ep93xx core and include/mach files (uncompress.h, debug-macro.S, etc).

Ryan Mallon (11):
ep93xx: Move PHYS_BASE defines to local SoC header file
ep93xx: Move GPIO defines to gpio-ep93xx.h
ep93xx: Move peripheral defines to local SoC header
ep93xx: Configure GPIO ports in core code
ep93xx: Move arch_reset to core.c
ep93xx: Don't use system controller defines in audio drivers
ep93xx: Make syscon access functions private to SoC
ep93xx: Move EP93XX_WATCHDOG_BASE define to driver
ep93xx: Move crunch code to mach-ep93xx directory
ep93xx: Move EP93XX_SYSCON defines to SoC private header
ep93xx: Remove unnecessary includes of ep93xx-regs.h

 arch/arm/kernel/Makefile                        |    3 -
 arch/arm/mach-ep93xx/Makefile                   |    3 +
 arch/arm/mach-ep93xx/adssphere.c                |    1 +
 arch/arm/mach-ep93xx/clock.c                    |    1 +
 arch/arm/mach-ep93xx/core.c                     |   47 ++++--
 arch/arm/{kernel => mach-ep93xx}/crunch-bits.S  |    0
 arch/arm/{kernel => mach-ep93xx}/crunch.c       |    4 +-
 arch/arm/mach-ep93xx/dma.c                      |    2 +
 arch/arm/mach-ep93xx/edb93xx.c                  |    2 +
 arch/arm/mach-ep93xx/gesbc9312.c                |    1 +
 arch/arm/mach-ep93xx/include/mach/ep93xx-regs.h |  191 +--------------------
 arch/arm/mach-ep93xx/include/mach/gpio-ep93xx.h |   10 +
 arch/arm/mach-ep93xx/include/mach/hardware.h    |    1 -
 arch/arm/mach-ep93xx/include/mach/platform.h    |   16 +--
 arch/arm/mach-ep93xx/include/mach/system.h      |   14 +--
 arch/arm/mach-ep93xx/micro9.c                   |    1 +
 arch/arm/mach-ep93xx/simone.c                   |    2 +
 arch/arm/mach-ep93xx/snappercl15.c              |    2 +
 arch/arm/mach-ep93xx/soc.h                      |  210 +++++++++++++++++++++++
 arch/arm/mach-ep93xx/ts72xx.c                   |    2 +
 arch/arm/mach-ep93xx/vision_ep9307.c            |    2 +
 drivers/gpio/gpio-ep93xx.c                      |    7 -
 drivers/watchdog/ep93xx_wdt.c                   |    2 +
 sound/soc/ep93xx/edb93xx.c                      |    4 +-
 sound/soc/ep93xx/snappercl15.c                  |    4 +-
 25 files changed, 279 insertions(+), 253 deletions(-)
 rename arch/arm/{kernel => mach-ep93xx}/crunch-bits.S (100%)
 rename arch/arm/{kernel => mach-ep93xx}/crunch.c (98%)
 create mode 100644 arch/arm/mach-ep93xx/soc.h

This patch series is an effort to move the ep93xx SoC specific code out of drivers and include/mach into arch/arm/mach-ep93xx. This work involves the following changes:

This patch series is an effort to move the ep93xx SoC specific code out of drivers and include/mach into arch/arm/mach-ep93xx. This work involves the following changes:

- Create a new private header called soc.h to replace most of mach/include/ep93xx-regs.h
- Move the Maverick crunch code from arch/arm/kernel to mach-ep93xx
- Move all system controller access to the ep93xx core code

There are a small handful of defines left in ep93xx-regs.h which are used by both the ep93xx core and include/mach files (uncompress.h, debug-macro.S, etc).

Quoted 4 lines · Ryan Mallon

This patch series is an effort to move the ep93xx SoC specific code out of drivers and include/mach into arch/arm/mach-ep93xx. This work involves the following changes:

Quoted 8 lines

- Create a new private header called soc.h to replace most of mach/include/ep93xx-regs.h
- Move the Maverick crunch code from arch/arm/kernel to mach-ep93xx
- Move all system controller access to the ep93xx core code

There are a small handful of defines left in ep93xx-regs.h which are used by both the ep93xx core and include/mach files (uncompress.h, debug-macro.S, etc).

Sorry for the delay. I went through the series and it looks good to me. You can add

Reviewed-by Mika Westerberg <[email protected]>

for all the patches, if you like.