pinctrl: enable pxa pinmux

In reply to Stephen Warren

Haojian Zhuang wrote at Friday, November 25, 2011 4:09 PM:
...

Quoted 1 line

drivers/pinctrl/Kconfig +0 -0

...

drivers/pinctrl/Kconfig config DEBUG_PINCTRL · line 27

Quoted 5 lines

drivers/pinctrl/Kconfig +5 -0

+config PINMUX_PXA3xx
+	bool "PXA pinmux driver"
+	default y if ARCH_PXA || ARCH_MMP
+	select PINMUX
+

I thought the plan here was for e.g. ARCH_PXA to "select PINMUX_PXA3xxx", since it's presumably mandatory?

Signature · 1 line

nvpublic

Eric Miao

2011-11-29 02:31 UTC #10 lore raw

PATCH 1/7 pinctrl: enable pxa3xx pinmux

Quoted 4 lines · Stephen Warren

Haojian Zhuang wrote at Friday, November 25, 2011 4:09 PM:
...
Quoted 1 line
drivers/pinctrl/Kconfig +0 -0
...

Quoted 8 lines

drivers/pinctrl/Kconfig
Quoted 5 lines
drivers/pinctrl/Kconfig +5 -0
+config PINMUX_PXA3xx
+ ? ? bool "PXA pinmux driver"
+ ? ? default y if ARCH_PXA || ARCH_MMP
+ ? ? select PINMUX
+
I thought the plan here was for e.g. ARCH_PXA to "select PINMUX_PXA3xxx", since it's presumably mandatory?

Can we pass the SoC type by platform data, instead of calling cpu_is_*()?

Linus Walleij

2011-11-29 08:39 UTC #11 lore raw

PATCH 1/7 pinctrl: enable pxa3xx pinmux

Quoted 7 lines · Stephen Warren

Haojian Zhuang wrote at Friday, November 25, 2011 4:09 PM:
...
Quoted 1 line
drivers/pinctrl/Kconfig +0 -0
...
Quoted 3 lines
+config PINMUX_PXA3xx
+ ? ? bool "PXA pinmux driver"
+ ? ? default y if ARCH_PXA || ARCH_MMP

So drop that line.

The rest is correct.

Quoted 5 lines

Quoted 2 lines
+ ? ? select PINMUX
+
I thought the plan here was for e.g. ARCH_PXA to "select PINMUX_PXA3xxx", since it's presumably mandatory?

Yes that is correct.

Haojian can you alter the patch so that arch/arm/mach*/Kconfig does

select PINCTRL
select PINMUX_PXA3XX

(Both are needed.)

And

Yours,
Linus Walleij

Haojian Zhuang

maintainer

2011-11-29 08:56 UTC #12 lore raw

PATCH 1/7 pinctrl: enable pxa3xx pinmux

Quoted 19 lines · Linus Walleij

Quoted 7 lines · Stephen Warren
Haojian Zhuang wrote at Friday, November 25, 2011 4:09 PM:
...
Quoted 1 line
drivers/pinctrl/Kconfig +0 -0
...
Quoted 3 lines
+config PINMUX_PXA3xx
+ ? ? bool "PXA pinmux driver"
+ ? ? default y if ARCH_PXA || ARCH_MMP
So drop that line.
The rest is correct.
Quoted 5 lines
Quoted 2 lines
+ ? ? select PINMUX
+
I thought the plan here was for e.g. ARCH_PXA to "select PINMUX_PXA3xxx", since it's presumably mandatory?

Quoted 9 lines

Yes that is correct.
Haojian can you alter the patch so that arch/arm/mach*/Kconfig does
select PINCTRL
select PINMUX_PXA3XX
(Both are needed.)

I'll change it.

Thanks
Haojian

Linus Walleij

2011-11-30 12:00 UTC #13 lore raw

PATCH 1/7 pinctrl: enable pxa3xx pinmux

Hi Haojian,

I thought about this last night:

Review

Quoted 3 lines · Haojian Zhuang

(quoted diff) +3 -0

+/* These 135 pins exists in PXA300, PXA310 & PXA320 */
+#define PXA3XX_COMMON_PINS() ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? PINCTRL_PIN(0, "GPIO0"), ? ? ? ?PINCTRL_PIN(1, "GPIO1"), ? ? ? ?\

