Skip to contentSkip to conversation

pxa2xx pcmcia - vpac glue code fix and trizeps cleanup.

PATCH New Jonathan Cameron 3 participants 8 messages Jun 6, 2011 – Jun 15, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v2 cover pxa2xx pcmcia - vpac glue code fix and trizeps cleanup.

V2 - actually put the right device name in the patch titles (thanks Marc)
add some more trivial cleanup to trizeps (patch 3)

Original message:

The first patch looks like a simple bug.
The second removes an ifdef that doesn't do anything.

I was looking through this code with a vague view to simplifying it. Why do pxa2xx and sa1100 require lots of platform glue when others subarches do not?

On the second one, it looks to me like all those ifdefs could be trivially removed.

Otherwise, a lot of these bits of glue code could be greatly reduced by introducing a few default functions and const structures. Anyone bored enough to do it?

I might carry on dabbling if people familiar with all these boards are happy to review.

Jonathan

Jonathan Cameron (3):
pxa2xx vpac: pcmcia - free gpios on exist rather than requesting them
again.
pxa2xx trizeps4: pcmcia remove unnecessary ifdefs
pxa2xx trizeps4 pcmcia: code cleanup

 drivers/pcmcia/pxa2xx_trizeps4.c |   95 +++++++++++++++++--------------------
 drivers/pcmcia/pxa2xx_vpac270.c  |    4 +-
 2 files changed, 46 insertions(+), 53 deletions(-)
Signature · 1 line
1.7.3.4
#1 lore raw

PATCH v2 1/3 pxa2xx vpac: pcmcia - free gpios on exist rather than requesting them again.

---
 drivers/pcmcia/pxa2xx_vpac270.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/pcmcia/pxa2xx_vpac270.c +2 -2
