On 11/14/20 5:33 PM, Jonathan Cameron wrote:
> On Thu, 12 Nov 2020 16:13:33 +0100
> Nuno Sá <nuno.sa@xxxxxxxxxx> wrote:
>
>> When going through the code of the buffer-cb interface and all it's
>> users, I realized that the stm32_adfsdm driver is calling
>> `iio_channel_get_all_cb()` with NULL for the cb. After going a bit
>> trough the stm drivers, it looks like this is actually intentional.
>> However, it is clear that we have a clear/direct route here for a NULL
>> pointer dereference. I'm being lazy in this RFC and just doing a
>> sanity check in the `iio_buffer_cb_store_to()` so that we don't have to
>> change the stm driver... The point is just to bring this up and see if
>> we want to do something about this.
>>
>> To be clear, the way I think this should go is just to return -EINVAL in
>> `iio_channel_get_all_cb()` if a NULL ptr is passed. Whats the point of a
>> buffer-cb if cb is NULL, right? This would naturally break the stm
>> driver, but I guess we could just define a dummy handler there that
>> would not be used (or could the HW consumer be an option here?)...
>>
>> Thoughts?
>
> Good description thanks. I think you are right and better option is
> to return -EINVAL in iio_channel_get_all_cb() and add a dummy handler to the
> stm driver.
>
> cc Arnaud and Olivier to see if they are fine with the dummy handler.
> (with appropriate comment on why it is there).
>
> Jonathan
>
>
Hi Jonathan,
Thanks for your notice. Arnaud and myself agree with this solution.
I attach to this mail a patch which implements this dummy handler
and can be added to Nuno's patchset, to avoid a regression on stm driver.
Regards,
Olivier
>>
>> Nuno Sá (1):
>> iio: buffer: Sanity check buffer callback
>>
>> drivers/iio/buffer/industrialio-buffer-cb.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>
From 98d3008bb2cdee75b2efb42a7d3a5af9bed47e9c Mon Sep 17 00:00:00 2001
From: Olivier Moysan <olivier.moysan@xxxxxx>
Date: Mon, 16 Nov 2020 17:23:43 +0100
Subject: [PATCH] ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback
Adapt STM32 DFSDM driver to a change in iio_channel_get_all_cb() API.
The callback pointer becomes a requested parameter of this API,
so add a dummy callback to be given as parameter of this function.
However, the stm32_dfsdm_get_buff_cb() API is still used instead,
to optimize DMA transfers.
Signed-off-by: Olivier Moysan <olivier.moysan@xxxxxx>
---
sound/soc/stm/stm32_adfsdm.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/sound/soc/stm/stm32_adfsdm.c b/sound/soc/stm/stm32_adfsdm.c
index c4031988f981..47fae8dd20b4 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -293,6 +293,16 @@ static int stm32_adfsdm_pcm_new(struct snd_soc_component *component,
return 0;
}
+static int stm32_adfsdm_dummy_cb(const void *data, void *private)
+{
+ /*
+ * This dummmy callback is requested by iio_channel_get_all_cb() API,
+ * but the stm32_dfsdm_get_buff_cb() API is used instead, to optimize
+ * DMA transfers.
+ */
+ return 0;
+}
+
static struct snd_soc_component_driver stm32_adfsdm_soc_platform = {
.open = stm32_adfsdm_pcm_open,
.close = stm32_adfsdm_pcm_close,
@@ -335,7 +345,7 @@ static int stm32_adfsdm_probe(struct platform_device *pdev)
if (IS_ERR(priv->iio_ch))
return PTR_ERR(priv->iio_ch);
- priv->iio_cb = iio_channel_get_all_cb(&pdev->dev, NULL, NULL);
+ priv->iio_cb = iio_channel_get_all_cb(&pdev->dev, &stm32_adfsdm_dummy_cb, NULL);
if (IS_ERR(priv->iio_cb))
return PTR_ERR(priv->iio_cb);
--
2.17.1
