drivers: gpio: emul: replace mutex with spinlock

[alexanderwachter]

Replace all mutex with spinlocks to make the driver usable from ISRs.

Fixes #53964

[alexanderwachter]

@cfriedt second try :-)

[henrikbrixandersen]

How will switching from a mutex to a spinlock "make the driver usable from ISRs"? If application level code holds the spinlock and an ISR attempts to acquire it, the ISR will spin forever?

[alexanderwachter]

How will switching from a mutex to a spinlock "make the driver usable from ISRs"? If application level code holds the spinlock and an ISR attempts to acquire it, the ISR will spin forever?

How can an ISR execute if the spinlock is locked?
Do you think there is an issue with multicore?
I'm only using it in native posix, where the current solution is unusable, because our app reads the state of an input within the ISR.

[cfriedt]

Looks OK to me. I was actually not really sure if this would be possible, but I'm impressed. Just some minor changes requested to remove unnecessary changes / update docs.

I would like to take another look at the second draft as well and see if I can break it.

[cfriedt]

@alexanderwachter - I would go so far as to say that this fixes a bug. Do you know if an issue exists for that already?

[alexanderwachter]

@cfriedt not that I'm aware of. At least not from me.
Agree, it is a bug. The API must return -EWOULDBLOCK.

[cfriedt]

@cfriedt not that I'm aware of. At least not from me.

@alexanderwachter - Oh, I meant not being able to use gpio_emul in IRQ contexts is a bug actually, so this PR is itself a Bugfix.

Can you also please create an issue and link it here?

Agree, it is a bug. The API must return -EWOULDBLOCK.

Right, I think I see what you mean.

Actually, that is part of a larger issue that we have in Zephyr. Very few Zephyr system calls have an associated timeout (including GPIO).

So there is no clean way at the moment to fail with an error if a such an operation would block.

Blocking I/O (with a timeout) would also require a mutex or something along those lines which is almost a step in the reverse direction. It would force users to incur not only the syscall overhead but also the overhead of locking a mutex. Maybe that would be negligible in the future with with zync, but we'll see.

One could also argue, much like accessing any kind of data structure, synchronizing control of a GPIO is entirely application dependent and is not needed by the large majority of GPIO users.

What you've identified is out of the scope of this PR, so I wouldn't worry about it here.

For the time being, the implicit assumption is that GPIO operations are quick and they never fail.

[alexanderwachter]

@cfriedt ready for merging?
I think this should go in 3.3.0

[cfriedt]

@alexanderwachter - yes, but I'll let @stephanosio or @laurenmurphyx64 do it once there is another review

[alexanderwachter]

@mnkp any input?

[cfriedt]

@henrikbrixandersen - will you ack?

[teburd]

Is the GPIO interface labeled as ISR safe? This is kind of a confusing change to be honest.

[andyross]

All looks reasonable to me. Note that the "release the lock around the callback" pattern opens races if other contexts want to modify the data used by the callback while it's running. That's arguably user error, but might be worth documenting.