hal/arm/at91/../var_io.h: HAL_ARM_AT91_GPIO_GET

hal/arm/at91/../var_io.h: HAL_ARM_AT91_GPIO_GET

[Jürgen Lambrecht]

Hello,

The current implementation of HAL_ARM_AT91_GPIO_GET in 
hal/arm/at91/../var_io.h uses the AT91_PIO_PDSR register.
But, "Reading the I/O line levels requires the clock of the PIO 
Controller to be enabled". So if you forget to enable the related PIO 
clock, this HAL_ARM_AT91_GPIO_GET always fails!
Therefore, I propose to use the AT91_PIO_ODSR register.
Anyhow, the added value of PDSR over ODSR is small I think:
-with ODSR you read the value you want this pin to be
-with PDSR you read the actual (physical) value of this pin
They are only different if there is a hardware problem..

So, to remove the burden of enabling also the relevant PIO clock, and to 
save power, I propose to use ODSR.
If you agree, I will add a patch.

Kind regards,
Jürgen

Re: hal/arm/at91/../var_io.h: HAL_ARM_AT91_GPIO_GET

[Nick Garnett]

Jürgen Lambrecht <J.Lambrecht@televic.com> writes:

> Hello,
> 
> The current implementation of HAL_ARM_AT91_GPIO_GET in
> hal/arm/at91/../var_io.h uses the AT91_PIO_PDSR register.
> But, "Reading the I/O line levels requires the clock of the PIO
> Controller to be enabled". So if you forget to enable the related PIO
> clock, this HAL_ARM_AT91_GPIO_GET always fails!
> Therefore, I propose to use the AT91_PIO_ODSR register.
> Anyhow, the added value of PDSR over ODSR is small I think:
> -with ODSR you read the value you want this pin to be
> -with PDSR you read the actual (physical) value of this pin
> They are only different if there is a hardware problem..

Without reading the documentaion: the ODSR give you the contents of
the output latch, while PDSR reads the line value.  ODSR and PDSR
should agree for output pins, but for inputs the ODSR may differ from
the line level, since it is no longer connected to the pin and pin is
being driven to its current level by some external device.

Also, surely if you are using a GPIO port you should be enabling its
clock. I would consider not doing that a programming error.


-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071

Re: hal/arm/at91/../var_io.h: HAL_ARM_AT91_GPIO_GET

[Jürgen Lambrecht]

Nick Garnett wrote:
> Jürgen Lambrecht <J.Lambrecht@televic.com> writes:
>
>   
>> Hello,
>>
>> The current implementation of HAL_ARM_AT91_GPIO_GET in
>> hal/arm/at91/../var_io.h uses the AT91_PIO_PDSR register.
>> But, "Reading the I/O line levels requires the clock of the PIO
>> Controller to be enabled". So if you forget to enable the related PIO
>> clock, this HAL_ARM_AT91_GPIO_GET always fails!
>> Therefore, I propose to use the AT91_PIO_ODSR register.
>> Anyhow, the added value of PDSR over ODSR is small I think:
>> -with ODSR you read the value you want this pin to be
>> -with PDSR you read the actual (physical) value of this pin
>> They are only different if there is a hardware problem..
>>     
>
> Without reading the documentaion: the ODSR give you the contents of
> the output latch, while PDSR reads the line value.  ODSR and PDSR
> should agree for output pins, but for inputs the ODSR may differ from
> the line level, since it is no longer connected to the pin and pin is
> being driven to its current level by some external device.
>   
Indeed. So in general, my proposition is wrong.
I then propose to make an "OUT" version of that macro.
> Also, surely if you are using a GPIO port you should be enabling its
> clock. I would consider not doing that a programming error.
>   
No, only for an input you must enable the PIO clock: the PIO clock is 
then used to sample the input signal.
For an output, you just write the output register from your application, 
using the application clock.

Kind regards,
Jürgen
>
> --
> Nick Garnett                                      eCos Kernel Architect
> eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
> Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
> Registered in England and Wales:                        Reg No: 4422071
>
>   

Re: hal/arm/at91/../var_io.h: HAL_ARM_AT91_GPIO_GET

[Nick Garnett]

Jürgen Lambrecht <J.Lambrecht@televic.com> writes:

> Nick Garnett wrote:
> > Jürgen Lambrecht <J.Lambrecht@televic.com> writes:
> >
> >
> >> Hello,
> >>
> >> The current implementation of HAL_ARM_AT91_GPIO_GET in
> >> hal/arm/at91/../var_io.h uses the AT91_PIO_PDSR register.
> >> But, "Reading the I/O line levels requires the clock of the PIO
> >> Controller to be enabled". So if you forget to enable the related PIO
> >> clock, this HAL_ARM_AT91_GPIO_GET always fails!
> >> Therefore, I propose to use the AT91_PIO_ODSR register.
> >> Anyhow, the added value of PDSR over ODSR is small I think:
> >> -with ODSR you read the value you want this pin to be
> >> -with PDSR you read the actual (physical) value of this pin
> >> They are only different if there is a hardware problem..
> >>
> >
> > Without reading the documentaion: the ODSR give you the contents of
> > the output latch, while PDSR reads the line value.  ODSR and PDSR
> > should agree for output pins, but for inputs the ODSR may differ from
> > the line level, since it is no longer connected to the pin and pin is
> > being driven to its current level by some external device.
> >
> Indeed. So in general, my proposition is wrong.
> I then propose to make an "OUT" version of that macro.
> > Also, surely if you are using a GPIO port you should be enabling its
> > clock. I would consider not doing that a programming error.
> >
> No, only for an input you must enable the PIO clock: the PIO clock is
> then used to sample the input signal.
> For an output, you just write the output register from your
> application, using the application clock.

OK. In that case a macro that reads back the value set in the output
register might make sense.

Of course one can make the argument that since your code must have set
this value initially, it shouldn't have to read it back to know what
it is. However, I appreciate that code is not always structured to
allow you to do that.


-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071