(...)
+#ifdef CONFIG_ARCH_PXA
+static struct pinctrl_pin_desc pxa300_pads[] = {
+ PXA3XX_COMMON_PINS(),
+ PINCTRL_PIN(128, "GPIO0_2"), PINCTRL_PIN(129, "GPIO1_2"),

Defining a huge macro like PXA3XX_COMMON_PINS() and reusing all over the place doesn't scale.

The real fix is that I fix so that you can call pinmux_register_mappings() more than one time.

So it will look like this or similar:

pinmux_register_mappings(common_pins);
if (plat->is_pxa)
pinmux_register_mappings(pxa_pins);
if (plat->is_mmp)
pinmux_register_mappings(mmp_pins);

etc.

Since I'm anyway fixing up this function I'll see if I can simply fit this in as patch 2/2 and send out ASAP.

Yours,
Linus Walleij

Haojian Zhuang

author

2011-11-25 23:08 UTC #14 lore raw

PATCH 2/7 gpio: make pxa gpio depend on pinmux

From: Haojian Zhuang <[email protected]>

While gpio_request() is called, pinmux_request_gpio() would be called too. Then alternate function of pin can be changed to gpio directly.

---
 drivers/gpio/gpio-pxa.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

drivers/gpio/gpio-pxa.c +14 -1

index b2d3ee1..8808004 100644
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/syscore_ops.h>
 #include <linux/slab.h>

@@ -267,6 +268,16 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 				(value ? GPSR_OFFSET : GPCR_OFFSET));
 }
 
+static int pxa_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	return pinmux_request_gpio(chip->base + offset);
+}
+
+static void pxa_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	pinmux_free_gpio(chip->base + offset);
+}
+
 static int __devinit pxa_init_gpio_chip(int gpio_end)
 {
 	int i, gpio, nbanks = gpio_to_bank(gpio_end) + 1;

@@ -292,6 +303,8 @@ static int __devinit pxa_init_gpio_chip(int gpio_end)
 		c->get = pxa_gpio_get;
 		c->set = pxa_gpio_set;
 		c->to_irq = pxa_gpio_to_irq;
+		c->request = pxa_gpio_request;
+		c->free = pxa_gpio_free;
 
 		/* number of GPIOs on last bank may be less than 32 */
 		c->ngpio = (gpio + 31 > gpio_end) ? (gpio_end - gpio + 1) : 32;

@@ -612,4 +625,4 @@ static int __init pxa_gpio_sysinit(void)
 	register_syscore_ops(&pxa_gpio_syscore_ops);
 	return 0;
 }
-postcore_initcall(pxa_gpio_sysinit);
+postcore_initcall_sync(pxa_gpio_sysinit);

Signature · 1 line

1.7.0.4

Linus Walleij

2011-11-28 09:09 UTC #15 lore raw

PATCH 2/7 gpio: make pxa gpio depend on pinmux

Quoted 6 lines · Haojian Zhuang

From: Haojian Zhuang <[email protected]>
While gpio_request() is called, pinmux_request_gpio() would be called too. Then alternate function of pin can be changed to gpio directly.
Signed-off-by Haojian Zhuang <[email protected]>

Looks good,

Thanks,
Linus Walleij

Haojian Zhuang

author

2011-11-25 23:09 UTC #16 lore raw

PATCH 3/7 ARM: pxa: enable pinmux in pxa pwm

From: Haojian Zhuang <[email protected]>

Add pinmux operations in pxa pwm driver.

---
 arch/arm/plat-pxa/pwm.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

arch/arm/plat-pxa/pwm.c +14 -1

index ef32686..93c0986 100644
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/clk.h>

@@ -47,6 +48,7 @@ struct pwm_device {
 	struct list_head	node;
 	struct pwm_device	*secondary;
 	struct platform_device	*pdev;
+	struct pinmux		*pmx;
 
 	const char	*label;
 	struct clk	*clk;

@@ -187,10 +189,16 @@ static int __devinit pwm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	pwm->pmx = pinmux_get(&pdev->dev, NULL);
+	if (IS_ERR(pwm->pmx)) {
+		ret = PTR_ERR(pwm->pmx);
+		goto err_free;
+	}
+	pinmux_enable(pwm->pmx);
 	pwm->clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(pwm->clk)) {
 		ret = PTR_ERR(pwm->clk);
-		goto err_free;
+		goto err_free_pwm;
 	}
 	pwm->clk_enabled = 0;

@@ -245,6 +253,9 @@ err_free_mem:
 	release_mem_region(r->start, resource_size(r));
 err_free_clk:
 	clk_put(pwm->clk);
+err_free_pwm:
+	pinmux_disable(pwm->pmx);
+	pinmux_put(pwm->pmx);
 err_free:
 	kfree(pwm);
 	return ret;

@@ -275,6 +286,8 @@ static int __devexit pwm_remove(struct platform_device *pdev)
 	release_mem_region(r->start, resource_size(r));
 
 	clk_put(pwm->clk);
+	pinmux_disable(pwm->pmx);
+	pinmux_put(pwm->pmx);
 	kfree(pwm);
 	return 0;
 }

Signature · 1 line

1.7.0.4

Linus Walleij

2011-11-28 09:18 UTC #17 lore raw

PATCH 3/7 ARM: pxa: enable pinmux in pxa pwm

Quoted 5 lines · Haojian Zhuang

From: Haojian Zhuang <[email protected]>
Add pinmux operations in pxa pwm driver.
Signed-off-by Haojian Zhuang <[email protected]>

Same comment as for the serial driver,

I need one of the PXA maintainers (Eric, Russell) ACK to merge it through the pinctrl tree.

Thanks,
Linus Walleij

Russell King - ARM Linux

