On Tue, Feb 06, 2024 at 03:47:45PM +0100, Mike Looijmans wrote:The spinlock also protects the call to spi_async().
On 06-02-2024 15:25, Mike Looijmans wrote:...
On 06-02-2024 14:50, Andy Shevchenko wrote:
On Tue, Feb 06, 2024 at 02:33:47PM +0100, Mike Looijmans wrote:
On 06-02-2024 13:57, Andy Shevchenko wrote:
On Tue, Feb 06, 2024 at 07:58:18AM +0100, Mike Looijmans wrote:
Yep and it looks like you reinvented atomics :-)Having done that, and looking at it again, it's better to just eliminateYes, looks more obvious.Maybe better write this as:+ wasbusy = --priv->rdata_xfer_busy;Why preincrement? How would it be different from postincrement?
--priv->rdata_xfer_busy;
wasbusy = priv->rdata_xfer_busy;
I want the value after decrementing it.
the local "wasbusy" altogether. More concise.
This removes the decrement operator, so the method now looks like this:
static void ads1298_rdata_release_busy_or_restart(struct ads1298_private
*priv)
{
guard(spinlock_irqsave)(&priv->irq_busy_lock);
if (priv->rdata_xfer_busy > 1) {
/*
* DRDY interrupt occurred before SPI completion. Start a new
* SPI transaction now to retrieve the data that wasn't latched
* into the ADS1298 chip's transfer buffer yet.
*/
spi_async(priv->spi, &priv->rdata_msg);
/*
* If more than one DRDY took place, there was an overrun. Since
* the sample is already lost, reset the counter to 1 so that
* we will wait for a DRDY interrupt after this SPI transaction.
*/
priv->rdata_xfer_busy = 1;
} else {
/* No pending data, wait for DRDY */
priv->rdata_xfer_busy = 0;
}
}
atomic_t rdata_xfer_busy;
...
But it's up to you what to do with that.
Maybe Jonathan can advice something different.