index 435002d..413dc7d 100644
@@ -75,10 +75,10 @@ static int vpac270_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 static void vpac270_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
 {
 	if (skt->nr == 0)
-		gpio_request_array(vpac270_pcmcia_gpios,
+		gpio_free_array(vpac270_pcmcia_gpios,
 					ARRAY_SIZE(vpac270_pcmcia_gpios));
 	else
-		gpio_request_array(vpac270_cf_gpios,
+		gpio_free_array(vpac270_cf_gpios,
 					ARRAY_SIZE(vpac270_cf_gpios));
 }
 
Signature · 1 line
1.7.3.4
#2 lore raw

PATCH 1/3 pxa2xx vpac: pcmcia - free gpios on exist rather than requesting them again.

Quoted 12 lines · Jonathan Cameron
---
 drivers/pcmcia/pxa2xx_vpac270.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/drivers/pcmcia/pxa2xx_vpac270.c index 435002d..413dc7d 100644

drivers/pcmcia/pxa2xx_vpac270.c +0 -0
@@ -75,10 +75,10 @@ static int vpac270_pcmcia_hw_init(struct

soc_pcmcia_socket *skt) static void vpac270_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt) {

drivers/pcmcia/pxa2xx_vpac270.c static int vpac270_pcmcia_hw_init(struct soc_pcmcia_socket *skt) · line 83

Quoted 9 lines
drivers/pcmcia/pxa2xx_vpac270.c +2 -2
 	if (skt->nr == 0)
-		gpio_request_array(vpac270_pcmcia_gpios,
+		gpio_free_array(vpac270_pcmcia_gpios,
 					ARRAY_SIZE(vpac270_pcmcia_gpios));
 	else
-		gpio_request_array(vpac270_cf_gpios,
+		gpio_free_array(vpac270_cf_gpios,
 					ARRAY_SIZE(vpac270_cf_gpios));
 }

Please add my

Thanks, cheers

Applied
#3 lore raw

PATCH 1/3 pxa2xx vpac: pcmcia - free gpios on exist rather than requesting them again.

2011/6/6 Marek Vasut <[email protected]>:

Quoted 25 lines · Jonathan Cameron
Quoted 13 lines

---
?drivers/pcmcia/pxa2xx_vpac270.c | ? ?4 ++--
?1 files changed, 2 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/drivers/pcmcia/pxa2xx_vpac270.c index 435002d..413dc7d 100644

drivers/pcmcia/pxa2xx_vpac270.c +0 -0
@@ -75,10 +75,10 @@ static int vpac270_pcmcia_hw_init(struct

soc_pcmcia_socket *skt) static void vpac270_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt) {
? ? ? if (skt->nr == 0)

Quoted 8 lines

- ? ? ? ? ? ? gpio_request_array(vpac270_pcmcia_gpios,
+ ? ? ? ? ? ? gpio_free_array(vpac270_pcmcia_gpios,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(vpac270_pcmcia_gpios)); ? ? ? else
- ? ? ? ? ? ? gpio_request_array(vpac270_cf_gpios,
+ ? ? ? ? ? ? gpio_free_array(vpac270_cf_gpios,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ARRAY_SIZE(vpac270_cf_gpios)); ?}

Please add my

Quoted 2 lines

Applied.

#4 lore raw

PATCH v2 2/3 pxa2xx trizeps4: pcmcia remove unnecessary ifdefs

In reply to Jonathan Cameron
---
 drivers/pcmcia/pxa2xx_trizeps4.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
drivers/pcmcia/pxa2xx_trizeps4.c +0 -4
index b829e65..57ddb96 100644
@@ -55,10 +55,6 @@ static int trizeps_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 		}
 		skt->socket.pci_irq = IRQ_GPIO(GPIO_PRDY);
 		break;
-
-#ifndef CONFIG_MACH_TRIZEPS_CONXS
-	case 1:
-#endif
 	default:
 		break;
 	}
Signature · 1 line
1.7.3.4
#5 lore raw

PATCH 2/3 pxa2xx trizeps4: pcmcia remove unnecessary ifdefs

Quoted 10 lines · Jonathan Cameron
---
 drivers/pcmcia/pxa2xx_trizeps4.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
(quoted diff) +0 -0

b/drivers/pcmcia/pxa2xx_trizeps4.c index b829e65..57ddb96 100644

drivers/pcmcia/pxa2xx_trizeps4.c static int trizeps_pcmcia_hw_init(struct · line 60

Quoted 10 lines
drivers/pcmcia/pxa2xx_trizeps4.c +0 -4
@@ -55,10 +55,6 @@ static int trizeps_pcmcia_hw_init(struct
 soc_pcmcia_socket *skt) }
 		skt->socket.pci_irq = IRQ_GPIO(GPIO_PRDY);
 		break;
-
-#ifndef CONFIG_MACH_TRIZEPS_CONXS
-	case 1:
-#endif
 	default:
 		break;
 	}

Please add my

Thanks, cheers

Applied
#6 lore raw

PATCH 2/3 pxa2xx trizeps4: pcmcia remove unnecessary ifdefs

Quoted 23 lines · Marek Vasut
Quoted 17 lines · Jonathan Cameron

---
?drivers/pcmcia/pxa2xx_trizeps4.c | ? ?4 ----
?1 files changed, 0 insertions(+), 4 deletions(-)

(quoted diff) +0 -0

b/drivers/pcmcia/pxa2xx_trizeps4.c index b829e65..57ddb96 100644

drivers/pcmcia/pxa2xx_trizeps4.c +0 -4
@@ -55,10 +55,6 @@ static int trizeps_pcmcia_hw_init(struct
 soc_pcmcia_socket *skt) }
 ? ? ? ? ? ? ? skt->socket.pci_irq = IRQ_GPIO(GPIO_PRDY);
 ? ? ? ? ? ? ? break;
-
-#ifndef CONFIG_MACH_TRIZEPS_CONXS
- ? ? case 1:
-#endif
Quoted 3 lines

? ? ? default:
? ? ? ? ? ? ? break;
? ? ? }

Please add my

Quoted 2 lines

Applied.

Quoted 7 lines

Thanks, cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#7 lore raw

PATCH v2 3/3 pxa2xx trizeps4 pcmcia: code cleanup

In reply to Jonathan Cameron

