[v2,2/5] iio: temperature: ltc2983: make bulk write buffer DMA-safe

Message ID 20221020090257.1717053-3-demonsingur@gmail.com
State New
Headers
Series Support more parts in LTC2983 |

Commit Message

Cosmin Tanislav Oct. 20, 2022, 9:02 a.m. UTC
  From: Cosmin Tanislav <cosmin.tanislav@analog.com>

regmap_bulk_write() does not guarantee implicit DMA-safety,
even though the current implementation duplicates the given
buffer. Do not rely on it.

Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
 drivers/iio/temperature/ltc2983.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
  

Comments

Jonathan Cameron Oct. 23, 2022, 12:42 p.m. UTC | #1
On Thu, 20 Oct 2022 12:02:54 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:

> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> 
> regmap_bulk_write() does not guarantee implicit DMA-safety,
> even though the current implementation duplicates the given
> buffer. Do not rely on it.
> 
> Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
LGTM.

As you right observed this is only sort of a fix because right now we
are fine anyway, so in the interests of getting the rest of the series
upstream quicker I'll take this one for the next merge window along
with the rest of the set.

Thanks,

Jonathan

> ---
>  drivers/iio/temperature/ltc2983.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index a60ccf183687..1117991ca2ab 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -209,6 +209,7 @@ struct ltc2983_data {
>  	 * Holds the converted temperature
>  	 */
>  	__be32 temp __aligned(IIO_DMA_MINALIGN);
> +	__be32 chan_val;
>  };
>  
>  struct ltc2983_sensor {
> @@ -313,19 +314,18 @@ static int __ltc2983_fault_handler(const struct ltc2983_data *st,
>  	return 0;
>  }
>  
> -static int __ltc2983_chan_assign_common(const struct ltc2983_data *st,
> +static int __ltc2983_chan_assign_common(struct ltc2983_data *st,
>  					const struct ltc2983_sensor *sensor,
>  					u32 chan_val)
>  {
>  	u32 reg = LTC2983_CHAN_START_ADDR(sensor->chan);
> -	__be32 __chan_val;
>  
>  	chan_val |= LTC2983_CHAN_TYPE(sensor->type);
>  	dev_dbg(&st->spi->dev, "Assign reg:0x%04X, val:0x%08X\n", reg,
>  		chan_val);
> -	__chan_val = cpu_to_be32(chan_val);
> -	return regmap_bulk_write(st->regmap, reg, &__chan_val,
> -				 sizeof(__chan_val));
> +	st->chan_val = cpu_to_be32(chan_val);
> +	return regmap_bulk_write(st->regmap, reg, &st->chan_val,
> +				 sizeof(st->chan_val));
>  }
>  
>  static int __ltc2983_chan_custom_sensor_assign(struct ltc2983_data *st,
  
Sa, Nuno Oct. 24, 2022, 6:35 a.m. UTC | #2
> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, October 23, 2022 2:43 PM
> To: Cosmin Tanislav <demonsingur@gmail.com>
> Cc: Sa, Nuno <Nuno.Sa@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Hennerich, Michael <Michael.Hennerich@analog.com>;
> Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@linaro.org>; linux-iio@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Tanislav, Cosmin
> <Cosmin.Tanislav@analog.com>
> Subject: Re: [PATCH v2 2/5] iio: temperature: ltc2983: make bulk write buffer
> DMA-safe
> 
> [External]
> 
> On Thu, 20 Oct 2022 12:02:54 +0300
> Cosmin Tanislav <demonsingur@gmail.com> wrote:
> 
> > From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> >
> > regmap_bulk_write() does not guarantee implicit DMA-safety,
> > even though the current implementation duplicates the given
> > buffer. Do not rely on it.
> >
> > Fixes: f110f3188e56 ("iio: temperature: Add support for LTC2983")
> > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> LGTM.
> 
> As you right observed this is only sort of a fix because right now we
> are fine anyway, so in the interests of getting the rest of the series
> upstream quicker I'll take this one for the next merge window along
> with the rest of the set.
> 
> Thanks,
> 
> Jonathan
> 

Not sure if you already applied this... Anyways:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>
  

Patch

diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
index a60ccf183687..1117991ca2ab 100644
--- a/drivers/iio/temperature/ltc2983.c
+++ b/drivers/iio/temperature/ltc2983.c
@@ -209,6 +209,7 @@  struct ltc2983_data {
 	 * Holds the converted temperature
 	 */
 	__be32 temp __aligned(IIO_DMA_MINALIGN);
+	__be32 chan_val;
 };
 
 struct ltc2983_sensor {
@@ -313,19 +314,18 @@  static int __ltc2983_fault_handler(const struct ltc2983_data *st,
 	return 0;
 }
 
-static int __ltc2983_chan_assign_common(const struct ltc2983_data *st,
+static int __ltc2983_chan_assign_common(struct ltc2983_data *st,
 					const struct ltc2983_sensor *sensor,
 					u32 chan_val)
 {
 	u32 reg = LTC2983_CHAN_START_ADDR(sensor->chan);
-	__be32 __chan_val;
 
 	chan_val |= LTC2983_CHAN_TYPE(sensor->type);
 	dev_dbg(&st->spi->dev, "Assign reg:0x%04X, val:0x%08X\n", reg,
 		chan_val);
-	__chan_val = cpu_to_be32(chan_val);
-	return regmap_bulk_write(st->regmap, reg, &__chan_val,
-				 sizeof(__chan_val));
+	st->chan_val = cpu_to_be32(chan_val);
+	return regmap_bulk_write(st->regmap, reg, &st->chan_val,
+				 sizeof(st->chan_val));
 }
 
 static int __ltc2983_chan_custom_sensor_assign(struct ltc2983_data *st,