[U-Boot] [PATCH 0/4] gpio: atmel: fix code to use pointer for pio port
This patch set fix code to use pointer for pio port as the warning message suggested, remove the warning message remove unused at91_pio structure definition add common gpio API add copyright and remove error header info
runtime testing on at91sam9x5ek and sama5d3xek board compile testing for all atmel ek board
Bo Shen (4): gpio: atmel: fix code to use pointer for pio port gpio: atmel: remove the at91_pio definition gpio: atmel: add gpio common API support gpio: atmel: add copyright and remove error header info
arch/arm/include/asm/arch-at91/at91_pio.h | 15 -- drivers/gpio/at91_gpio.c | 277 ++++++++++++++++++----------- 2 files changed, 178 insertions(+), 114 deletions(-)
fix code to use pointer for pio port as the warning message suggested remove the warning message
Signed-off-by: Bo Shen voice.shen@atmel.com --- drivers/gpio/at91_gpio.c | 232 ++++++++++++++++++++++++++-------------------- 1 file changed, 134 insertions(+), 98 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 2322914..15f396f 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -8,16 +8,6 @@ * SPDX-License-Identifier: GPL-2.0+ */
-/* - * WARNING: - * - * As the code is right now, it expects all PIO ports A,B,C,... - * to be evenly spaced in the memory map: - * ATMEL_BASE_PIOA + port * sizeof at91pio_t - * This might not necessaryly be true in future Atmel SoCs. - * This code should be fixed to use a pointer array to the ports. - */ - #include <config.h> #include <common.h> #include <asm/io.h> @@ -25,19 +15,52 @@ #include <asm/arch/hardware.h> #include <asm/arch/at91_pio.h>
+static unsigned at91_pio_get_port(unsigned port) +{ + unsigned at91_port; + + switch (port) { + case AT91_PIO_PORTA: + at91_port = ATMEL_BASE_PIOA; + break; + case AT91_PIO_PORTB: + at91_port = ATMEL_BASE_PIOB; + break; + case AT91_PIO_PORTC: + at91_port = ATMEL_BASE_PIOC; + break; + #if (ATMEL_PIO_PORTS > 3) + case AT91_PIO_PORTD: + at91_port = ATMEL_BASE_PIOD; + break; + #endif + #if (ATMEL_PIO_PORTS > 4) + case AT91_PIO_PORTE: + at91_port = ATMEL_BASE_PIOE; + break; + #endif + default: + at91_port = 0; + break; + } + + return at91_port; +} + int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; if (use_pullup) - writel(1 << pin, &pio->port[port].puer); + writel(1 << pin, &at91_port->puer); else - writel(1 << pin, &pio->port[port].pudr); - writel(mask, &pio->port[port].per); + writel(1 << pin, &at91_port->pudr); + writel(mask, &at91_port->per); } + return 0; }
@@ -46,15 +69,16 @@ int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) */ int at91_set_pio_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->per); } + return 0; }
@@ -63,23 +87,24 @@ int at91_set_pio_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_a_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); #if defined(CPU_HAS_PIO3) - writel(readl(&pio->port[port].abcdsr1) & ~mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) & ~mask, - &pio->port[port].abcdsr2); + writel(readl(&at91_port->abcdsr1) & ~mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) & ~mask, + &at91_port->abcdsr2); #else - writel(mask, &pio->port[port].asr); + writel(mask, &at91_port->asr); #endif - writel(mask, &pio->port[port].pdr); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -88,23 +113,24 @@ int at91_set_a_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_b_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); #if defined(CPU_HAS_PIO3) - writel(readl(&pio->port[port].abcdsr1) | mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) & ~mask, - &pio->port[port].abcdsr2); + writel(readl(&at91_port->abcdsr1) | mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) & ~mask, + &at91_port->abcdsr2); #else - writel(mask, &pio->port[port].bsr); + writel(mask, &at91_port->bsr); #endif - writel(mask, &pio->port[port].pdr); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -114,19 +140,20 @@ int at91_set_b_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_c_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(readl(&pio->port[port].abcdsr1) & ~mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) | mask, - &pio->port[port].abcdsr2); - writel(mask, &pio->port[port].pdr); + writel(readl(&at91_port->abcdsr1) & ~mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) | mask, + &at91_port->abcdsr2); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -135,19 +162,20 @@ int at91_set_c_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_d_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(readl(&pio->port[port].abcdsr1) | mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) | mask, - &pio->port[port].abcdsr2); - writel(mask, &pio->port[port].pdr); + writel(readl(&at91_port->abcdsr1) | mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) | mask, + &at91_port->abcdsr2); + writel(mask, &at91_port->pdr); } + return 0; } #endif @@ -158,15 +186,15 @@ int at91_set_d_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_pio_input(unsigned port, u32 pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(mask, &pio->port[port].odr); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->odr); + writel(mask, &at91_port->per); } return 0; } @@ -177,20 +205,21 @@ int at91_set_pio_input(unsigned port, u32 pin, int use_pullup) */ int at91_set_pio_output(unsigned port, u32 pin, int value) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); - writel(mask, &pio->port[port].pudr); + writel(mask, &at91_port->idr); + writel(mask, &at91_port->pudr); if (value) - writel(mask, &pio->port[port].sodr); + writel(mask, &at91_port->sodr); else - writel(mask, &pio->port[port].codr); - writel(mask, &pio->port[port].oer); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->codr); + writel(mask, &at91_port->oer); + writel(mask, &at91_port->per); } + return 0; }
@@ -199,20 +228,21 @@ int at91_set_pio_output(unsigned port, u32 pin, int value) */ int at91_set_pio_deglitch(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; if (is_on) { #if defined(CPU_HAS_PIO3) - writel(mask, &pio->port[port].ifscdr); + writel(mask, &at91_port->ifscdr); #endif - writel(mask, &pio->port[port].ifer); + writel(mask, &at91_port->ifer); } else { - writel(mask, &pio->port[port].ifdr); + writel(mask, &at91_port->ifdr); } } + return 0; }
@@ -222,19 +252,20 @@ int at91_set_pio_deglitch(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_debounce(unsigned port, unsigned pin, int is_on, int div) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; if (is_on) { - writel(mask, &pio->port[port].ifscer); - writel(div & PIO_SCDR_DIV, &pio->port[port].scdr); - writel(mask, &pio->port[port].ifer); + writel(mask, &at91_port->ifscer); + writel(div & PIO_SCDR_DIV, &at91_port->scdr); + writel(mask, &at91_port->ifer); } else { - writel(mask, &pio->port[port].ifdr); + writel(mask, &at91_port->ifdr); } } + return 0; }
@@ -244,17 +275,18 @@ int at91_set_pio_debounce(unsigned port, unsigned pin, int is_on, int div) */ int at91_set_pio_pulldown(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].pudr); + writel(mask, &at91_port->pudr); if (is_on) - writel(mask, &pio->port[port].ppder); + writel(mask, &at91_port->ppder); else - writel(mask, &pio->port[port].ppddr); + writel(mask, &at91_port->ppddr); } + return 0; }
@@ -263,14 +295,15 @@ int at91_set_pio_pulldown(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_disable_schmitt_trig(unsigned port, unsigned pin) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(readl(&pio->port[port].schmitt) | mask, - &pio->port[port].schmitt); + writel(readl(&at91_port->schmitt) | mask, + &at91_port->schmitt); } + return 0; } #endif @@ -281,16 +314,17 @@ int at91_set_pio_disable_schmitt_trig(unsigned port, unsigned pin) */ int at91_set_pio_multi_drive(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; if (is_on) - writel(mask, &pio->port[port].mder); + writel(mask, &at91_port->mder); else - writel(mask, &pio->port[port].mddr); + writel(mask, &at91_port->mddr); } + return 0; }
@@ -299,16 +333,17 @@ int at91_set_pio_multi_drive(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_value(unsigned port, unsigned pin, int value) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; if (value) - writel(mask, &pio->port[port].sodr); + writel(mask, &at91_port->sodr); else - writel(mask, &pio->port[port].codr); + writel(mask, &at91_port->codr); } + return 0; }
@@ -317,13 +352,14 @@ int at91_set_pio_value(unsigned port, unsigned pin, int value) */ int at91_get_pio_value(unsigned port, unsigned pin) { - u32 pdsr = 0; - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + u32 pdsr = 0; + at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - pdsr = readl(&pio->port[port].pdsr) & mask; + pdsr = readl(&at91_port->pdsr) & mask; } + return pdsr != 0; }
Hi Bo,
On 08/13/2013 08:38 AM, Bo Shen wrote:
fix code to use pointer for pio port as the warning message suggested remove the warning message
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 232 ++++++++++++++++++++++++++-------------------- 1 file changed, 134 insertions(+), 98 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 2322914..15f396f 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -8,16 +8,6 @@
- SPDX-License-Identifier: GPL-2.0+
*/
-/*
- WARNING:
- As the code is right now, it expects all PIO ports A,B,C,...
- to be evenly spaced in the memory map:
- ATMEL_BASE_PIOA + port * sizeof at91pio_t
- This might not necessaryly be true in future Atmel SoCs.
- This code should be fixed to use a pointer array to the ports.
- */
#include <config.h> #include <common.h> #include <asm/io.h> @@ -25,19 +15,52 @@ #include <asm/arch/hardware.h> #include <asm/arch/at91_pio.h>
+static unsigned at91_pio_get_port(unsigned port) +{
- unsigned at91_port;
- switch (port) {
- case AT91_PIO_PORTA:
at91_port = ATMEL_BASE_PIOA;break;- case AT91_PIO_PORTB:
at91_port = ATMEL_BASE_PIOB;break;- case AT91_PIO_PORTC:
at91_port = ATMEL_BASE_PIOC;break;- #if (ATMEL_PIO_PORTS > 3)
fix indention
- case AT91_PIO_PORTD:
at91_port = ATMEL_BASE_PIOD;break;- #endif
- #if (ATMEL_PIO_PORTS > 4)
nit ... if ATMEL_PIO_PORTS is > 4 it also matches '>3'
if >3 if >4 endif endif
- case AT91_PIO_PORTE:
at91_port = ATMEL_BASE_PIOE;break;- #endif
- default:
at91_port = 0;break;- }
- return at91_port;
+}
int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) {
- at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA;
- u32 mask;
- at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port);
This cast here is annoying, can't we just change the return type of at91_pio_get_port()?
u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) {
if (at91_port && (pin < 32))
The logic for correct range of port is delegated to at91_pio_get_port()
mask = 1 << pin; if (use_pullup)
writel(1 << pin, &pio->port[port].puer);
writel(1 << pin, &at91_port->puer);
writel(1 << pin, &pio->port[port].pudr);writel(mask, &pio->port[port].per);
writel(1 << pin, &at91_port->pudr);writel(mask, &at91_port->per);
I wonder if we should break the current usage and return another value (-ENODEV ?) on error.
return 0; }
<snip>
Please adopt all places in this file with mentioned changes and tell me your opinion about erroneous return value.
Best regards
Andreas Bießmann
Hi Andreas,
On 8/21/2013 23:08, Andreas Bießmann wrote:
Hi Bo,
On 08/13/2013 08:38 AM, Bo Shen wrote:
fix code to use pointer for pio port as the warning message suggested remove the warning message
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 232 ++++++++++++++++++++++++++-------------------- 1 file changed, 134 insertions(+), 98 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 2322914..15f396f 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -8,16 +8,6 @@
- SPDX-License-Identifier: GPL-2.0+
*/
-/*
- WARNING:
- As the code is right now, it expects all PIO ports A,B,C,...
- to be evenly spaced in the memory map:
- ATMEL_BASE_PIOA + port * sizeof at91pio_t
- This might not necessaryly be true in future Atmel SoCs.
- This code should be fixed to use a pointer array to the ports.
- */
- #include <config.h> #include <common.h> #include <asm/io.h>
@@ -25,19 +15,52 @@ #include <asm/arch/hardware.h> #include <asm/arch/at91_pio.h>
+static unsigned at91_pio_get_port(unsigned port) +{
- unsigned at91_port;
- switch (port) {
- case AT91_PIO_PORTA:
at91_port = ATMEL_BASE_PIOA;break;- case AT91_PIO_PORTB:
at91_port = ATMEL_BASE_PIOB;break;- case AT91_PIO_PORTC:
at91_port = ATMEL_BASE_PIOC;break;- #if (ATMEL_PIO_PORTS > 3)
fix indention
OK. Thanks.
- case AT91_PIO_PORTD:
at91_port = ATMEL_BASE_PIOD;break;- #endif
- #if (ATMEL_PIO_PORTS > 4)
nit ... if ATMEL_PIO_PORTS is > 4 it also matches '>3'
if >3 if >4 endif endif
OK, I will change style as is.
- case AT91_PIO_PORTE:
at91_port = ATMEL_BASE_PIOE;break;- #endif
- default:
at91_port = 0;break;- }
- return at91_port;
+}
- int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) {
- at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA;
- u32 mask;
- at91_port_t *at91_port = (at91_port_t *)at91_pio_get_port(port);
This cast here is annoying, can't we just change the return type of at91_pio_get_port()?
I will change the return type of at91_pio_get_port().
u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) {
if (at91_port && (pin < 32))
The logic for correct range of port is delegated to at91_pio_get_port()
Yes, this check should be in at91_pio_get_port();
mask = 1 << pin; if (use_pullup)
writel(1 << pin, &pio->port[port].puer);
writel(1 << pin, &at91_port->puer);
writel(1 << pin, &pio->port[port].pudr);writel(mask, &pio->port[port].per);
writel(1 << pin, &at91_port->pudr);writel(mask, &at91_port->per);I wonder if we should break the current usage and return another value (-ENODEV ?) on error.
return 0; }
<snip>
Please adopt all places in this file with mentioned changes and tell me your opinion about erroneous return value.
For the erroneous return value, actually we never check it (consider it always successfully). So, I think now we just keep it as is, and consider a proper method after this patch set. Would it be OK?
Best regards
Andreas Bießmann
Best Regards, Bo Shen
Hi Bo,
On 22.08.13 05:15, Bo Shen wrote:
Hi Andreas,
On 8/21/2013 23:08, Andreas Bießmann wrote:
Hi Bo,
On 08/13/2013 08:38 AM, Bo Shen wrote:
<snip>
Please adopt all places in this file with mentioned changes and tell me your opinion about erroneous return value.
For the erroneous return value, actually we never check it (consider it always successfully). So, I think now we just keep it as is, and consider a proper method after this patch set. Would it be OK?
I'm fine with that. If no one else complains I'll try to push this into -rc2 for this release. The API change would then come for next release, am I right?
Best regards
Andreas Bießmann
Hi Andreas,
On 8/22/2013 14:26, Andreas Bießmann wrote:
Please adopt all places in this file with mentioned changes and tell me
your opinion about erroneous return value.
For the erroneous return value, actually we never check it (consider it always successfully). So, I think now we just keep it as is, and consider a proper method after this patch set. Would it be OK?
I'm fine with that. If no one else complains I'll try to push this into -rc2 for this release. The API change would then come for next release, am I right?
I will prepare the v2 for this patch, after test OK and compile testing for all Atmel EK board, I will send out the patch.
If this series can go into this release will be better. As if without the common GPIO API patch applied, we can not use gpio command, software i2c support and etc.
Best regards
Andreas Bießmann
Best Regards, Bo Shen
fix code to use pointer for pio port as the warning message suggested remove the warning message
Signed-off-by: Bo Shen voice.shen@atmel.com
--- Changes in v2: - fix indention - change the return type of at91_pio_get_port() to avoid many cast
drivers/gpio/at91_gpio.c | 250 +++++++++++++++++++++++++--------------------- 1 file changed, 138 insertions(+), 112 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 2322914..6d227d3 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -8,16 +8,6 @@ * SPDX-License-Identifier: GPL-2.0+ */
-/* - * WARNING: - * - * As the code is right now, it expects all PIO ports A,B,C,... - * to be evenly spaced in the memory map: - * ATMEL_BASE_PIOA + port * sizeof at91pio_t - * This might not necessaryly be true in future Atmel SoCs. - * This code should be fixed to use a pointer array to the ports. - */ - #include <config.h> #include <common.h> #include <asm/io.h> @@ -25,19 +15,42 @@ #include <asm/arch/hardware.h> #include <asm/arch/at91_pio.h>
+static struct at91_port *at91_pio_get_port(unsigned port) +{ + switch (port) { + case AT91_PIO_PORTA: + return (struct at91_port *)ATMEL_BASE_PIOA; + case AT91_PIO_PORTB: + return (struct at91_port *)ATMEL_BASE_PIOB; + case AT91_PIO_PORTC: + return (struct at91_port *)ATMEL_BASE_PIOC; +#if (ATMEL_PIO_PORTS > 3) + case AT91_PIO_PORTD: + return (struct at91_port *)ATMEL_BASE_PIOD; +#if (ATMEL_PIO_PORTS > 4) + case AT91_PIO_PORTE: + return (struct at91_port *)ATMEL_BASE_PIOE; +#endif +#endif + default: + return NULL; + } +} + int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; if (use_pullup) - writel(1 << pin, &pio->port[port].puer); + writel(1 << pin, &at91_port->puer); else - writel(1 << pin, &pio->port[port].pudr); - writel(mask, &pio->port[port].per); + writel(1 << pin, &at91_port->pudr); + writel(mask, &at91_port->per); } + return 0; }
@@ -46,15 +59,16 @@ int at91_set_pio_pullup(unsigned port, unsigned pin, int use_pullup) */ int at91_set_pio_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->per); } + return 0; }
@@ -63,23 +77,24 @@ int at91_set_pio_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_a_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); #if defined(CPU_HAS_PIO3) - writel(readl(&pio->port[port].abcdsr1) & ~mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) & ~mask, - &pio->port[port].abcdsr2); + writel(readl(&at91_port->abcdsr1) & ~mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) & ~mask, + &at91_port->abcdsr2); #else - writel(mask, &pio->port[port].asr); + writel(mask, &at91_port->asr); #endif - writel(mask, &pio->port[port].pdr); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -88,23 +103,24 @@ int at91_set_a_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_b_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); #if defined(CPU_HAS_PIO3) - writel(readl(&pio->port[port].abcdsr1) | mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) & ~mask, - &pio->port[port].abcdsr2); + writel(readl(&at91_port->abcdsr1) | mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) & ~mask, + &at91_port->abcdsr2); #else - writel(mask, &pio->port[port].bsr); + writel(mask, &at91_port->bsr); #endif - writel(mask, &pio->port[port].pdr); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -114,19 +130,20 @@ int at91_set_b_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_c_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(readl(&pio->port[port].abcdsr1) & ~mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) | mask, - &pio->port[port].abcdsr2); - writel(mask, &pio->port[port].pdr); + writel(readl(&at91_port->abcdsr1) & ~mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) | mask, + &at91_port->abcdsr2); + writel(mask, &at91_port->pdr); } + return 0; }
@@ -135,19 +152,20 @@ int at91_set_c_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_d_periph(unsigned port, unsigned pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(readl(&pio->port[port].abcdsr1) | mask, - &pio->port[port].abcdsr1); - writel(readl(&pio->port[port].abcdsr2) | mask, - &pio->port[port].abcdsr2); - writel(mask, &pio->port[port].pdr); + writel(readl(&at91_port->abcdsr1) | mask, + &at91_port->abcdsr1); + writel(readl(&at91_port->abcdsr2) | mask, + &at91_port->abcdsr2); + writel(mask, &at91_port->pdr); } + return 0; } #endif @@ -158,16 +176,17 @@ int at91_set_d_periph(unsigned port, unsigned pin, int use_pullup) */ int at91_set_pio_input(unsigned port, u32 pin, int use_pullup) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); + writel(mask, &at91_port->idr); at91_set_pio_pullup(port, pin, use_pullup); - writel(mask, &pio->port[port].odr); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->odr); + writel(mask, &at91_port->per); } + return 0; }
@@ -177,20 +196,21 @@ int at91_set_pio_input(unsigned port, u32 pin, int use_pullup) */ int at91_set_pio_output(unsigned port, u32 pin, int value) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].idr); - writel(mask, &pio->port[port].pudr); + writel(mask, &at91_port->idr); + writel(mask, &at91_port->pudr); if (value) - writel(mask, &pio->port[port].sodr); + writel(mask, &at91_port->sodr); else - writel(mask, &pio->port[port].codr); - writel(mask, &pio->port[port].oer); - writel(mask, &pio->port[port].per); + writel(mask, &at91_port->codr); + writel(mask, &at91_port->oer); + writel(mask, &at91_port->per); } + return 0; }
@@ -199,20 +219,21 @@ int at91_set_pio_output(unsigned port, u32 pin, int value) */ int at91_set_pio_deglitch(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; if (is_on) { #if defined(CPU_HAS_PIO3) - writel(mask, &pio->port[port].ifscdr); + writel(mask, &at91_port->ifscdr); #endif - writel(mask, &pio->port[port].ifer); + writel(mask, &at91_port->ifer); } else { - writel(mask, &pio->port[port].ifdr); + writel(mask, &at91_port->ifdr); } } + return 0; }
@@ -222,19 +243,20 @@ int at91_set_pio_deglitch(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_debounce(unsigned port, unsigned pin, int is_on, int div) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; if (is_on) { - writel(mask, &pio->port[port].ifscer); - writel(div & PIO_SCDR_DIV, &pio->port[port].scdr); - writel(mask, &pio->port[port].ifer); + writel(mask, &at91_port->ifscer); + writel(div & PIO_SCDR_DIV, &at91_port->scdr); + writel(mask, &at91_port->ifer); } else { - writel(mask, &pio->port[port].ifdr); + writel(mask, &at91_port->ifdr); } } + return 0; }
@@ -244,17 +266,18 @@ int at91_set_pio_debounce(unsigned port, unsigned pin, int is_on, int div) */ int at91_set_pio_pulldown(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(mask, &pio->port[port].pudr); + writel(mask, &at91_port->pudr); if (is_on) - writel(mask, &pio->port[port].ppder); + writel(mask, &at91_port->ppder); else - writel(mask, &pio->port[port].ppddr); + writel(mask, &at91_port->ppddr); } + return 0; }
@@ -263,14 +286,15 @@ int at91_set_pio_pulldown(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_disable_schmitt_trig(unsigned port, unsigned pin) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - writel(readl(&pio->port[port].schmitt) | mask, - &pio->port[port].schmitt); + writel(readl(&at91_port->schmitt) | mask, + &at91_port->schmitt); } + return 0; } #endif @@ -281,16 +305,17 @@ int at91_set_pio_disable_schmitt_trig(unsigned port, unsigned pin) */ int at91_set_pio_multi_drive(unsigned port, unsigned pin, int is_on) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; if (is_on) - writel(mask, &pio->port[port].mder); + writel(mask, &at91_port->mder); else - writel(mask, &pio->port[port].mddr); + writel(mask, &at91_port->mddr); } + return 0; }
@@ -299,16 +324,17 @@ int at91_set_pio_multi_drive(unsigned port, unsigned pin, int is_on) */ int at91_set_pio_value(unsigned port, unsigned pin, int value) { - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; if (value) - writel(mask, &pio->port[port].sodr); + writel(mask, &at91_port->sodr); else - writel(mask, &pio->port[port].codr); + writel(mask, &at91_port->codr); } + return 0; }
@@ -317,13 +343,13 @@ int at91_set_pio_value(unsigned port, unsigned pin, int value) */ int at91_get_pio_value(unsigned port, unsigned pin) { - u32 pdsr = 0; - at91_pio_t *pio = (at91_pio_t *) ATMEL_BASE_PIOA; - u32 mask; + struct at91_port *at91_port = at91_pio_get_port(port); + u32 pdsr = 0, mask;
- if ((port < ATMEL_PIO_PORTS) && (pin < 32)) { + if (at91_port && (pin < 32)) { mask = 1 << pin; - pdsr = readl(&pio->port[port].pdsr) & mask; + pdsr = readl(&at91_port->pdsr) & mask; } + return pdsr != 0; }
Dear Bo Shen,
Bo Shen voice.shen@atmel.com writes:
fix code to use pointer for pio port as the warning message suggested remove the warning message
Signed-off-by: Bo Shen voice.shen@atmel.com
Changes in v2:
- fix indention
- change the return type of at91_pio_get_port() to avoid many cast
drivers/gpio/at91_gpio.c | 250 +++++++++++++++++++++++++--------------------- 1 file changed, 138 insertions(+), 112 deletions(-)
applied to u-boot-atmel/master, thanks!
Best regards, Andreas Bießmann
the at91_pio definition is no longer needed, so remove it
Signed-off-by: Bo Shen voice.shen@atmel.com --- arch/arm/include/asm/arch-at91/at91_pio.h | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/arch/arm/include/asm/arch-at91/at91_pio.h b/arch/arm/include/asm/arch-at91/at91_pio.h index 676f024..ba61542 100644 --- a/arch/arm/include/asm/arch-at91/at91_pio.h +++ b/arch/arm/include/asm/arch-at91/at91_pio.h @@ -109,21 +109,6 @@ typedef struct at91_port { #endif } at91_port_t;
-typedef union at91_pio { - struct { - at91_port_t pioa; - at91_port_t piob; - at91_port_t pioc; - #if (ATMEL_PIO_PORTS > 3) - at91_port_t piod; - #endif - #if (ATMEL_PIO_PORTS > 4) - at91_port_t pioe; - #endif - } ; - at91_port_t port[ATMEL_PIO_PORTS]; -} at91_pio_t; - #ifdef CONFIG_AT91_GPIO int at91_set_a_periph(unsigned port, unsigned pin, int use_pullup); int at91_set_b_periph(unsigned port, unsigned pin, int use_pullup);
On 08/13/2013 08:38 AM, Bo Shen wrote:
the at91_pio definition is no longer needed, so remove it
Signed-off-by: Bo Shen voice.shen@atmel.com
arch/arm/include/asm/arch-at91/at91_pio.h | 15 --------------- 1 file changed, 15 deletions(-)
diff --git a/arch/arm/include/asm/arch-at91/at91_pio.h b/arch/arm/include/asm/arch-at91/at91_pio.h index 676f024..ba61542 100644 --- a/arch/arm/include/asm/arch-at91/at91_pio.h +++ b/arch/arm/include/asm/arch-at91/at91_pio.h @@ -109,21 +109,6 @@ typedef struct at91_port { #endif } at91_port_t;
-typedef union at91_pio {
- struct {
at91_port_t pioa;at91_port_t piob;at91_port_t pioc;- #if (ATMEL_PIO_PORTS > 3)
at91_port_t piod;- #endif
- #if (ATMEL_PIO_PORTS > 4)
at91_port_t pioe;- #endif
- } ;
- at91_port_t port[ATMEL_PIO_PORTS];
-} at91_pio_t;
NAK, this breaks at least 7 boards:
14: gpio: atmel: remove the at91_pio definition arm: + at91sam9263ek_dataflash_cs0 eb_cpux9k2 at91rm9200ek_ram cpuat91 vl_ma2sc vl_ma2sc_ram cpuat91_ram
We need to have this typedef while we have the new API and change the users of this struct to the new API after introducing.
Best regards
Andreas Bießmann
add gpio common API support for gpio command
Signed-off-by: Bo Shen voice.shen@atmel.com --- drivers/gpio/at91_gpio.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 15f396f..3de0844 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -363,3 +363,46 @@ int at91_get_pio_value(unsigned port, unsigned pin)
return pdsr != 0; } + +/* Common GPIO API */ + +#define at91_gpio_to_port(gpio) (gpio / 32) +#define at91_gpio_to_pin(gpio) (gpio % 32) + +int gpio_request(unsigned gpio, const char *label) +{ + return 0; +} + +int gpio_free(unsigned gpio) +{ + return 0; +} + +int gpio_direction_input(unsigned gpio) +{ + at91_set_pio_input(at91_gpio_to_port(gpio), + at91_gpio_to_pin(gpio), 0); + return 0; +} + +int gpio_direction_output(unsigned gpio, int value) +{ + at91_set_pio_output(at91_gpio_to_port(gpio), + at91_gpio_to_pin(gpio), value); + return 0; +} + +int gpio_get_value(unsigned gpio) +{ + return (int) at91_get_pio_value(at91_gpio_to_port(gpio), + at91_gpio_to_pin(gpio)); +} + +int gpio_set_value(unsigned gpio, int value) +{ + at91_set_pio_value(at91_gpio_to_port(gpio), + at91_gpio_to_pin(gpio), value); + + return 0; +}
On 08/13/2013 08:38 AM, Bo Shen wrote:
add gpio common API support for gpio command
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 15f396f..3de0844 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -363,3 +363,46 @@ int at91_get_pio_value(unsigned port, unsigned pin)
return pdsr != 0; }
+/* Common GPIO API */
+#define at91_gpio_to_port(gpio) (gpio / 32) +#define at91_gpio_to_pin(gpio) (gpio % 32)
+int gpio_request(unsigned gpio, const char *label) +{
- return 0;
+}
+int gpio_free(unsigned gpio) +{
- return 0;
+}
+int gpio_direction_input(unsigned gpio) +{
- at91_set_pio_input(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), 0);- return 0;
+}
+int gpio_direction_output(unsigned gpio, int value) +{
- at91_set_pio_output(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
+int gpio_get_value(unsigned gpio) +{
- return (int) at91_get_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio));
why cast to int here?
+}
+int gpio_set_value(unsigned gpio, int value) +{
- at91_set_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
Great, I love this. But wasn't there some define for generic GPIO? Shouldn't we encapsulate this API into this other define?
Best regards
Andreas Bießmann
Hi Andreas,
On 8/21/2013 23:14, Andreas Bießmann wrote:
On 08/13/2013 08:38 AM, Bo Shen wrote:
add gpio common API support for gpio command
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 15f396f..3de0844 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -363,3 +363,46 @@ int at91_get_pio_value(unsigned port, unsigned pin)
return pdsr != 0; }
+/* Common GPIO API */
+#define at91_gpio_to_port(gpio) (gpio / 32) +#define at91_gpio_to_pin(gpio) (gpio % 32)
+int gpio_request(unsigned gpio, const char *label) +{
- return 0;
+}
+int gpio_free(unsigned gpio) +{
- return 0;
+}
+int gpio_direction_input(unsigned gpio) +{
- at91_set_pio_input(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), 0);- return 0;
+}
+int gpio_direction_output(unsigned gpio, int value) +{
- at91_set_pio_output(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
+int gpio_get_value(unsigned gpio) +{
- return (int) at91_get_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio));why cast to int here?
Actually no need, as the at91_get_pio_value() return value is int. I will remove the cast in next version.
+}
+int gpio_set_value(unsigned gpio, int value) +{
- at91_set_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
Great, I love this. But wasn't there some define for generic GPIO?
I am not fully get your meaning, what you mean "define for generic GPIO"? define gpio pin number (?)
Shouldn't we encapsulate this API into this other define?
You mean, not in this file or anything else?
Best regards
Andreas Bießmann
Best Regards, Bo Shen
Hi Bo,
On 22.08.13 05:21, Bo Shen wrote:
Hi Andreas,
On 8/21/2013 23:14, Andreas Bießmann wrote:
On 08/13/2013 08:38 AM, Bo Shen wrote:
add gpio common API support for gpio command
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 15f396f..3de0844 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -363,3 +363,46 @@ int at91_get_pio_value(unsigned port, unsigned pin)
return pdsr != 0;}
+/* Common GPIO API */
+#define at91_gpio_to_port(gpio) (gpio / 32) +#define at91_gpio_to_pin(gpio) (gpio % 32)
+int gpio_request(unsigned gpio, const char *label) +{
- return 0;
+}
+int gpio_free(unsigned gpio) +{
- return 0;
+}
+int gpio_direction_input(unsigned gpio) +{
- at91_set_pio_input(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), 0);- return 0;
+}
+int gpio_direction_output(unsigned gpio, int value) +{
- at91_set_pio_output(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
+int gpio_get_value(unsigned gpio) +{
- return (int) at91_get_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio));why cast to int here?
Actually no need, as the at91_get_pio_value() return value is int. I will remove the cast in next version.
I could change this also when applying.
+}
+int gpio_set_value(unsigned gpio, int value) +{
- at91_set_pio_value(at91_gpio_to_port(gpio),
at91_gpio_to_pin(gpio), value);- return 0;
+}
Great, I love this. But wasn't there some define for generic GPIO?
I am not fully get your meaning, what you mean "define for generic GPIO"? define gpio pin number (?)
My fault, I thought there is some CONFIG_XXX for the 'generic GPIO API' (gpio_set_value/gpio_get_value/gpio_direction_input/...). It seems there is no such define, at91 gpio did just miss the time when this API was introduced.
I'm fine if you just send a v2 of the 1/4 patch. I can remove the cast in here and will _not_ apply 2/4 cause it breaks boards.
Best regards
Andreas Bießmann
Hi Andreas,
On 8/22/2013 14:34, Andreas Bießmann wrote:
I'm fine if you just send a v2 of the 1/4 patch. I can remove the cast in here and will_not_ apply 2/4 cause it breaks boards.
OK, I will send out the v2 of the 1/4 patch.
Best regards
Andreas Bießmann
Best Regards, Bo Shen
Dear Bo Shen,
Bo Shen voice.shen@atmel.com writes:
add gpio common API support for gpio command
Signed-off-by: Bo Shen voice.shen@atmel.com [fix unnecessary cast] Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com
drivers/gpio/at91_gpio.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
applied to u-boot-atmel/master, thanks!
Best regards, Andreas Bießmann
Signed-off-by: Bo Shen voice.shen@atmel.com
--- drivers/gpio/at91_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 3de0844..72161dc 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -1,5 +1,5 @@ /* - * Memory Setup stuff - taken from blob memsetup.S + * Copyright (C) 2013 Bo Shen voice.shen@atmel.com * * Copyright (C) 2009 Jens Scharsig (js_at_ng@scharsoft.de) *
Hi Bo, Jens,
On 08/13/2013 08:38 AM, Bo Shen wrote:
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 3de0844..72161dc 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -1,5 +1,5 @@ /*
- Memory Setup stuff - taken from blob memsetup.S
Jens, could you please ACK this and maybe clarify why it was here at all?
- Copyright (C) 2013 Bo Shen voice.shen@atmel.com
- Copyright (C) 2009 Jens Scharsig (js_at_ng@scharsoft.de)
Best regards
Andreas Bießmann
Am 2013-08-21 16:17, schrieb Andreas Bießmann:
Hi Bo, Jens,
On 08/13/2013 08:38 AM, Bo Shen wrote:
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 3de0844..72161dc 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -1,5 +1,5 @@ /*
- Memory Setup stuff - taken from blob memsetup.S
Jens, could you please ACK this and maybe clarify why it was here at all?
This line has the origin in a wrong copy & paste
- Copyright (C) 2013 Bo Shen voice.shen@atmel.com
- Copyright (C) 2009 Jens Scharsig (js_at_ng@scharsoft.de)
Best regards
Andreas Bießmann
Regards Jens
Am 2013-08-13 07:38, schrieb Bo Shen:
Signed-off-by: Bo Shen voice.shen@atmel.com
drivers/gpio/at91_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c index 3de0844..72161dc 100644 --- a/drivers/gpio/at91_gpio.c +++ b/drivers/gpio/at91_gpio.c @@ -1,5 +1,5 @@ /*
- Memory Setup stuff - taken from blob memsetup.S
- Copyright (C) 2013 Bo Shen voice.shen@atmel.com
- Copyright (C) 2009 Jens Scharsig (js_at_ng@scharsoft.de)
Acked-by: Jens Scharsig js_at_ng@scharsoft.de
Dear Bo Shen,
Bo Shen voice.shen@atmel.com writes:
Signed-off-by: Bo Shen voice.shen@atmel.com Acked-by: Jens Scharsig js_at_ng@scharsoft.de
drivers/gpio/at91_gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
applied to u-boot-atmel/master, thanks!
Best regards, Andreas Bießmann
participants (3)
-
Andreas Bießmann -
Bo Shen -
Jens Scharsig