diff mbox

[U-Boot,1/4] gpio: atmel: fix code to use pointer for pio port

Message ID 1376375912-13835-2-git-send-email-voice.shen@atmel.com
State Changes Requested, archived
Delegated to: Andreas Bießmann
Headers show

Commit Message

Bo Shen Aug. 13, 2013, 6:38 a.m. UTC 
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(-)

Comments

Andreas Bießmann Aug. 21, 2013, 3:08 p.m. UTC | #1 
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);
>  		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);
>  	}
> +

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
Bo Shen Aug. 22, 2013, 3:15 a.m. UTC | #2 
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);
>>   		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);
>>   	}
>> +
>
> 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
Andreas Bießmann Aug. 22, 2013, 6:26 a.m. UTC | #3 
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
Bo Shen Aug. 22, 2013, 7:03 a.m. UTC | #4 
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
diff mbox

Patch

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;
 }