linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function
@ 2020-09-24 10:29 Alexandru Ardelean
  2020-09-26 15:55 ` Jonathan Cameron
  0 siblings, 1 reply; 2+ messages in thread
From: Alexandru Ardelean @ 2020-09-24 10:29 UTC (permalink / raw)
  To: linux-iio, linux-arm-kernel, linux-kernel
  Cc: jic23, eugen.hristev, nicolas.ferre, alexandre.belloni,
	ludovic.desroches, Alexandru Ardelean

This change is mostly cosmetic, but it's also a pre-cursor to the
the change for 'iio_buffer_set_attrs()', where the helper gets updated to
better support multiple IIO buffers for 1 IIO device.

The only functional change is that the error message for the trigger alloc
failure is bound to the parent device vs the IIO device object.

Also, the new at91_adc_buffer_and_trigger_init() function was moved after
the definition of the 'at91_adc_fifo_attributes'.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/adc/at91-sama5d2_adc.c | 78 ++++++++++++++----------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index ad7d9819f83c..b9c3cc6d5913 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1014,21 +1014,6 @@ static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
 
 	return trig;
 }
-
-static int at91_adc_trigger_init(struct iio_dev *indio)
-{
-	struct at91_adc_state *st = iio_priv(indio);
-
-	st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
-	if (IS_ERR(st->trig)) {
-		dev_err(&indio->dev,
-			"could not allocate trigger\n");
-		return PTR_ERR(st->trig);
-	}
-
-	return 0;
-}
-
 static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
 					   struct iio_poll_func *pf)
 {
@@ -1156,13 +1141,6 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
-static int at91_adc_buffer_init(struct iio_dev *indio)
-{
-	return devm_iio_triggered_buffer_setup(&indio->dev, indio,
-		&iio_pollfunc_store_time,
-		&at91_adc_trigger_handler, &at91_buffer_setup_ops);
-}
-
 static unsigned at91_adc_startup_time(unsigned startup_time_min,
 				      unsigned adc_clk_khz)
 {
@@ -1683,6 +1661,40 @@ static const struct iio_info at91_adc_info = {
 	.hwfifo_set_watermark = &at91_adc_set_watermark,
 };
 
+static int at91_adc_buffer_and_trigger_init(struct device *dev,
+					    struct iio_dev *indio)
+{
+	struct at91_adc_state *st = iio_priv(indio);
+	int ret;
+
+	ret = devm_iio_triggered_buffer_setup(&indio->dev, indio,
+		&iio_pollfunc_store_time,
+		&at91_adc_trigger_handler, &at91_buffer_setup_ops);
+	if (ret < 0) {
+		dev_err(dev, "couldn't initialize the buffer.\n");
+		return ret;
+	}
+
+	if (!st->selected_trig->hw_trig)
+		return 0;
+
+	iio_buffer_set_attrs(indio->buffer, at91_adc_fifo_attributes);
+
+	st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
+	if (IS_ERR(st->trig)) {
+		dev_err(dev, "could not allocate trigger\n");
+		return PTR_ERR(st->trig);
+	}
+
+	/*
+	 * Initially the iio buffer has a length of 2 and
+	 * a watermark of 1
+	 */
+	st->dma_st.watermark = 1;
+
+	return 0;
+}
+
 static int at91_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
@@ -1818,27 +1830,9 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	ret = at91_adc_buffer_init(indio_dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
+	ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);
+	if (ret < 0)
 		goto per_clk_disable_unprepare;
-	}
-
-	if (st->selected_trig->hw_trig) {
-		ret = at91_adc_trigger_init(indio_dev);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "couldn't setup the triggers.\n");
-			goto per_clk_disable_unprepare;
-		}
-		/*
-		 * Initially the iio buffer has a length of 2 and
-		 * a watermark of 1
-		 */
-		st->dma_st.watermark = 1;
-
-		iio_buffer_set_attrs(indio_dev->buffer,
-				     at91_adc_fifo_attributes);
-	}
 
 	if (dma_coerce_mask_and_coherent(&indio_dev->dev, DMA_BIT_MASK(32)))
 		dev_info(&pdev->dev, "cannot set DMA mask to 32-bit\n");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function
  2020-09-24 10:29 [PATCH] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function Alexandru Ardelean
@ 2020-09-26 15:55 ` Jonathan Cameron
  0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2020-09-26 15:55 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: linux-iio, linux-arm-kernel, linux-kernel, eugen.hristev,
	nicolas.ferre, alexandre.belloni, ludovic.desroches

