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