Atomicity in GPIO pin changes


Jon Medhurst (Tixy) <tixy@...>
 

Hi

I have a hardware register I want to change individual bits in as they
are used for different purposes on my SoC. I thought the GPIO APIs might
be the correct abstraction to represent that, and even if that wasn't
suitable I thought I could at least look at those to work out how
atomicity of updates is achieved (Zephyr doesn't seem to have spinlocks,
and mutexes and the like seem a bit heavyweight for the task, ...).

However, looking at the GPIO API's and various implementations I can't
spot anything that serialises register updates, it just seems to boil
down to an unsafe "port |= (1 << bit_to_set)" or similar. Have I missed
things or is it expected that all users of a given port agree on a
mutual exclusion method? That doesn't seem very practical if they are
each different drivers or apps with no real need to know of each others
existence.

--
Tixy


Benjamin Walsh <benjamin.walsh@...>
 

On Fri, Aug 26, 2016 at 12:10:40PM +0100, Jon Medhurst (Tixy) wrote:
Hi

I have a hardware register I want to change individual bits in as they
are used for different purposes on my SoC. I thought the GPIO APIs might
be the correct abstraction to represent that, and even if that wasn't
suitable I thought I could at least look at those to work out how
atomicity of updates is achieved (Zephyr doesn't seem to have spinlocks,
and mutexes and the like seem a bit heavyweight for the task, ...).
Spinlocks don't give you anything more than interrupt or scheduler
locking in a uniprocessor system with a scheduler like Zephyr's, since
you need to lock interrupts and/or scheduling as well when you're holing
them or there is a good chance you'll deadlock. You probably want just
interrupt or scheduler locking instead. Scheduler locking is coming up
in the upcoming unified kernel, but is not available at the moment;
interrupt locking is of course available.

A mutex is indeed too heavy for this, and they're currently not usable
by fibers (upcoming change in the unified kernel as well). A nanokernel
semaphore might be a good fit. Or interrupt locking.

However, looking at the GPIO API's and various implementations I can't
spot anything that serialises register updates, it just seems to boil
down to an unsafe "port |= (1 << bit_to_set)" or similar. Have I missed
things or is it expected that all users of a given port agree on a
mutual exclusion method? That doesn't seem very practical if they are
each different drivers or apps with no real need to know of each others
existence.


Jon Medhurst (Tixy) <tixy@...>
 

On Fri, 2016-08-26 at 10:32 -0400, Benjamin Walsh wrote:
Spinlocks don't give you anything more than interrupt or scheduler
locking in a uniprocessor system with a scheduler like Zephyr's, since
you need to lock interrupts and/or scheduling as well when you're holing
them or there is a good chance you'll deadlock. You probably want just
interrupt or scheduler locking instead. Scheduler locking is coming up
in the upcoming unified kernel, but is not available at the moment;
interrupt locking is of course available.
I figured that interrupt locking was the way to go, was just wary of it
since it would break with SMP. Guess when that arrives all interrupt
disabling will need to be audited and converted to spinlocks if
appropriate.

That still leaves the issue that current GPIO drivers look dodgy without
any locking.

--
Tixy


Maciek Borzecki <maciek.borzecki@...>
 

On Fri, Aug 26, 2016 at 4:57 PM, Jon Medhurst (Tixy) <tixy(a)linaro.org> wrote:
On Fri, 2016-08-26 at 10:32 -0400, Benjamin Walsh wrote:
Spinlocks don't give you anything more than interrupt or scheduler
locking in a uniprocessor system with a scheduler like Zephyr's, since
you need to lock interrupts and/or scheduling as well when you're holing
them or there is a good chance you'll deadlock. You probably want just
interrupt or scheduler locking instead. Scheduler locking is coming up
in the upcoming unified kernel, but is not available at the moment;
interrupt locking is of course available.
I figured that interrupt locking was the way to go, was just wary of it
since it would break with SMP. Guess when that arrives all interrupt
disabling will need to be audited and converted to spinlocks if
appropriate.

That still leaves the issue that current GPIO drivers look dodgy without
any locking.
Not entirely. A particular SoC may provide means of setting/resetting
GPIO in an atomic fashion. Driver implementation should use such
mechanism if present.

For instance, STM32 GPIO peripheral has 2 dedicated bit set/reset
registers (GPIOx_BSRR/BRR) that allow for switching individual line
states in an atomic fashion. In this case, the driver does not perform
an intermediate read, but rather writes a desired value to the
register.

Cheers,
--
Maciek Borzecki


Jon Medhurst (Tixy) <tixy@...>
 

On Fri, 2016-08-26 at 17:18 +0200, Maciek Borzecki wrote:
On Fri, Aug 26, 2016 at 4:57 PM, Jon Medhurst (Tixy) <tixy(a)linaro.org> wrote:
On Fri, 2016-08-26 at 10:32 -0400, Benjamin Walsh wrote:
Spinlocks don't give you anything more than interrupt or scheduler
locking in a uniprocessor system with a scheduler like Zephyr's, since
you need to lock interrupts and/or scheduling as well when you're holing
them or there is a good chance you'll deadlock. You probably want just
interrupt or scheduler locking instead. Scheduler locking is coming up
in the upcoming unified kernel, but is not available at the moment;
interrupt locking is of course available.
I figured that interrupt locking was the way to go, was just wary of it
since it would break with SMP. Guess when that arrives all interrupt
disabling will need to be audited and converted to spinlocks if
appropriate.

That still leaves the issue that current GPIO drivers look dodgy without
any locking.
Not entirely. A particular SoC may provide means of setting/resetting
GPIO in an atomic fashion. Driver implementation should use such
mechanism if present.

For instance, STM32 GPIO peripheral has 2 dedicated bit set/reset
registers (GPIOx_BSRR/BRR) that allow for switching individual line
states in an atomic fashion. In this case, the driver does not perform
an intermediate read, but rather writes a desired value to the
register.
Thanks. I can see stm32 is OK in this regard, none of the others appear
to be.

I'll use interrupt disabling for my code to make it safe until SMP
arrives.

--
Tixy