On Thu, 24 Sep 2020 13:29:02 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change is mostly cosmetic, but it's also a pre-cursor to the
> the change for 'iio_buffer_set_attrs()', where the helper gets updated to
> better support multiple IIO buffers for 1 IIO device.
> 
> The only functional change is that the error message for the trigger alloc
> failure is bound to the parent device vs the IIO device object.
> 
> Also, the new at91_adc_buffer_and_trigger_init() function was moved after
> the definition of the 'at91_adc_fifo_attributes'.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Looks fine to me, but will let this sit on the list for a while
so others have time to comment.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/at91-sama5d2_adc.c | 78 ++++++++++++++----------------
>  1 file changed, 36 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index ad7d9819f83c..b9c3cc6d5913 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -1014,21 +1014,6 @@ static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
>  
>  	return trig;
>  }
> -
> -static int at91_adc_trigger_init(struct iio_dev *indio)
> -{
> -	struct at91_adc_state *st = iio_priv(indio);
> -
> -	st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
> -	if (IS_ERR(st->trig)) {
> -		dev_err(&indio->dev,
> -			"could not allocate trigger\n");
> -		return PTR_ERR(st->trig);
> -	}
> -
> -	return 0;
> -}
> -
>  static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev,
>  					   struct iio_poll_func *pf)
>  {
> @@ -1156,13 +1141,6 @@ static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>  	return IRQ_HANDLED;
>  }
>  
> -static int at91_adc_buffer_init(struct iio_dev *indio)
> -{
> -	return devm_iio_triggered_buffer_setup(&indio->dev, indio,
> -		&iio_pollfunc_store_time,
> -		&at91_adc_trigger_handler, &at91_buffer_setup_ops);
> -}
> -
>  static unsigned at91_adc_startup_time(unsigned startup_time_min,
>  				      unsigned adc_clk_khz)
>  {
> @@ -1683,6 +1661,40 @@ static const struct iio_info at91_adc_info = {
>  	.hwfifo_set_watermark = &at91_adc_set_watermark,
>  };
>  
> +static int at91_adc_buffer_and_trigger_init(struct device *dev,
> +					    struct iio_dev *indio)
> +{
> +	struct at91_adc_state *st = iio_priv(indio);
> +	int ret;
> +
> +	ret = devm_iio_triggered_buffer_setup(&indio->dev, indio,
> +		&iio_pollfunc_store_time,
> +		&at91_adc_trigger_handler, &at91_buffer_setup_ops);
> +	if (ret < 0) {
> +		dev_err(dev, "couldn't initialize the buffer.\n");
> +		return ret;
> +	}
> +
> +	if (!st->selected_trig->hw_trig)
> +		return 0;
> +
> +	iio_buffer_set_attrs(indio->buffer, at91_adc_fifo_attributes);
> +
> +	st->trig = at91_adc_allocate_trigger(indio, st->selected_trig->name);
> +	if (IS_ERR(st->trig)) {
> +		dev_err(dev, "could not allocate trigger\n");
> +		return PTR_ERR(st->trig);
> +	}
> +
> +	/*
> +	 * Initially the iio buffer has a length of 2 and
> +	 * a watermark of 1
> +	 */
> +	st->dma_st.watermark = 1;
> +
> +	return 0;
> +}
> +
>  static int at91_adc_probe(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev;
> @@ -1818,27 +1830,9 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> -	ret = at91_adc_buffer_init(indio_dev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "couldn't initialize the buffer.\n");
> +	ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);
> +	if (ret < 0)
>  		goto per_clk_disable_unprepare;
> -	}
> -
> -	if (st->selected_trig->hw_trig) {
> -		ret = at91_adc_trigger_init(indio_dev);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "couldn't setup the triggers.\n");
> -			goto per_clk_disable_unprepare;
> -		}
> -		/*
> -		 * Initially the iio buffer has a length of 2 and
> -		 * a watermark of 1
> -		 */
> -		st->dma_st.watermark = 1;
> -
> -		iio_buffer_set_attrs(indio_dev->buffer,
> -				     at91_adc_fifo_attributes);
> -	}
>  
>  	if (dma_coerce_mask_and_coherent(&indio_dev->dev, DMA_BIT_MASK(32)))
>  		dev_info(&pdev->dev, "cannot set DMA mask to 32-bit\n");


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-26 15:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 10:29 [PATCH] iio: adc: at91-sama5d2_adc: merge buffer & trigger init into a function Alexandru Ardelean
2020-09-26 15:55 ` Jonathan Cameron

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).