Check patch warnings and use of more modern gpio allocation etc. More can be done in here, but this is a start. Getting these cleaned up will make shared functionality much easier to spot and patches easier to review.

---
 drivers/pcmcia/pxa2xx_trizeps4.c |   91 ++++++++++++++++++-------------------
 1 files changed, 44 insertions(+), 47 deletions(-)
drivers/pcmcia/pxa2xx_trizeps4.c +44 -47
index 57ddb96..e275402 100644
@@ -42,16 +42,12 @@ static int trizeps_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 	 */
 	switch (skt->nr) {
 	case 0:
-		if (gpio_request(GPIO_PRDY, "cf_irq") < 0) {
-			pr_err("%s: sock %d unable to request gpio %d\n", __func__,
-				skt->nr, GPIO_PRDY);
-			return -EBUSY;
-		}
-		if (gpio_direction_input(GPIO_PRDY) < 0) {
-			pr_err("%s: sock %d unable to set input gpio %d\n", __func__,
-				skt->nr, GPIO_PRDY);
-			gpio_free(GPIO_PRDY);
-			return -EINVAL;
+		ret = gpio_request_one(GPIO_PRDY, GPIOF_IN, "cf_irq");
+		if (ret < 0) {
+			pr_err("%s: sock %d unable to request gpio %d\n",
+			       __func__,
+			       skt->nr, GPIO_PRDY);
+			return ret;
 		}
 		skt->socket.pci_irq = IRQ_GPIO(GPIO_PRDY);
 		break;
@@ -59,32 +55,28 @@ static int trizeps_pcmcia_hw_init(struct soc_pcmcia_socket *skt)
 		break;
 	}
 	/* release the reset of this card */
-	pr_debug("%s: sock %d irq %d\n", __func__, skt->nr, skt->socket.pci_irq);
+	pr_debug("%s: sock %d irq %d\n", __func__, skt->nr,
+		 skt->socket.pci_irq);
 
 	/* supplementory irqs for the socket */
 	for (i = 0; i < ARRAY_SIZE(irqs); i++) {
 		if (irqs[i].sock != skt->nr)
 			continue;
-		if (gpio_request(irq_to_gpio(irqs[i].irq), irqs[i].str) < 0) {
+		ret = gpio_request_one(irq_to_gpio(irqs[i].irq), GPIOF_IN,
+				       irqs[i].str);
+		if (ret < 0) {
 			pr_err("%s: sock %d unable to request gpio %d\n",
 				__func__, skt->nr, irq_to_gpio(irqs[i].irq));
-			ret = -EBUSY;
-			goto error;
-		}
-		if (gpio_direction_input(irq_to_gpio(irqs[i].irq)) < 0) {
-			pr_err("%s: sock %d unable to set input gpio %d\n",
-				__func__, skt->nr, irq_to_gpio(irqs[i].irq));
-			ret = -EINVAL;
 			goto error;
 		}
 	}
 	return soc_pcmcia_request_irqs(skt, irqs, ARRAY_SIZE(irqs));
 
 error:
-	for (; i >= 0; i--) {
+	for (; i >= 0; i--)
 		gpio_free(irq_to_gpio(irqs[i].irq));
-	}
-	return (ret);
+
+	return ret;
 }
 
 static void trizeps_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
@@ -94,6 +86,7 @@ static void trizeps_pcmcia_hw_shutdown(struct soc_pcmcia_socket *skt)
 	gpio_free(GPIO_PRDY);
 	for (i = 0; i < ARRAY_SIZE(irqs); i++)
 		gpio_free(irq_to_gpio(irqs[i].irq));
+	soc_pcmcia_free_irqs(skt, irqs, ARRAY_SIZE(irqs));
 }
 
 static unsigned long trizeps_pcmcia_status[2];
