m68knommu: Coldfire gpio fixes and updates.

PATCH In review [email protected] 4 participants 11 messages May 21, 2014 – May 22, 2014 m68k architecture

lore

Share & tools

lore

[email protected]

author

2014-05-21 23:00 UTC #0 lore raw

cover m68knommu: Coldfire gpio fixes and updates.

From: Steven King <[email protected]>

I had these queued up, forgot to send. This fixes a heinous bug in mcfgpio_direction_output, adds code to enable gpio_to_irq functionality, removes some of the global namespace pollution in gpio.c and adds gpio support for the m54xx. Tested on m5253, m5282, m5484 and m54418; compile tested only for the rest of the currenly supported coldfire targets.

Steven King (4):
add to_irq function so we can map gpios to external interrupts. setting the gpio data direction register to output doesn't dependent
upon the value to output!
Make everything thats not exported, static.
Implement gpio support for m54xx.

 arch/m68k/include/asm/m54xxsim.h   |   12 +++++++++---
 arch/m68k/include/asm/mcfgpio.h    |   12 ++++++++----
 arch/m68k/platform/coldfire/gpio.c |   34 +++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 18 deletions(-)

Signature · 1 line

1.7.10.4

[email protected]

author

2014-05-21 23:00 UTC #1 lore raw

PATCH 1/4 add to_irq function so we can map gpios to external interrupts.

From: Steven King <[email protected]>

---
 arch/m68k/platform/coldfire/gpio.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

arch/m68k/platform/coldfire/gpio.c +13 -0

index 9cd2b5c..ab9ac41 100644
@@ -147,6 +147,18 @@ void mcfgpio_free(struct gpio_chip *chip, unsigned offset)
 	__mcfgpio_free(offset);
 }
 
+int mcfgpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+#if defined(MCFGPIO_IRQ_MIN)
+	if ((offset >= MCFGPIO_IRQ_MIN) && (offset < MCFGPIO_IRQ_MAX))
+#else
+	if (offset < MCFGPIO_IRQ_MAX)
+#endif
+		return MCFGPIO_IRQ_VECBASE + offset;
+	else
+		return -EINVAL;
+}
+
 struct bus_type mcfgpio_subsys = {
 	.name		= "gpio",
 	.dev_name	= "gpio",

@@ -160,6 +172,7 @@ static struct gpio_chip mcfgpio_chip = {
 	.direction_output	= mcfgpio_direction_output,
 	.get			= mcfgpio_get_value,
 	.set			= mcfgpio_set_value,
+	.to_irq			= mcfgpio_to_irq,
 	.base			= 0,
 	.ngpio			= MCFGPIO_PIN_MAX,
 };

Signature · 1 line

1.7.10.4

[email protected]

author

2014-05-21 23:00 UTC #2 lore raw

PATCH 2/4 setting the gpio data direction register to output doesn't dependent upon the value to output!

In reply to [email protected]

From: Steven King <[email protected]>

---
 arch/m68k/platform/coldfire/gpio.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

arch/m68k/platform/coldfire/gpio.c +1 -4

index ab9ac41..e168886 100644
@@ -76,10 +76,7 @@ int __mcfgpio_direction_output(unsigned gpio, int value)
 
 	local_irq_save(flags);
 	data = mcfgpio_read(__mcfgpio_pddr(gpio));
-	if (value)
-		data |= mcfgpio_bit(gpio);
-	else
-		data &= mcfgpio_bit(gpio);
+	data |= mcfgpio_bit(gpio);
 	mcfgpio_write(data, __mcfgpio_pddr(gpio));
 
 	/* now set the data to output */

Signature · 1 line

1.7.10.4

[email protected]

author

2014-05-21 23:00 UTC #3 lore raw

PATCH 3/4 Make everything thats not exported, static.

In reply to [email protected]

From: Steven King <[email protected]>

---
 arch/m68k/platform/coldfire/gpio.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

arch/m68k/platform/coldfire/gpio.c +10 -8

index e168886..e7e4286 100644
@@ -114,37 +114,39 @@ EXPORT_SYMBOL(__mcfgpio_free);
 
 #ifdef CONFIG_GPIOLIB
 
-int mcfgpio_direction_input(struct gpio_chip *chip, unsigned offset)
+static int mcfgpio_direction_input(struct gpio_chip *chip, unsigned offset)
 {
 	return __mcfgpio_direction_input(offset);
 }
 
-int mcfgpio_get_value(struct gpio_chip *chip, unsigned offset)
+static int mcfgpio_get_value(struct gpio_chip *chip, unsigned offset)
 {
 	return __mcfgpio_get_value(offset);
 }
 
-int mcfgpio_direction_output(struct gpio_chip *chip, unsigned offset, int value)
+static int mcfgpio_direction_output(struct gpio_chip *chip, unsigned offset,
+				    int value)
 {
 	return __mcfgpio_direction_output(offset, value);
 }
 
-void mcfgpio_set_value(struct gpio_chip *chip, unsigned offset, int value)
+static void mcfgpio_set_value(struct gpio_chip *chip, unsigned offset,
+			      int value)
 {
 	__mcfgpio_set_value(offset, value);
 }
 
-int mcfgpio_request(struct gpio_chip *chip, unsigned offset)
+static int mcfgpio_request(struct gpio_chip *chip, unsigned offset)
 {
 	return __mcfgpio_request(offset);
 }
 
-void mcfgpio_free(struct gpio_chip *chip, unsigned offset)
+static void mcfgpio_free(struct gpio_chip *chip, unsigned offset)
 {
 	__mcfgpio_free(offset);
 }
 
-int mcfgpio_to_irq(struct gpio_chip *chip, unsigned offset)
+static int mcfgpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 #if defined(MCFGPIO_IRQ_MIN)
 	if ((offset >= MCFGPIO_IRQ_MIN) && (offset < MCFGPIO_IRQ_MAX))

@@ -156,7 +158,7 @@ int mcfgpio_to_irq(struct gpio_chip *chip, unsigned offset)
 		return -EINVAL;
 }
 
