* [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations
@ 2025-08-31 15:37 Mohammad Amin Hosseini
2025-09-01 6:10 ` Greg KH
2025-09-01 16:47 ` David Lechner
0 siblings, 2 replies; 3+ messages in thread
From: Mohammad Amin Hosseini @ 2025-08-31 15:37 UTC (permalink / raw)
To: linux-iio, linux-staging
Cc: linux-kernel, gregkh, jic23, lars, Michael.Hennerich, dlechner,
nuno.sa, andy, mohammad amin hosseini
From: mohammad amin hosseini <moahmmad.hosseinii@gmail.com>
The ad7816 driver was accessing SPI and GPIO lines without
synchronization, which could lead to race conditions when accessed
concurrently from multiple contexts. This might result in corrupted
readings or inconsistent GPIO states.
Introduce an io_lock mutex in the driver structure to serialize:
- SPI transactions in ad7816_spi_read() and ad7816_spi_write()
- GPIO pin toggling sequences
- Updates to device state via sysfs store functions (mode, channel, oti)
The mutex ensures proper mutual exclusion and prevents race
conditions under concurrent access.
Signed-off-by: Mohammad Amin Hosseini <moahmmad.hosseinii@gmail.com>
---
drivers/staging/iio/adc/ad7816.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 4774df778de9..06567d048a6d 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -50,6 +50,7 @@ struct ad7816_chip_info {
u8 oti_data[AD7816_CS_MAX + 1];
u8 channel_id; /* 0 always be temperature */
u8 mode;
+ struct mutex io_lock; /* Protects SPI transactions and GPIO toggling */
};
enum ad7816_type {
@@ -67,13 +68,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
int ret;
__be16 buf;
+ mutex_lock(&chip->io_lock);
+
gpiod_set_value(chip->rdwr_pin, 1);
gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
- if (ret < 0) {
- dev_err(&spi_dev->dev, "SPI channel setting error\n");
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
gpiod_set_value(chip->rdwr_pin, 1);
if (chip->mode == AD7816_PD) { /* operating mode 2 */
@@ -92,13 +93,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
gpiod_set_value(chip->rdwr_pin, 0);
gpiod_set_value(chip->rdwr_pin, 1);
ret = spi_read(spi_dev, &buf, sizeof(*data));
- if (ret < 0) {
- dev_err(&spi_dev->dev, "SPI data read error\n");
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
*data = be16_to_cpu(buf);
+unlock:
+ mutex_unlock(&chip->io_lock);
return ret;
}
@@ -107,12 +108,13 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
struct spi_device *spi_dev = chip->spi_dev;
int ret;
+ mutex_lock(&chip->io_lock);
+
gpiod_set_value(chip->rdwr_pin, 1);
gpiod_set_value(chip->rdwr_pin, 0);
ret = spi_write(spi_dev, &data, sizeof(data));
- if (ret < 0)
- dev_err(&spi_dev->dev, "SPI oti data write error\n");
+ mutex_unlock(&chip->io_lock);
return ret;
}
@@ -136,6 +138,7 @@ static ssize_t ad7816_store_mode(struct device *dev,
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7816_chip_info *chip = iio_priv(indio_dev);
+ mutex_lock(&chip->io_lock);
if (strcmp(buf, "full") == 0) {
gpiod_set_value(chip->rdwr_pin, 1);
chip->mode = AD7816_FULL;
@@ -143,6 +146,7 @@ static ssize_t ad7816_store_mode(struct device *dev,
gpiod_set_value(chip->rdwr_pin, 0);
chip->mode = AD7816_PD;
}
+ mutex_unlock(&chip->io_lock);
return len;
}
@@ -200,7 +204,9 @@ static ssize_t ad7816_store_channel(struct device *dev,
return -EINVAL;
}
+ mutex_lock(&chip->io_lock);
chip->channel_id = data;
+ mutex_unlock(&chip->io_lock);
return len;
}
@@ -322,7 +328,9 @@ static inline ssize_t ad7816_set_oti(struct device *dev,
if (ret)
return -EIO;
+ mutex_lock(&chip->io_lock);
chip->oti_data[chip->channel_id] = data;
+ mutex_unlock(&chip->io_lock);
return len;
}
@@ -363,6 +371,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
dev_set_drvdata(&spi_dev->dev, indio_dev);
chip->spi_dev = spi_dev;
+ mutex_init(&chip->io_lock);
for (i = 0; i <= AD7816_CS_MAX; i++)
chip->oti_data[i] = 203;
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations
2025-08-31 15:37 [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations Mohammad Amin Hosseini
@ 2025-09-01 6:10 ` Greg KH
2025-09-01 16:47 ` David Lechner
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2025-09-01 6:10 UTC (permalink / raw)
To: Mohammad Amin Hosseini
Cc: linux-iio, linux-staging, linux-kernel, jic23, lars,
Michael.Hennerich, dlechner, nuno.sa, andy
On Sun, Aug 31, 2025 at 07:07:41PM +0330, Mohammad Amin Hosseini wrote:
> From: mohammad amin hosseini <moahmmad.hosseinii@gmail.com>
>
> The ad7816 driver was accessing SPI and GPIO lines without
> synchronization, which could lead to race conditions when accessed
> concurrently from multiple contexts. This might result in corrupted
> readings or inconsistent GPIO states.
>
> Introduce an io_lock mutex in the driver structure to serialize:
> - SPI transactions in ad7816_spi_read() and ad7816_spi_write()
> - GPIO pin toggling sequences
> - Updates to device state via sysfs store functions (mode, channel, oti)
>
> The mutex ensures proper mutual exclusion and prevents race
> conditions under concurrent access.
>
> Signed-off-by: Mohammad Amin Hosseini <moahmmad.hosseinii@gmail.com>
Doesn't match the From: line exactly :(
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations
2025-08-31 15:37 [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations Mohammad Amin Hosseini
2025-09-01 6:10 ` Greg KH
@ 2025-09-01 16:47 ` David Lechner
1 sibling, 0 replies; 3+ messages in thread
From: David Lechner @ 2025-09-01 16:47 UTC (permalink / raw)
To: Mohammad Amin Hosseini, linux-iio, linux-staging
Cc: linux-kernel, gregkh, jic23, lars, Michael.Hennerich, nuno.sa,
andy
On 8/31/25 10:37 AM, Mohammad Amin Hosseini wrote:
> From: mohammad amin hosseini <moahmmad.hosseinii@gmail.com>
>
> The ad7816 driver was accessing SPI and GPIO lines without
> synchronization, which could lead to race conditions when accessed
> concurrently from multiple contexts. This might result in corrupted
> readings or inconsistent GPIO states.
>
> Introduce an io_lock mutex in the driver structure to serialize:
> - SPI transactions in ad7816_spi_read() and ad7816_spi_write()
> - GPIO pin toggling sequences
> - Updates to device state via sysfs store functions (mode, channel, oti)
>
> The mutex ensures proper mutual exclusion and prevents race
> conditions under concurrent access.
>
> Signed-off-by: Mohammad Amin Hosseini <moahmmad.hosseinii@gmail.com>
> ---
> drivers/staging/iio/adc/ad7816.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
> index 4774df778de9..06567d048a6d 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -50,6 +50,7 @@ struct ad7816_chip_info {
> u8 oti_data[AD7816_CS_MAX + 1];
> u8 channel_id; /* 0 always be temperature */
> u8 mode;
> + struct mutex io_lock; /* Protects SPI transactions and GPIO toggling */
> };
>
> enum ad7816_type {
> @@ -67,13 +68,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> int ret;
> __be16 buf;
>
> + mutex_lock(&chip->io_lock);
Could avoid the gotos by using:
guard(mutex)(&chip->io_lock);
> +
> gpiod_set_value(chip->rdwr_pin, 1);
> gpiod_set_value(chip->rdwr_pin, 0);
> ret = spi_write(spi_dev, &chip->channel_id, sizeof(chip->channel_id));
> - if (ret < 0) {
> - dev_err(&spi_dev->dev, "SPI channel setting error\n");
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
> gpiod_set_value(chip->rdwr_pin, 1);
>
> if (chip->mode == AD7816_PD) { /* operating mode 2 */
> @@ -92,13 +93,13 @@ static int ad7816_spi_read(struct ad7816_chip_info *chip, u16 *data)
> gpiod_set_value(chip->rdwr_pin, 0);
> gpiod_set_value(chip->rdwr_pin, 1);
> ret = spi_read(spi_dev, &buf, sizeof(*data));
> - if (ret < 0) {
> - dev_err(&spi_dev->dev, "SPI data read error\n");
> - return ret;
> - }
> + if (ret < 0)
> + goto unlock;
>
> *data = be16_to_cpu(buf);
>
> +unlock:
> + mutex_unlock(&chip->io_lock);
> return ret;
> }
>
> @@ -107,12 +108,13 @@ static int ad7816_spi_write(struct ad7816_chip_info *chip, u8 data)
> struct spi_device *spi_dev = chip->spi_dev;
> int ret;
>
> + mutex_lock(&chip->io_lock);
> +
> gpiod_set_value(chip->rdwr_pin, 1);
> gpiod_set_value(chip->rdwr_pin, 0);
> ret = spi_write(spi_dev, &data, sizeof(data));
> - if (ret < 0)
> - dev_err(&spi_dev->dev, "SPI oti data write error\n");
>
> + mutex_unlock(&chip->io_lock);
> return ret;
> }
>
...
> @@ -363,6 +371,7 @@ static int ad7816_probe(struct spi_device *spi_dev)
> dev_set_drvdata(&spi_dev->dev, indio_dev);
>
> chip->spi_dev = spi_dev;
> + mutex_init(&chip->io_lock);
Prefer devm_mutex_init() instead.
> for (i = 0; i <= AD7816_CS_MAX; i++)
> chip->oti_data[i] = 203;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-01 16:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-31 15:37 [PATCH] staging: iio: adc: ad7816: add mutex to serialize SPI/GPIO operations Mohammad Amin Hosseini
2025-09-01 6:10 ` Greg KH
2025-09-01 16:47 ` David Lechner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).