2011-11-28 12:49 UTC #18 lore raw

PATCH 3/7 ARM: pxa: enable pinmux in pxa pwm

Quoted 7 lines · Linus Walleij

Quoted 4 lines · Haojian Zhuang
From: Haojian Zhuang <[email protected]>
Add pinmux operations in pxa pwm driver.

Quoted 7 lines

Quoted 1 line
Signed-off-by Haojian Zhuang <[email protected]>
Same comment as for the serial driver,
Acked-by Linus Walleij <[email protected]>
I need one of the PXA maintainers (Eric, Russell) ACK to merge it through the pinctrl tree.

Acked-by Russell King <[email protected]>

Haojian Zhuang

author

2011-11-25 23:09 UTC #19 lore raw

PATCH 4/7 tty: serial: support pinmux in pxa serial

From: Haojian Zhuang <[email protected]>

PXA serial driver can configure pin directly.

---
 drivers/tty/serial/pxa.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

drivers/tty/serial/pxa.c +15 -1

index 5c8e3bb..98df48d 100644
@@ -37,6 +37,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/serial_core.h>

@@ -51,6 +52,7 @@ struct uart_pxa_port {
 	unsigned char           mcr;
 	unsigned int            lsr_break_flag;
 	struct clk		*clk;
+	struct pinmux		*pmx;
 	char			*name;
 };

@@ -802,6 +804,13 @@ static int serial_pxa_probe(struct platform_device *dev)
 		goto err_free;
 	}
 
+	sport->pmx = pinmux_get(&dev->dev, NULL);
+	if (IS_ERR(sport->pmx)) {
+		ret = PTR_ERR(sport->pmx);
+		goto err_clk;
+	}
+	pinmux_enable(sport->pmx);
+
 	sport->port.type = PORT_PXA;
 	sport->port.iotype = UPIO_MEM;
 	sport->port.mapbase = mmres->start;

@@ -826,7 +835,7 @@ static int serial_pxa_probe(struct platform_device *dev)
 	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
 	if (!sport->port.membase) {
 		ret = -ENOMEM;
-		goto err_clk;
+		goto err_pmx;
 	}
 
 	serial_pxa_ports[dev->id] = sport;

@@ -836,6 +845,9 @@ static int serial_pxa_probe(struct platform_device *dev)
 
 	return 0;
 
+ err_pmx:
+	pinmux_disable(sport->pmx);
+	pinmux_put(sport->pmx);
  err_clk:
 	clk_put(sport->clk);
  err_free:

@@ -851,6 +863,8 @@ static int serial_pxa_remove(struct platform_device *dev)
 
 	uart_remove_one_port(&serial_pxa_reg, &sport->port);
 	clk_put(sport->clk);
+	pinmux_disable(sport->pmx);
+	pinmux_put(sport->pmx);
 	kfree(sport);
 
 	return 0;

Signature · 1 line

1.7.0.4

Linus Walleij

2011-11-28 09:15 UTC #20 lore raw

PATCH 4/7 tty: serial: support pinmux in pxa serial

Quoted 5 lines · Haojian Zhuang

From: Haojian Zhuang <[email protected]>
PXA serial driver can configure pin directly.
Signed-off-by Haojian Zhuang <[email protected]>

(...)

Quoted 3 lines

+ err_pmx:
+ ? ? ? pinmux_disable(sport->pmx);
+ ? ? ? pinmux_put(sport->pmx);

There is this little discussion whether each individual driver should access it's pins or whether it should be done all centrally.

After some thinking and resoning I actually think this may be best to have in each driver in most cases. The reason is that while it is possible to standardize for simple usecases (like this one, just activate it at probe() and deactivate it at remove()) it gets really complicated and hairy to have it in central places when we come to things like suspend()/resume(), idling and sometimes even runtime_[suspend|resume].

So unless someone else disagrees or talks agains me, and provided this is how you want to do it for all PXA drivers:

I need Gregs ACK if you want it to go iin through the pinctrl tree.

Thanks,
Linus Walleij

Greg KH

2011-11-30 11:04 UTC #21 lore raw

PATCH 4/7 tty: serial: support pinmux in pxa serial

Quoted 24 lines · Linus Walleij

Quoted 5 lines · Haojian Zhuang
From: Haojian Zhuang <[email protected]>
PXA serial driver can configure pin directly.
Signed-off-by Haojian Zhuang <[email protected]>
(...)
Quoted 3 lines
+ err_pmx:
+ ? ? ? pinmux_disable(sport->pmx);
+ ? ? ? pinmux_put(sport->pmx);
There is this little discussion whether each individual driver should access it's pins or whether it should be done all centrally.
After some thinking and resoning I actually think this may be best to have in each driver in most cases. The reason is that while it is possible to standardize for simple usecases (like this one, just activate it at probe() and deactivate it at remove()) it gets really complicated and hairy to have it in central places when we come to things like suspend()/resume(), idling and sometimes even runtime_[suspend|resume].

Quoted 6 lines