-struct bus_type mcfgpio_subsys = {
+static struct bus_type mcfgpio_subsys = {
 	.name		= "gpio",
 	.dev_name	= "gpio",
 };

Signature · 1 line

1.7.10.4

[email protected]

author

2014-05-21 23:00 UTC #4 lore raw

PATCH 4/4 Implement gpio support for m54xx.

In reply to [email protected]

From: Steven King <[email protected]>

---
 arch/m68k/include/asm/m54xxsim.h |   12 +++++++++---
 arch/m68k/include/asm/mcfgpio.h  |   12 ++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

arch/m68k/include/asm/m54xxsim.h +9 -3

index d3bd838..a5fbd17 100644
@@ -55,9 +55,15 @@
 /*
  *	Generic GPIO support
  */
-#define MCFGPIO_PIN_MAX		0	/* I am too lazy to count */
-#define MCFGPIO_IRQ_MAX		-1
-#define MCFGPIO_IRQ_VECBASE	-1
+#define MCFGPIO_PODR		(MCF_MBAR + 0xA00)
+#define MCFGPIO_PDDR		(MCF_MBAR + 0xA10)
+#define MCFGPIO_PPDR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_SETR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_CLRR		(MCF_MBAR + 0xA30)
+
+#define MCFGPIO_PIN_MAX		136	/* 128 gpio + 8 eport */
+#define MCFGPIO_IRQ_MAX		8
+#define MCFGPIO_IRQ_VECBASE	MCFINT_VECBASE
 
 /*
  *	EDGE Port support.

arch/m68k/include/asm/mcfgpio.h +8 -4

index c41ebf4..66203c3 100644
@@ -139,7 +139,8 @@ static inline void gpio_free(unsigned gpio)
 
 #if defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
     defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-    defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+    defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+    defined(CONFIG_M5441x)
 /*
  * These parts have an 'Edge' Port module (external interrupt/GPIO) which uses
  * read-modify-write to change an output and a GPIO module which has separate

@@ -195,7 +196,8 @@ static inline u32 __mcfgpio_ppdr(unsigned gpio)
 		return MCFSIM2_GPIO1READ;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)
 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPPDR;

@@ -237,7 +239,8 @@ static inline u32 __mcfgpio_podr(unsigned gpio)
 		return MCFSIM2_GPIO1WRITE;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)
 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPDR;

@@ -279,7 +282,8 @@ static inline u32 __mcfgpio_pddr(unsigned gpio)
 		return MCFSIM2_GPIO1ENABLE;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)
 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPDDR;

Signature · 1 line

1.7.10.4

Greg Ungerer

2014-05-22 01:29 UTC #5 lore raw

Re: [PATCH 4/4] Implement gpio support for m54xx.

Quoted 45 lines · On 22/05/14 09:00, [email protected] wrote:

From: Steven King <[email protected]>

---
 arch/m68k/include/asm/m54xxsim.h |   12 +++++++++---
 arch/m68k/include/asm/mcfgpio.h  |   12 ++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

arch/m68k/include/asm/m54xxsim.h +9 -3

index d3bd838..a5fbd17 100644
@@ -55,9 +55,15 @@
 /*
  *	Generic GPIO support
  */
-#define MCFGPIO_PIN_MAX		0	/* I am too lazy to count */
-#define MCFGPIO_IRQ_MAX		-1
-#define MCFGPIO_IRQ_VECBASE	-1
+#define MCFGPIO_PODR		(MCF_MBAR + 0xA00)
+#define MCFGPIO_PDDR		(MCF_MBAR + 0xA10)
+#define MCFGPIO_PPDR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_SETR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_CLRR		(MCF_MBAR + 0xA30)
+
+#define MCFGPIO_PIN_MAX		136	/* 128 gpio + 8 eport */
+#define MCFGPIO_IRQ_MAX		8
+#define MCFGPIO_IRQ_VECBASE	MCFINT_VECBASE
 
 /*
  *	EDGE Port support.

arch/m68k/include/asm/mcfgpio.h +2 -1

index c41ebf4..66203c3 100644
@@ -139,7 +139,8 @@ static inline void gpio_free(unsigned gpio)
 
 #if defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
     defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-    defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+    defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+    defined(CONFIG_M5441x)
 /*
  * These parts have an 'Edge' Port module (external interrupt/GPIO) which uses
  * read-modify-write to change an output and a GPIO module which has separate

arch/m68k/include/asm/mcfgpio.h static inline u32 __mcfgpio_ppdr(unsigned gpio) · line 200

Quoted 6 lines

arch/m68k/include/asm/mcfgpio.h +2 -1

@@ -195,7 +196,8 @@ static inline u32 __mcfgpio_ppdr(unsigned gpio)
 		return MCFSIM2_GPIO1READ;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)

Not proposing for this patch... But I wonder if this could be done a little more cleanly by basing the #elif on the presence of the MCFEPORT_EPPDR definition.

Same goes for the other #ifdef/#elif clauses.

Just a thought.

Regards
Greg

Quoted 24 lines

arch/m68k/include/asm/mcfgpio.h +4 -2

 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPPDR;

@@ -237,7 +239,8 @@ static inline u32 __mcfgpio_podr(unsigned gpio)
 		return MCFSIM2_GPIO1WRITE;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)
 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPDR;

@@ -279,7 +282,8 @@ static inline u32 __mcfgpio_pddr(unsigned gpio)
 		return MCFSIM2_GPIO1ENABLE;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)
 #if !defined(CONFIG_M5441x)
 	if (gpio < 8)
 		return MCFEPORT_EPDDR;

Steven King

author

2014-05-22 02:58 UTC #6 lore raw

Re: [PATCH 4/4] Implement gpio support for m54xx.

Quoted 57 lines · Greg Ungerer

Quoted 45 lines · On 22/05/14 09:00, [email protected] wrote:

From: Steven King <[email protected]>

---
 arch/m68k/include/asm/m54xxsim.h |   12 +++++++++---
 arch/m68k/include/asm/mcfgpio.h  |   12 ++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

arch/m68k/include/asm/m54xxsim.h +9 -3

index d3bd838..a5fbd17 100644
@@ -55,9 +55,15 @@
 /*
  *	Generic GPIO support
  */
-#define MCFGPIO_PIN_MAX		0	/* I am too lazy to count */
-#define MCFGPIO_IRQ_MAX		-1
-#define MCFGPIO_IRQ_VECBASE	-1
+#define MCFGPIO_PODR		(MCF_MBAR + 0xA00)
+#define MCFGPIO_PDDR		(MCF_MBAR + 0xA10)
+#define MCFGPIO_PPDR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_SETR		(MCF_MBAR + 0xA20)
+#define MCFGPIO_CLRR		(MCF_MBAR + 0xA30)
+
+#define MCFGPIO_PIN_MAX		136	/* 128 gpio + 8 eport */
+#define MCFGPIO_IRQ_MAX		8
+#define MCFGPIO_IRQ_VECBASE	MCFINT_VECBASE
 
 /*
  *	EDGE Port support.

arch/m68k/include/asm/mcfgpio.h +2 -1

index c41ebf4..66203c3 100644
@@ -139,7 +139,8 @@ static inline void gpio_free(unsigned gpio)
 
 #if defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
     defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-    defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+    defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+    defined(CONFIG_M5441x)
 /*
  * These parts have an 'Edge' Port module (external interrupt/GPIO) which uses
  * read-modify-write to change an output and a GPIO module which has separate

arch/m68k/include/asm/mcfgpio.h static inline u32 __mcfgpio_ppdr(unsigned gpio) · line 200

Quoted 6 lines

arch/m68k/include/asm/mcfgpio.h +2 -1

@@ -195,7 +196,8 @@ static inline u32 __mcfgpio_ppdr(unsigned gpio)
 		return MCFSIM2_GPIO1READ;
 #elif defined(CONFIG_M520x) || defined(CONFIG_M523x) || \
       defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
-      defined(CONFIG_M53xx) || defined(CONFIG_M5441x)
+      defined(CONFIG_M53xx) || defined(CONFIG_M54xx) || \
+      defined(CONFIG_M5441x)

Not proposing for this patch... But I wonder if this could be done a little more cleanly by basing the #elif on the presence of the MCFEPORT_EPPDR definition.

Quoted 3 lines

Same goes for the other #ifdef/#elif clauses.
Just a thought.

Hmm, I like this. I like the idea of a check for the presence of a feature rather than a config option there, then all that would be needed to add support for a new processor (m54445?) would be to make sure the correct definitions are present in the m5xxxsim.h. I'll take a look at it...

Geert Uytterhoeven

maintainer

2014-05-22 06:39 UTC #7 lore raw

Re: [PATCH 4/4] Implement gpio support for m54xx.

In reply to [email protected]

On Thu, May 22, 2014 at 1:00 AM,  <[email protected]> wrote:
> +#define MCFGPIO_PODR           (MCF_MBAR + 0xA00)
> +#define MCFGPIO_PDDR           (MCF_MBAR + 0xA10)
> +#define MCFGPIO_PPDR           (MCF_MBAR + 0xA20)
> +#define MCFGPIO_SETR           (MCF_MBAR + 0xA20)

The above two are identical. Is that intentional?

Quoted 1 line

+#define MCFGPIO_CLRR (MCF_MBAR + 0xA30)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Steven King

author

2014-05-22 13:46 UTC #8 lore raw

Re: [PATCH 4/4] Implement gpio support for m54xx.

Quoted 7 lines · Geert Uytterhoeven

On Thu, May 22, 2014 at 1:00 AM,  <[email protected]> wrote:
> +#define MCFGPIO_PODR           (MCF_MBAR + 0xA00)
> +#define MCFGPIO_PDDR           (MCF_MBAR + 0xA10)
> +#define MCFGPIO_PPDR           (MCF_MBAR + 0xA20)
> +#define MCFGPIO_SETR           (MCF_MBAR + 0xA20)

The above two are identical. Is that intentional?

Yes. Functionally, the PPDRs are read-only registers that when read, return the current state of the pins and the SETRs are write-only registers that for each pin configured as an output, if the corresponding bit in the write is a 1, the pin is driven high, and if the bit is 0, no effect.

Greg Ungerer

2014-05-22 01:29 UTC #9 lore raw

In reply to [email protected]

Hi Steven,

Quoted 7 lines · On 22/05/14 09:00, [email protected] wrote:

From: Steven King <[email protected]>
I had these queued up, forgot to send. This fixes a heinous bug in mcfgpio_direction_output, adds code to enable gpio_to_irq functionality, removes some of the global namespace pollution in gpio.c and adds gpio support for the m54xx. Tested on m5253, m5282, m5484 and m54418; compile tested only for the rest of the currenly supported coldfire targets.

All looks good to me. Can I add your:

Singed-off-by Steven King <[email protected]>

Thanks
Greg

Quoted 12 lines

Steven King (4):
add to_irq function so we can map gpios to external interrupts. setting the gpio data direction register to output doesn't dependent
upon the value to output!
Make everything thats not exported, static.
Implement gpio support for m54xx.
 arch/m68k/include/asm/m54xxsim.h   |   12 +++++++++---
 arch/m68k/include/asm/mcfgpio.h    |   12 ++++++++----
 arch/m68k/platform/coldfire/gpio.c |   34 +++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 18 deletions(-)

Steven King

author

2014-05-22 02:06 UTC #10 lore raw

Quoted 11 lines · Greg Ungerer

Hi Steven,
Quoted 7 lines · On 22/05/14 09:00, [email protected] wrote:
From: Steven King <[email protected]>
I had these queued up, forgot to send. This fixes a heinous bug in mcfgpio_direction_output, adds code to enable gpio_to_irq functionality, removes some of the global namespace pollution in gpio.c and adds gpio support for the m54xx. Tested on m5253, m5282, m5484 and m54418; compile tested only for the rest of the currenly supported coldfire targets.

Quoted 3 lines

All looks good to me. Can I add your:
Singed-off-by Steven King <[email protected]>

D'oh! Sure, you can do that ;)