[PATCH 0/2] Make cb a required parameter of buffer-cb

[PATCH 0/2] Make cb a required parameter of buffer-cb

[Nuno Sá]

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. This change makes cb a required parameter of the
API.

The first patch makes the necessary changes to the stm32_adfsdm driver
so that it does not break.

Nuno Sá (1):
  iio: buffer: Return error if no callback is given

Olivier Moysan (1):
  ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback

 drivers/iio/buffer/industrialio-buffer-cb.c |  5 +++++
 sound/soc/stm/stm32_adfsdm.c                | 12 +++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH 1/2] ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback

[Nuno Sá]

From: Olivier Moysan <olivier.moysan@st.com>

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@st.com>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 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 ec27c13af04f..fb9df3caba12 100644
--- a/sound/soc/stm/stm32_adfsdm.c
+++ b/sound/soc/stm/stm32_adfsdm.c
@@ -295,6 +295,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,
@@ -337,7 +347,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

[PATCH 2/2] iio: buffer: Return error if no callback is given

[Nuno Sá]

Return error in case no callback is provided to
`iio_channel_get_all_cb()`. There's no point in setting up a buffer-cb
if no callback is provided.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/buffer/industrialio-buffer-cb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iio/buffer/industrialio-buffer-cb.c b/drivers/iio/buffer/industrialio-buffer-cb.c
index 47c96f7f4976..4c12b7a94af5 100644
--- a/drivers/iio/buffer/industrialio-buffer-cb.c
+++ b/drivers/iio/buffer/industrialio-buffer-cb.c
@@ -54,6 +54,11 @@ struct iio_cb_buffer *iio_channel_get_all_cb(struct device *dev,
 	struct iio_cb_buffer *cb_buff;
 	struct iio_channel *chan;
 
+	if (!cb) {
+		dev_err(dev, "Invalid arguments: A callback must be provided!\n");
+		return ERR_PTR(-EINVAL);
+	}
+
 	cb_buff = kzalloc(sizeof(*cb_buff), GFP_KERNEL);
 	if (cb_buff == NULL)
 		return ERR_PTR(-ENOMEM);
-- 
2.17.1

Re: [PATCH 0/2] Make cb a required parameter of buffer-cb

[Jonathan Cameron]

On Sat, 21 Nov 2020 17:14:55 +0100
Nuno Sá <nuno.sa@analog.com> 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. This change makes cb a required parameter of the
> API.
> 
> The first patch makes the necessary changes to the stm32_adfsdm driver
> so that it does not break.

Looks good to me.  Will leave it a bit longer to let ASOC people
take a quick look at it. Give me a poke in a few weeks if it seems
like I might have lost it!

Thanks,

Jonathan

> 
> Nuno Sá (1):
>   iio: buffer: Return error if no callback is given
> 
> Olivier Moysan (1):
>   ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback
> 
>  drivers/iio/buffer/industrialio-buffer-cb.c |  5 +++++
>  sound/soc/stm/stm32_adfsdm.c                | 12 +++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 

Re: [PATCH 2/2] iio: buffer: Return error if no callback is given

[Olivier MOYSAN]

On 11/21/20 5:14 PM, Nuno Sá wrote:
> Return error in case no callback is provided to
> `iio_channel_get_all_cb()`. There's no point in setting up a buffer-cb
> if no callback is provided.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>   drivers/iio/buffer/industrialio-buffer-cb.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-cb.c b/drivers/iio/buffer/industrialio-buffer-cb.c
> index 47c96f7f4976..4c12b7a94af5 100644
> --- a/drivers/iio/buffer/industrialio-buffer-cb.c
> +++ b/drivers/iio/buffer/industrialio-buffer-cb.c
> @@ -54,6 +54,11 @@ struct iio_cb_buffer *iio_channel_get_all_cb(struct device *dev,
>   	struct iio_cb_buffer *cb_buff;
>   	struct iio_channel *chan;
>   
> +	if (!cb) {
> +		dev_err(dev, "Invalid arguments: A callback must be provided!\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
>   	cb_buff = kzalloc(sizeof(*cb_buff), GFP_KERNEL);
>   	if (cb_buff == NULL)
>   		return ERR_PTR(-ENOMEM);
> 

Reviewed-by: Olivier Moysan <olivier.moysan@st.com>

Best regards,
Olivier

Re: [PATCH 1/2] ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

On Sat, Nov 21, 2020 at 05:14:56PM +0100, Nuno Sá wrote:
> From: Olivier Moysan <olivier.moysan@st.com>
> 
> 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.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/2] Make cb a required parameter of buffer-cb

[Jonathan Cameron]

On Sat, 21 Nov 2020 17:14:55 +0100
Nuno Sá <nuno.sa@analog.com> 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. This change makes cb a required parameter of the
> API.
> 
> The first patch makes the necessary changes to the stm32_adfsdm driver
> so that it does not break.
> 
> Nuno Sá (1):
>   iio: buffer: Return error if no callback is given
> 
> Olivier Moysan (1):
>   ASoC: stm32: dfsdm: add stm32_adfsdm_dummy_cb() callback
> 
>  drivers/iio/buffer/industrialio-buffer-cb.c |  5 +++++
>  sound/soc/stm/stm32_adfsdm.c                | 12 +++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to do their magic.

Thanks,

Jonathan