So unless someone else disagrees or talks agains me, and provided this is how you want to do it for all PXA drivers:
Acked-by Linus Walleij <[email protected]>
I need Gregs ACK if you want it to go iin through the pinctrl tree.

Acked-by Greg Kroah-Hartman <[email protected]>

Haojian Zhuang

author

2011-11-25 23:09 UTC #22 lore raw

PATCH 5/7 fb: pxa: support pinmux operation

---
 drivers/video/pxafb.c |   10 ++++++++++
 drivers/video/pxafb.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

drivers/video/pxafb.c +10 -0

index 1d1e4f1..af5f618 100644
@@ -47,6 +47,7 @@
 #include <linux/ioport.h>
 #include <linux/cpufreq.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/dma-mapping.h>
 #include <linux/clk.h>
 #include <linux/err.h>

@@ -1799,11 +1800,18 @@ static struct pxafb_info * __devinit pxafb_init_fbinfo(struct device *dev)
 	memset(fbi, 0, sizeof(struct pxafb_info));
 	fbi->dev = dev;
 
+	fbi->pmx = pinmux_get(dev, NULL);
+	if (IS_ERR(fbi->pmx)) {
+		kfree(fbi);
+		return NULL;
+	}
 	fbi->clk = clk_get(dev, NULL);
 	if (IS_ERR(fbi->clk)) {
+		pinmux_put(fbi->pmx);
 		kfree(fbi);
 		return NULL;
 	}
+	pinmux_enable(fbi->pmx);
 
 	strcpy(fbi->fb.fix.id, PXA_NAME);

@@ -2293,6 +2301,8 @@ static int __devexit pxafb_remove(struct platform_device *dev)
 	release_mem_region(r->start, resource_size(r));
 
 	clk_put(fbi->clk);
+	pinmux_disable(fbi->pmx);
+	pinmux_put(fbi->pmx);
 	kfree(fbi);
 
 	return 0;

drivers/video/pxafb.h +1 -0

index 26ba9fa..d232816 100644
@@ -110,6 +110,7 @@ struct pxafb_info {
 	struct fb_info		fb;
 	struct device		*dev;
 	struct clk		*clk;
+	struct pinmux		*pmx;
 
 	void __iomem		*mmio_base;

Signature · 1 line

1.7.0.4

Linus Walleij

2011-11-28 09:19 UTC #23 lore raw

PATCH 5/7 fb: pxa: support pinmux operation

On Sat, Nov 26, 2011 at 12:09 AM, Haojian Zhuang
<[email protected]> wrote:

More verbose commit log please.

Quoted 1 line

Signed-off-by Haojian Zhuang <[email protected]>

Apart from that, same comment as for the serial driver,

I need the framebuffer maintainers ACK to merge this through the pinctrl tree.

Thanks,
Linus Walleij

Florian Tobias Schandinat

2011-12-02 21:53 UTC #24 lore raw

PATCH 5/7 fb: pxa: support pinmux operation

Hi Linus,

Quoted 5 lines · Linus Walleij

On Sat, Nov 26, 2011 at 12:09 AM, Haojian Zhuang
<[email protected]> wrote:
More verbose commit log please.

Quoted 7 lines

Quoted 1 line
Signed-off-by Haojian Zhuang <[email protected]>
Apart from that, same comment as for the serial driver,
Acked-by Linus Walleij <[email protected]>
I need the framebuffer maintainers ACK to merge this through the pinctrl tree.

You could have made my life easier by quoting the patch. Anyway, after I found it in an archive, you may add

Acked-by Florian Tobias Schandinat <[email protected]>

Best regards,

Florian Tobias Schandinat

Linus Walleij

2011-12-05 13:07 UTC #25 lore raw

PATCH 5/7 fb: pxa: support pinmux operation

Quoted 4 lines · Florian Tobias Schandinat

You could have made my life easier by quoting the patch. Anyway, after I found it in an archive, you may add
Acked-by Florian Tobias Schandinat <[email protected]>

Ah. sorry, I compulsory cut down my reply quotes after being yelled at to stop quoting too much, in this case I didn't think long enough...

Yours,
Linus Walleij

Haojian Zhuang

author

2011-11-25 23:09 UTC #26 lore raw

PATCH 6/7 ARM: pxa: enable pinmux mapping in zylonite

Remove MFP operation in zylonite. Use pinmux mapping instead.

---
 arch/arm/mach-pxa/Kconfig                 |    1 +
 arch/arm/mach-pxa/devices.c               |   15 ++
 arch/arm/mach-pxa/devices.h               |    1 +
 arch/arm/mach-pxa/include/mach/zylonite.h |    2 +
 arch/arm/mach-pxa/pxa3xx.c                |    1 +
 arch/arm/mach-pxa/zylonite.c              |    6 +
 arch/arm/mach-pxa/zylonite_pxa300.c       |  298 +++++++++++------------------
 arch/arm/mach-pxa/zylonite_pxa320.c       |  211 +++++----------------
 8 files changed, 183 insertions(+), 352 deletions(-)

arch/arm/mach-pxa/Kconfig +1 -0

index 61d3c72..ea569ff 100644
@@ -39,6 +39,7 @@ config MACH_ZYLONITE
 	bool
 	select PXA3xx
 	select HAVE_PWM
+	select PINCTRL
 
 config MACH_ZYLONITE300
 	bool "PXA3xx Development Platform (aka Zylonite) PXA300/310"

arch/arm/mach-pxa/devices.c +15 -0

index 5bc1312..3302bed 100644
@@ -946,6 +946,21 @@ struct platform_device pxa3xx_device_gcu = {
 	},
 };
 