@@ -101,32 +94,25 @@ static unsigned long trizeps_pcmcia_status[2];
 static void trizeps_pcmcia_socket_state(struct soc_pcmcia_socket *skt,
 				struct pcmcia_state *state)
 {
-	unsigned short status = 0, change;
+	unsigned short status, change;
+
 	status = CFSR_readw();
 	change = (status ^ trizeps_pcmcia_status[skt->nr]) &
 				ConXS_CFSR_BVD_MASK;
-	if (change) {
+	if (change)
 		trizeps_pcmcia_status[skt->nr] = status;
-		if (status & ConXS_CFSR_BVD1) {
-			/* enable_irq empty */
-		} else {
-			/* disable_irq empty */
-		}
-	}
 
 	switch (skt->nr) {
 	case 0:
 		/* just fill in fix states */
-		state->detect = gpio_get_value(GPIO_PCD) ? 0 : 1;
-		state->ready  = gpio_get_value(GPIO_PRDY) ? 1 : 0;
-		state->bvd1   = (status & ConXS_CFSR_BVD1) ? 1 : 0;
-		state->bvd2   = (status & ConXS_CFSR_BVD2) ? 1 : 0;
-		state->vs_3v  = (status & ConXS_CFSR_VS1) ? 0 : 1;
-		state->vs_Xv  = (status & ConXS_CFSR_VS2) ? 0 : 1;
+		state->detect = !gpio_get_value(GPIO_PCD);
+		state->ready  = !!gpio_get_value(GPIO_PRDY);
+		state->bvd1   = !!(status & ConXS_CFSR_BVD1);
+		state->bvd2   = !!(status & ConXS_CFSR_BVD2);
+		state->vs_3v  = !(status & ConXS_CFSR_VS1);
+		state->vs_Xv  = !(status & ConXS_CFSR_VS2);
 		state->wrprot = 0;	/* not available */
 		break;
-
-#ifndef CONFIG_MACH_TRIZEPS_CONXS
 	/* on ConXS we only have one slot. Second is inactive */
 	case 1:
 		state->detect = 0;
@@ -137,8 +123,6 @@ static void trizeps_pcmcia_socket_state(struct soc_pcmcia_socket *skt,
 		state->vs_Xv  = 0;
 		state->wrprot = 0;
 		break;
-
-#endif
 	}
 }
 
@@ -150,8 +134,12 @@ static int trizeps_pcmcia_configure_socket(struct soc_pcmcia_socket *skt,
 
 	/* we do nothing here just check a bit */
 	switch (state->Vcc) {
-	case 0:  power &= 0xfc; break;
-	case 33: power |= ConXS_BCR_S0_VCC_3V3; break;
+	case 0:
+		power &= 0xfc;
+		break;
+	case 33:
+		power |= ConXS_BCR_S0_VCC_3V3;
+		break;
 	case 50:
 		pr_err("%s(): Vcc 5V not supported in socket\n", __func__);
 		break;
@@ -161,8 +149,12 @@ static int trizeps_pcmcia_configure_socket(struct soc_pcmcia_socket *skt,
 	}
 
 	switch (state->Vpp) {
-	case 0:  power &= 0xf3; break;
-	case 33: power |= ConXS_BCR_S0_VPP_3V3; break;
+	case 0:
+		power &= 0xf3;
+		break;
+	case 33:
+		power |= ConXS_BCR_S0_VPP_3V3;
+		break;
 	case 120:
 		pr_err("%s(): Vpp 12V not supported in socket\n", __func__);
 		break;
@@ -232,12 +224,17 @@ static int __init trizeps_pcmcia_init(void)
 	ret = platform_device_add_data(trizeps_pcmcia_device,
 			&trizeps_pcmcia_ops, sizeof(trizeps_pcmcia_ops));
 
-	if (ret == 0)
-		ret = platform_device_add(trizeps_pcmcia_device);
+	if (ret)
+		goto error_put_dev;
 
+	ret = platform_device_add(trizeps_pcmcia_device);
 	if (ret)
-		platform_device_put(trizeps_pcmcia_device);
+		goto error_put_dev;
+
+	return 0;
 
+error_put_dev:
+	platform_device_put(trizeps_pcmcia_device);
 	return ret;
 }
 
Signature · 1 line
1.7.3.4