+static struct resource pxa3xx_resources_pinmux[] = {
+	{
+		.start	= 0x40e10000,
+		.end	= 0x40e10fff,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+struct platform_device pxa3xx_device_pinmux = {
+	.name		= "pxa3xx-pinmux",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(pxa3xx_resources_pinmux),
+	.resource	= pxa3xx_resources_pinmux,
+};
+
 #endif /* CONFIG_PXA3xx */
 
 #if defined(CONFIG_PXA3xx) || defined(CONFIG_PXA95x)

arch/arm/mach-pxa/devices.h +1 -0

index 1475db1..d440fbb 100644
@@ -39,6 +39,7 @@ extern struct platform_device pxa3xx_device_nand;
 extern struct platform_device pxa3xx_device_i2c_power;
 
 extern struct platform_device pxa3xx_device_gcu;
+extern struct platform_device pxa3xx_device_pinmux;
 
 extern struct platform_device pxa_device_asoc_platform;
 extern struct platform_device pxa_device_asoc_ssp1;

arch/arm/mach-pxa/include/mach/zylonite.h +2 -0

index ecca976..70e5017 100644
@@ -15,6 +15,8 @@ extern int gpio_debug_led1;
 extern int gpio_debug_led2;
 
 extern int wm9713_irq;
+extern int ac97_sdata1;
+extern int mmc1_cmd1;
 
 extern int lcd_id;
 extern int lcd_orientation;

arch/arm/mach-pxa/pxa3xx.c +1 -0

index 4f402af..eea2cb6 100644
@@ -435,6 +435,7 @@ static struct platform_device *devices[] __initdata = {
 	&pxa3xx_device_ssp4,
 	&pxa27x_device_pwm0,
 	&pxa27x_device_pwm1,
+	&pxa3xx_device_pinmux,
 };
 
 static int __init pxa3xx_init(void)

arch/arm/mach-pxa/zylonite.c +6 -0

index 98eec80..13ddba0 100644
@@ -41,6 +41,8 @@ int gpio_debug_led1;
 int gpio_debug_led2;
 
 int wm9713_irq;
+int ac97_sdata1;
+int mmc1_cmd1;
 
 int lcd_id;
 int lcd_orientation;

@@ -52,6 +54,7 @@ struct platform_device pxa_device_wm9713_audio = {
 
 static void __init zylonite_init_wm9713_audio(void)
 {
+	gpio_request_one(wm9713_irq, GPIOF_DIR_IN, "wm9713 irq");
 	platform_device_register(&pxa_device_wm9713_audio);
 }

@@ -253,6 +256,7 @@ static struct pxamci_platform_data zylonite_mci3_platform_data = {
 
 static void __init zylonite_init_mmc(void)
 {
+	gpio_request_one(mmc1_cmd1, GPIOF_DIR_IN, "mmc1 CDM1");
 	pxa_set_mci_info(&zylonite_mci_platform_data);
 	pxa3xx_set_mci2_info(&zylonite_mci2_platform_data);
 	if (cpu_is_pxa310())

@@ -408,10 +412,12 @@ static void __init zylonite_init(void)
 	 * Note: We depend that the bootloader set
 	 * the correct value to MSC register for SMC91x.
 	 */
+	gpio_request_one(gpio_eth_irq, GPIOF_DIR_IN, "eth irq");
 	smc91x_resources[1].start = PXA_GPIO_TO_IRQ(gpio_eth_irq);
 	smc91x_resources[1].end   = PXA_GPIO_TO_IRQ(gpio_eth_irq);
 	platform_device_register(&smc91x_device);
 
+	gpio_request_one(ac97_sdata1, GPIOF_DIR_IN, "SDATA_IN_1 not used");
 	pxa_set_ac97_info(NULL);
 	zylonite_init_lcd();
 	zylonite_init_mmc();

arch/arm/mach-pxa/zylonite_pxa300.c +108 ~~-190~~

348 lines not on this page · raw message

arch/arm/mach-pxa/zylonite_pxa320.c +49 ~~-162~~

248 lines not on this page · raw message

Signature · 1 line

1.7.0.4

Arnd Bergmann

2011-11-26 03:53 UTC #27 lore raw

PATCH 6/7 ARM: pxa: enable pinmux mapping in zylonite

Quoted 13 lines · Haojian Zhuang

Remove MFP operation in zylonite. Use pinmux mapping instead.

---
 arch/arm/mach-pxa/Kconfig                 |    1 +
 arch/arm/mach-pxa/devices.c               |   15 ++
 arch/arm/mach-pxa/devices.h               |    1 +
 arch/arm/mach-pxa/include/mach/zylonite.h |    2 +
 arch/arm/mach-pxa/pxa3xx.c                |    1 +
 arch/arm/mach-pxa/zylonite.c              |    6 +
 arch/arm/mach-pxa/zylonite_pxa300.c       |  298 +++++++++++------------------
 arch/arm/mach-pxa/zylonite_pxa320.c       |  211 +++++----------------
 8 files changed, 183 insertions(+), 352 deletions(-)

This looks quite nice overall, but there is one thing that made me wonder if there should be a better way to express it:

Quoted 32 lines

arch/arm/mach-pxa/zylonite_pxa300.c +32 -0

+#define COMMON_PINMUX_MAP()					\
+	{							\
+		.name = "uart2",				\
+		.ctrl_dev_name = "pinctrl.0",			\
+		.function = "uart2",				\
+		.dev_name = "pxa2xx-uart.2",			\
+		.group = "stuart1",				\
+		.hog_on_boot = true,				\
+	}, {							\
+		.name = "key",					\
+		.ctrl_dev_name = "pinctrl.0",			\
+		.function = "key",				\
+		.dev_name = "pxa27x-keyboard",			\
+		.group = "key0",				\
+		.hog_on_boot = true,				\
+	}, {							\
+		.name = "mmc1",					\
+		.ctrl_dev_name = "pinctrl.0",			\
+		.function = "mmc1",				\
+		.group = "mmc1_0",				\
+		.hog_on_boot = true,				\
+	},							\
+	PINMUX_MAP_PRIMARY_SYS_HOG("mmc2", "mmc2"),		\
+	PINMUX_MAP_PRIMARY_SYS_HOG("usbh", "usbh"),		\
+	PINMUX_MAP_PRIMARY_SYS_HOG("ac97", "ac97"),		\
+	PINMUX_MAP_PRIMARY_SYS_HOG("smc", "smc"),		\
+	PINMUX_MAP_PRIMARY("pwm3", "pwm3", "pxa27x-pwm.1"),	\
+	PINMUX_MAP_PRIMARY("uart1", "uart1", "pxa2xx-uart.1"),	\
+	PINMUX_MAP_PRIMARY("ssp3", "ssp3", "pxa27x-ssp.2"),	\
+	PINMUX_MAP_PRIMARY("lcd", "lcd", "pxa2xx-fb"),		\
+	PINMUX_MAP_PRIMARY("i2c", "i2c", "pxa2xx-i2c.0")
+

arch/arm/mach-pxa/zylonite_pxa300.c line 71

Quoted 10 lines

arch/arm/mach-pxa/zylonite_pxa300.c +9 -0

+static struct pinmux_map pxa300_pmx_map[] = {
+	COMMON_PINMUX_MAP(),
+	{
+		.name = "uart0",
+		.ctrl_dev_name = "pinctrl.0",
+		.function = "uart0",
+		.dev_name = "pxa2xx-uart.0",
+		.group = "ffuart0",
+	},
 };

I think we should not be forced to use macros like this in order to extend a common pinmux mapping. Unfortunately, I can't see an easy way to do it differently with the pinmux core, but it might not be too late to extend the interfaces now.

I also noticed that the pinmux_map arrays are not marked __initdata, because it gets referenced later. This means that in a kernel that supports many machines, we also have to keep all pinmux configurations around at runtime.

Linus, what do you think about changing the pinmux_register_mappings function to make a copy of the data in order to let the platform code mark the data as __initdata, and to allow appending the maps at boot time so that the above can become two arrays that are registered individually?

arch/arm/mach-pxa/zylonite_pxa320.c line 92

Quoted 8 lines

arch/arm/mach-pxa/zylonite_pxa320.c +4 -1

 		/* GPIO pin assignment */
-		gpio_eth_irq	= mfp_to_gpio(MFP_PIN_GPIO9);
+		gpio_eth_irq	= 90;
+#if 0
+		/* FIXME: can't support additional gpio pins */
 		gpio_debug_led1	= mfp_to_gpio(MFP_PIN_GPIO1_2);
 		gpio_debug_led2	= mfp_to_gpio(MFP_PIN_GPIO4_2);
+#endif

If I read this correctly, your comment here refers to a different aspect of the same problem, not being able to amend the mapping at a later point.

Arnd

Haojian Zhuang

maintainer

2011-11-26 04:45 UTC #28 lore raw

PATCH 6/7 ARM: pxa: enable pinmux mapping in zylonite

Quoted 16 lines · Arnd Bergmann

Quoted 13 lines · Haojian Zhuang

Remove MFP operation in zylonite. Use pinmux mapping instead.

In reply to Arnd Bergmann

---

?arch/arm/mach-pxa/Kconfig ? ? ? ? ? ? ? ? | ? ?1 +
?arch/arm/mach-pxa/devices.c ? ? ? ? ? ? ? | ? 15 ++
?arch/arm/mach-pxa/devices.h ? ? ? ? ? ? ? | ? ?1 +
?arch/arm/mach-pxa/include/mach/zylonite.h | ? ?2 +
?arch/arm/mach-pxa/pxa3xx.c ? ? ? ? ? ? ? ?| ? ?1 +
?arch/arm/mach-pxa/zylonite.c ? ? ? ? ? ? ?| ? ?6 +
?arch/arm/mach-pxa/zylonite_pxa300.c ? ? ? | ?298 +++++++++++------------------
?arch/arm/mach-pxa/zylonite_pxa320.c ? ? ? | ?211 +++++----------------

?8 files changed, 183 insertions(+), 352 deletions(-)

Quoted 12 lines

Quoted 8 lines
? ? ? ? ? ? ? /* GPIO pin assignment */
- ? ? ? ? ? ? gpio_eth_irq ? ?= mfp_to_gpio(MFP_PIN_GPIO9);
+ ? ? ? ? ? ? gpio_eth_irq ? ?= 90;
+#if 0
+ ? ? ? ? ? ? /* FIXME: can't support additional gpio pins */ ? ? ? ? ? ? ? gpio_debug_led1 = mfp_to_gpio(MFP_PIN_GPIO1_2); ? ? ? ? ? ? ? gpio_debug_led2 = mfp_to_gpio(MFP_PIN_GPIO4_2); +#endif
If I read this correctly, your comment here refers to a different aspect of the same problem, not being able to amend the mapping at a later point.

GPIO1, GPIO4, GPIO1_2, GPIO4_2 are four pads. Both GPIO1 and GPIO1_2 can be configured as GPIO1. It's same on GPIO4/GPIO4/2.

At here, we need to configure GPIO1_2 as GPIO1. It also means that we need set GPIO1 as not gpio function automatically. In current pinmux driver, one GPIO can only be routed to one pad. Now I don't have good ideas on how to resolve this. Any comments are welcome.

Best Regards
Haojian

Linus Walleij

2011-11-28 09:32 UTC #29 lore raw

PATCH 6/7 ARM: pxa: enable pinmux mapping in zylonite

Quoted 41 lines · Arnd Bergmann

Quoted 40 lines · Haojian Zhuang

Remove MFP operation in zylonite. Use pinmux mapping instead.

(quoted diff) +38 -0

+#define COMMON_PINMUX_MAP() ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .name = "uart2", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .ctrl_dev_name = "pinctrl.0", ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .function = "uart2", ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .dev_name = "pxa2xx-uart.2", ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .group = "stuart1", ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .hog_on_boot = true, ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? }, { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .name = "key", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .ctrl_dev_name = "pinctrl.0", ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .function = "key", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .dev_name = "pxa27x-keyboard", ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .group = "key0", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .hog_on_boot = true, ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? }, { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .name = "mmc1", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .ctrl_dev_name = "pinctrl.0", ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .function = "mmc1", ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? .group = "mmc1_0", ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .hog_on_boot = true, ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? PINMUX_MAP_PRIMARY_SYS_HOG("mmc2", "mmc2"), ? ? ? ? ? ? \
+ ? ? PINMUX_MAP_PRIMARY_SYS_HOG("usbh", "usbh"), ? ? ? ? ? ? \
+ ? ? PINMUX_MAP_PRIMARY_SYS_HOG("ac97", "ac97"), ? ? ? ? ? ? \
+ ? ? PINMUX_MAP_PRIMARY_SYS_HOG("smc", "smc"), ? ? ? ? ? ? ? \
+ ? ? PINMUX_MAP_PRIMARY("pwm3", "pwm3", "pxa27x-pwm.1"), ? ? \
+ ? ? PINMUX_MAP_PRIMARY("uart1", "uart1", "pxa2xx-uart.1"), ?\
+ ? ? PINMUX_MAP_PRIMARY("ssp3", "ssp3", "pxa27x-ssp.2"), ? ? \
+ ? ? PINMUX_MAP_PRIMARY("lcd", "lcd", "pxa2xx-fb"), ? ? ? ? ?\
+ ? ? PINMUX_MAP_PRIMARY("i2c", "i2c", "pxa2xx-i2c.0")
+
+static struct pinmux_map pxa300_pmx_map[] = {
+ ? ? COMMON_PINMUX_MAP(),
+ ? ? {
+ ? ? ? ? ? ? .name = "uart0",
+ ? ? ? ? ? ? .ctrl_dev_name = "pinctrl.0",
+ ? ? ? ? ? ? .function = "uart0",

Quoted 8 lines

Quoted 4 lines
+ ? ? ? ? ? ? .dev_name = "pxa2xx-uart.0",
+ ? ? ? ? ? ? .group = "ffuart0",
+ ? ? },
?};
I think we should not be forced to use macros like this in order to extend a common pinmux mapping. Unfortunately, I can't see an easy way to do it differently with the pinmux core, but it might not be too late to extend the interfaces now.

No problem at all, if I just get a patch to extend the mapping with whatever is needed to use large common subsets I'll review/merge, it looks pretty useful for PXA.

Quoted 8 lines

I also noticed that the pinmux_map arrays are not marked __initdata, because it gets referenced later. This means that in a kernel that supports many machines, we also have to keep all pinmux configurations around at runtime.
Linus, what do you think about changing the pinmux_register_mappings function to make a copy of the data in order to let the platform code mark the data as __initdata, and to allow appending the maps at boot time so that the above can become two arrays that are registered individually?

It's a bit costly since it involves making deep copies of structs that in turn contain a few strings each, leading to a lot of kstrdup() happening for a large map, but I'll propose something for the single in-kernel user so we can see how it looks.

Yours,
Linus Walleij

Haojian Zhuang

author

2011-11-25 23:09 UTC #30 lore raw

PATCH 7/7 ARM: mmp: enable pinmux on ttc dkb

Remove MFP operation. Use pinmux mapping instead.

---
 arch/arm/mach-mmp/Kconfig   |    1 +
 arch/arm/mach-mmp/ttc_dkb.c |   40 ++++++++++------------------------------
 2 files changed, 11 insertions(+), 30 deletions(-)

arch/arm/mach-mmp/Kconfig +1 -0

index 323d4c9..bd67c38 100644
@@ -33,6 +33,7 @@ config MACH_TAVOREVB
 config MACH_TTC_DKB
 	bool "Marvell's PXA910 TavorEVB Development Board"
 	select CPU_PXA910
+	select PINCTRL
 	help
 	  Say 'Y' here if you want to support the Marvell PXA910-based
 	  TTC_DKB Development Board.

arch/arm/mach-mmp/ttc_dkb.c +10 ~~-30~~

index 5ac5d58..28d3cc8 100644
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/machine.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/onenand.h>

@@ -22,7 +23,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <mach/addr-map.h>
-#include <mach/mfp-pxa910.h>
 #include <mach/pxa910.h>
 #include <mach/irqs.h>

@@ -40,34 +40,8 @@
  */
 #define TTCDKB_NR_IRQS		(IRQ_BOARD_START + 16 + 16 + 24)
 
-static unsigned long ttc_dkb_pin_config[] __initdata = {
-	/* UART2 */
-	GPIO47_UART2_RXD,
-	GPIO48_UART2_TXD,
-
-	/* DFI */
-	DF_IO0_ND_IO0,
-	DF_IO1_ND_IO1,
-	DF_IO2_ND_IO2,
-	DF_IO3_ND_IO3,
-	DF_IO4_ND_IO4,
-	DF_IO5_ND_IO5,
-	DF_IO6_ND_IO6,
-	DF_IO7_ND_IO7,
-	DF_IO8_ND_IO8,
-	DF_IO9_ND_IO9,
-	DF_IO10_ND_IO10,
-	DF_IO11_ND_IO11,
-	DF_IO12_ND_IO12,
-	DF_IO13_ND_IO13,
-	DF_IO14_ND_IO14,
-	DF_IO15_ND_IO15,
-	DF_nCS0_SM_nCS2_nCS0,
-	DF_ALE_SM_WEn_ND_ALE,
-	DF_CLE_SM_OEn_ND_CLE,
-	DF_WEn_DF_WEn,
-	DF_REn_DF_REn,
-	DF_RDY0_DF_RDY0,
+static struct pinmux_map ttc_dkb_pmx_map[] = {
+	PINMUX_MAP_PRIMARY("uart2", "uart2", "pxa2xx-uart.1"),
 };
 
 static struct mtd_partition ttc_dkb_onenand_partitions[] = {

@@ -145,14 +119,20 @@ static struct i2c_board_info ttc_dkb_i2c_info[] = {
 
 static void __init ttc_dkb_init(void)
 {
-	mfp_config(ARRAY_AND_SIZE(ttc_dkb_pin_config));
+	int ret;
 
+	ret = pinmux_register_mappings(ARRAY_AND_SIZE(ttc_dkb_pmx_map));
+	if (ret < 0)
+		goto out;
 	/* on-chip devices */
 	pxa910_add_uart(1);
 
 	/* off-chip devices */
 	pxa910_add_twsi(0, NULL, ARRAY_AND_SIZE(ttc_dkb_i2c_info));
 	platform_add_devices(ARRAY_AND_SIZE(ttc_dkb_devices));
+	return;
+out:
+	BUG();
 }
 
 MACHINE_START(TTC_DKB, "PXA910-based TTC_DKB Development Platform")

Signature · 1 line

1.7.0.4

Linus Walleij

2011-11-28 09:22 UTC #31 lore raw

PATCH 7/7 ARM: mmp: enable pinmux on ttc dkb

Quoted 3 lines · Haojian Zhuang

Remove MFP operation. Use pinmux mapping instead.
Signed-off-by Haojian Zhuang <[email protected]>

As stated earlier, I think the new pinctrl platform data should be passed in from
here, apart from that: