Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-24 Thread Jonathan Cameron 


On 24 January 2017 14:43:56 GMT+00:00, Fabrice Gasnier  
wrote:
>On 01/22/2017 02:14 PM, Jonathan Cameron wrote:
>> On 19/01/17 13:34, Fabrice Gasnier wrote:
>>> Add optional DMA support to STM32 ADC.
>>> Use dma cyclic mode with at least two periods.
>>>
>>> Signed-off-by: Fabrice Gasnier 
>> What is the point going forward in supporting non dma buffered reads
>at all?
>> Is there hardware that doesn't have DMA support?
>
>Hi Jonathan,
>
>Basically no, but there is a limited number of DMA request lines.
>DMA request lines mapping is documented in STM32F4 reference manual
>(DMA1/2 request mapping).
>There may be a situation where user (in dt) assign all DMA channels to
>other IPs. Then only choice would be to fall back using interrupt mode 
>for ADC.
>This is why I'd like to keep support for both interrupt and DMA modes.
Ah. Fair enough.  Thanks for the explanation. Perhaps a note in the patch 
description
would give us something to point at in future or even something in the bonding 
docs?
>
>> Just strikes me that the driver would be slight simpler if we dropped
>that
>> support.
>>
>> Various comments inline.  Mostly around crossing the boundary to
>fetch
>> from the IIO specific buffer (indio_dev->buffer).  That should never
>be
>> used directly as we can have multiple consumers of the datastream so
>> the numbers you get from that may represent only part of what is
>going on.
>>
>>
>>> ---
>>>  drivers/iio/adc/Kconfig  |   2 +
>>>  drivers/iio/adc/stm32-adc-core.c |   1 +
>>>  drivers/iio/adc/stm32-adc-core.h |   2 +
>>>  drivers/iio/adc/stm32-adc.c  | 209
>---
>>>  4 files changed, 202 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 9a7b090..2a2ef78 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
>>>  config STM32_ADC_CORE
>>> tristate "STMicroelectronics STM32 adc core"
>>> depends on ARCH_STM32 || COMPILE_TEST
>>> +   depends on HAS_DMA
>>> depends on OF
>>> depends on REGULATOR
>>> select IIO_BUFFER
>>> select MFD_STM32_TIMERS
>>> select IIO_STM32_TIMER_TRIGGER
>>> select IIO_TRIGGERED_BUFFER
>>> +   select IRQ_WORK
>>> help
>>>   Select this option to enable the core driver for
>STMicroelectronics
>>>   STM32 analog-to-digital converter (ADC).
>>> diff --git a/drivers/iio/adc/stm32-adc-core.c
>b/drivers/iio/adc/stm32-adc-core.c
>>> index 4214b0c..22b7c93 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.c
>>> +++ b/drivers/iio/adc/stm32-adc-core.c
>>> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct
>platform_device *pdev)
>>> priv->common.base = devm_ioremap_resource(>dev, res);
>>> if (IS_ERR(priv->common.base))
>>> return PTR_ERR(priv->common.base);
>>> +   priv->common.phys_base = res->start;
>>>
>>> priv->vref = devm_regulator_get(>dev, "vref");
>>> if (IS_ERR(priv->vref)) {
>>> diff --git a/drivers/iio/adc/stm32-adc-core.h
>b/drivers/iio/adc/stm32-adc-core.h
>>> index 081fa5f..2ec7abb 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.h
>>> +++ b/drivers/iio/adc/stm32-adc-core.h
>>> @@ -42,10 +42,12 @@
>>>  /**
>>>   * struct stm32_adc_common - stm32 ADC driver common data (for all
>instances)
>>>   * @base:  control registers base cpu addr
>>> + * @phys_base: control registers base physical addr
>>>   * @vref_mv:   vref voltage (mv)
>>>   */
>>>  struct stm32_adc_common {
>>> void __iomem*base;
>>> +   phys_addr_t phys_base;
>>> int vref_mv;
>>>  };
>>>
>>> diff --git a/drivers/iio/adc/stm32-adc.c
>b/drivers/iio/adc/stm32-adc.c
>>> index 9753c39..3439f4c 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -21,6 +21,8 @@
>>>
>>>  #include 
>>>  #include 
>>> +#include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -29,6 +31,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -69,6 +72,8 @@
>>>  #define STM32F4_EXTSEL_SHIFT   24
>>>  #define STM32F4_EXTSEL_MASKGENMASK(27, 24)
>>>  #define STM32F4_EOCS   BIT(10)
>>> +#define STM32F4_DDSBIT(9)
>>> +#define STM32F4_DMABIT(8)
>>>  #define STM32F4_ADON   BIT(0)
>>>
>>>  /* STM32F4_ADC_SQR1 - bit fields */
>>> @@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
>>>   * @bufi:  data buffer index
>>>   * @num_conv:  expected number of scan conversions
>>>   * @exten: external trigger config (enable/polarity)
>>> + * @work:  irq work used to call trigger poll routine
>>> + * @dma_chan:  dma channel
>>> + * @rx_buf:dma rx buffer cpu address
>>>

Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-24 Thread Jonathan Cameron 


On 24 January 2017 14:43:56 GMT+00:00, Fabrice Gasnier  
wrote:
>On 01/22/2017 02:14 PM, Jonathan Cameron wrote:
>> On 19/01/17 13:34, Fabrice Gasnier wrote:
>>> Add optional DMA support to STM32 ADC.
>>> Use dma cyclic mode with at least two periods.
>>>
>>> Signed-off-by: Fabrice Gasnier 
>> What is the point going forward in supporting non dma buffered reads
>at all?
>> Is there hardware that doesn't have DMA support?
>
>Hi Jonathan,
>
>Basically no, but there is a limited number of DMA request lines.
>DMA request lines mapping is documented in STM32F4 reference manual
>(DMA1/2 request mapping).
>There may be a situation where user (in dt) assign all DMA channels to
>other IPs. Then only choice would be to fall back using interrupt mode 
>for ADC.
>This is why I'd like to keep support for both interrupt and DMA modes.
Ah. Fair enough.  Thanks for the explanation. Perhaps a note in the patch 
description
would give us something to point at in future or even something in the bonding 
docs?
>
>> Just strikes me that the driver would be slight simpler if we dropped
>that
>> support.
>>
>> Various comments inline.  Mostly around crossing the boundary to
>fetch
>> from the IIO specific buffer (indio_dev->buffer).  That should never
>be
>> used directly as we can have multiple consumers of the datastream so
>> the numbers you get from that may represent only part of what is
>going on.
>>
>>
>>> ---
>>>  drivers/iio/adc/Kconfig  |   2 +
>>>  drivers/iio/adc/stm32-adc-core.c |   1 +
>>>  drivers/iio/adc/stm32-adc-core.h |   2 +
>>>  drivers/iio/adc/stm32-adc.c  | 209
>---
>>>  4 files changed, 202 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 9a7b090..2a2ef78 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
>>>  config STM32_ADC_CORE
>>> tristate "STMicroelectronics STM32 adc core"
>>> depends on ARCH_STM32 || COMPILE_TEST
>>> +   depends on HAS_DMA
>>> depends on OF
>>> depends on REGULATOR
>>> select IIO_BUFFER
>>> select MFD_STM32_TIMERS
>>> select IIO_STM32_TIMER_TRIGGER
>>> select IIO_TRIGGERED_BUFFER
>>> +   select IRQ_WORK
>>> help
>>>   Select this option to enable the core driver for
>STMicroelectronics
>>>   STM32 analog-to-digital converter (ADC).
>>> diff --git a/drivers/iio/adc/stm32-adc-core.c
>b/drivers/iio/adc/stm32-adc-core.c
>>> index 4214b0c..22b7c93 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.c
>>> +++ b/drivers/iio/adc/stm32-adc-core.c
>>> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct
>platform_device *pdev)
>>> priv->common.base = devm_ioremap_resource(>dev, res);
>>> if (IS_ERR(priv->common.base))
>>> return PTR_ERR(priv->common.base);
>>> +   priv->common.phys_base = res->start;
>>>
>>> priv->vref = devm_regulator_get(>dev, "vref");
>>> if (IS_ERR(priv->vref)) {
>>> diff --git a/drivers/iio/adc/stm32-adc-core.h
>b/drivers/iio/adc/stm32-adc-core.h
>>> index 081fa5f..2ec7abb 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.h
>>> +++ b/drivers/iio/adc/stm32-adc-core.h
>>> @@ -42,10 +42,12 @@
>>>  /**
>>>   * struct stm32_adc_common - stm32 ADC driver common data (for all
>instances)
>>>   * @base:  control registers base cpu addr
>>> + * @phys_base: control registers base physical addr
>>>   * @vref_mv:   vref voltage (mv)
>>>   */
>>>  struct stm32_adc_common {
>>> void __iomem*base;
>>> +   phys_addr_t phys_base;
>>> int vref_mv;
>>>  };
>>>
>>> diff --git a/drivers/iio/adc/stm32-adc.c
>b/drivers/iio/adc/stm32-adc.c
>>> index 9753c39..3439f4c 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -21,6 +21,8 @@
>>>
>>>  #include 
>>>  #include 
>>> +#include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -29,6 +31,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -69,6 +72,8 @@
>>>  #define STM32F4_EXTSEL_SHIFT   24
>>>  #define STM32F4_EXTSEL_MASKGENMASK(27, 24)
>>>  #define STM32F4_EOCS   BIT(10)
>>> +#define STM32F4_DDSBIT(9)
>>> +#define STM32F4_DMABIT(8)
>>>  #define STM32F4_ADON   BIT(0)
>>>
>>>  /* STM32F4_ADC_SQR1 - bit fields */
>>> @@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
>>>   * @bufi:  data buffer index
>>>   * @num_conv:  expected number of scan conversions
>>>   * @exten: external trigger config (enable/polarity)
>>> + * @work:  irq work used to call trigger poll routine
>>> + * @dma_chan:  dma channel
>>> + * @rx_buf:dma rx buffer cpu address
>>> + * @rx_dma_buf:dma rx buffer bus

Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-24 Thread Fabrice Gasnier 

On 01/22/2017 02:14 PM, Jonathan Cameron wrote:

On 19/01/17 13:34, Fabrice Gasnier wrote:

Add optional DMA support to STM32 ADC.
Use dma cyclic mode with at least two periods.

Signed-off-by: Fabrice Gasnier 

What is the point going forward in supporting non dma buffered reads at all?
Is there hardware that doesn't have DMA support?


Hi Jonathan,

Basically no, but there is a limited number of DMA request lines.
DMA request lines mapping is documented in STM32F4 reference manual
(DMA1/2 request mapping).
There may be a situation where user (in dt) assign all DMA channels to
other IPs. Then only choice would be to fall back using interrupt mode 
for ADC.

This is why I'd like to keep support for both interrupt and DMA modes.


Just strikes me that the driver would be slight simpler if we dropped that
support.

Various comments inline.  Mostly around crossing the boundary to fetch
from the IIO specific buffer (indio_dev->buffer).  That should never be
used directly as we can have multiple consumers of the datastream so
the numbers you get from that may represent only part of what is going on.



---
 drivers/iio/adc/Kconfig  |   2 +
 drivers/iio/adc/stm32-adc-core.c |   1 +
 drivers/iio/adc/stm32-adc-core.h |   2 +
 drivers/iio/adc/stm32-adc.c  | 209 ---
 4 files changed, 202 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9a7b090..2a2ef78 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
 config STM32_ADC_CORE
tristate "STMicroelectronics STM32 adc core"
depends on ARCH_STM32 || COMPILE_TEST
+   depends on HAS_DMA
depends on OF
depends on REGULATOR
select IIO_BUFFER
select MFD_STM32_TIMERS
select IIO_STM32_TIMER_TRIGGER
select IIO_TRIGGERED_BUFFER
+   select IRQ_WORK
help
  Select this option to enable the core driver for STMicroelectronics
  STM32 analog-to-digital converter (ADC).
diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index 4214b0c..22b7c93 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
priv->common.base = devm_ioremap_resource(>dev, res);
if (IS_ERR(priv->common.base))
return PTR_ERR(priv->common.base);
+   priv->common.phys_base = res->start;

priv->vref = devm_regulator_get(>dev, "vref");
if (IS_ERR(priv->vref)) {
diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
index 081fa5f..2ec7abb 100644
--- a/drivers/iio/adc/stm32-adc-core.h
+++ b/drivers/iio/adc/stm32-adc-core.h
@@ -42,10 +42,12 @@
 /**
  * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
  * @base:  control registers base cpu addr
+ * @phys_base: control registers base physical addr
  * @vref_mv:   vref voltage (mv)
  */
 struct stm32_adc_common {
void __iomem*base;
+   phys_addr_t phys_base;
int vref_mv;
 };

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 9753c39..3439f4c 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -21,6 +21,8 @@

 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -29,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -69,6 +72,8 @@
 #define STM32F4_EXTSEL_SHIFT   24
 #define STM32F4_EXTSEL_MASKGENMASK(27, 24)
 #define STM32F4_EOCS   BIT(10)
+#define STM32F4_DDSBIT(9)
+#define STM32F4_DMABIT(8)
 #define STM32F4_ADON   BIT(0)

 /* STM32F4_ADC_SQR1 - bit fields */
@@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
  * @bufi:  data buffer index
  * @num_conv:  expected number of scan conversions
  * @exten: external trigger config (enable/polarity)
+ * @work:  irq work used to call trigger poll routine
+ * @dma_chan:  dma channel
+ * @rx_buf:dma rx buffer cpu address
+ * @rx_dma_buf:dma rx buffer bus address
+ * @rx_buf_sz: dma rx buffer size
  */
 struct stm32_adc {
struct stm32_adc_common *common;
@@ -177,6 +187,11 @@ struct stm32_adc {
int bufi;
int num_conv;
enum stm32_adc_extenexten;
+   struct irq_work work;
+   struct dma_chan *dma_chan;
+   u8  *rx_buf;
+   dma_addr_t  rx_dma_buf;
+   int rx_buf_sz;
 };

 /**
@@ -332,10 +347,20 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc 
*adc)

Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-24 Thread Fabrice Gasnier 

On 01/22/2017 02:14 PM, Jonathan Cameron wrote:

On 19/01/17 13:34, Fabrice Gasnier wrote:

Add optional DMA support to STM32 ADC.
Use dma cyclic mode with at least two periods.

Signed-off-by: Fabrice Gasnier 

What is the point going forward in supporting non dma buffered reads at all?
Is there hardware that doesn't have DMA support?


Hi Jonathan,

Basically no, but there is a limited number of DMA request lines.
DMA request lines mapping is documented in STM32F4 reference manual
(DMA1/2 request mapping).
There may be a situation where user (in dt) assign all DMA channels to
other IPs. Then only choice would be to fall back using interrupt mode 
for ADC.

This is why I'd like to keep support for both interrupt and DMA modes.


Just strikes me that the driver would be slight simpler if we dropped that
support.

Various comments inline.  Mostly around crossing the boundary to fetch
from the IIO specific buffer (indio_dev->buffer).  That should never be
used directly as we can have multiple consumers of the datastream so
the numbers you get from that may represent only part of what is going on.



---
 drivers/iio/adc/Kconfig  |   2 +
 drivers/iio/adc/stm32-adc-core.c |   1 +
 drivers/iio/adc/stm32-adc-core.h |   2 +
 drivers/iio/adc/stm32-adc.c  | 209 ---
 4 files changed, 202 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9a7b090..2a2ef78 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
 config STM32_ADC_CORE
tristate "STMicroelectronics STM32 adc core"
depends on ARCH_STM32 || COMPILE_TEST
+   depends on HAS_DMA
depends on OF
depends on REGULATOR
select IIO_BUFFER
select MFD_STM32_TIMERS
select IIO_STM32_TIMER_TRIGGER
select IIO_TRIGGERED_BUFFER
+   select IRQ_WORK
help
  Select this option to enable the core driver for STMicroelectronics
  STM32 analog-to-digital converter (ADC).
diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index 4214b0c..22b7c93 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
priv->common.base = devm_ioremap_resource(>dev, res);
if (IS_ERR(priv->common.base))
return PTR_ERR(priv->common.base);
+   priv->common.phys_base = res->start;

priv->vref = devm_regulator_get(>dev, "vref");
if (IS_ERR(priv->vref)) {
diff --git a/drivers/iio/adc/stm32-adc-core.h b/drivers/iio/adc/stm32-adc-core.h
index 081fa5f..2ec7abb 100644
--- a/drivers/iio/adc/stm32-adc-core.h
+++ b/drivers/iio/adc/stm32-adc-core.h
@@ -42,10 +42,12 @@
 /**
  * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
  * @base:  control registers base cpu addr
+ * @phys_base: control registers base physical addr
  * @vref_mv:   vref voltage (mv)
  */
 struct stm32_adc_common {
void __iomem*base;
+   phys_addr_t phys_base;
int vref_mv;
 };

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 9753c39..3439f4c 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -21,6 +21,8 @@

 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -29,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -69,6 +72,8 @@
 #define STM32F4_EXTSEL_SHIFT   24
 #define STM32F4_EXTSEL_MASKGENMASK(27, 24)
 #define STM32F4_EOCS   BIT(10)
+#define STM32F4_DDSBIT(9)
+#define STM32F4_DMABIT(8)
 #define STM32F4_ADON   BIT(0)

 /* STM32F4_ADC_SQR1 - bit fields */
@@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
  * @bufi:  data buffer index
  * @num_conv:  expected number of scan conversions
  * @exten: external trigger config (enable/polarity)
+ * @work:  irq work used to call trigger poll routine
+ * @dma_chan:  dma channel
+ * @rx_buf:dma rx buffer cpu address
+ * @rx_dma_buf:dma rx buffer bus address
+ * @rx_buf_sz: dma rx buffer size
  */
 struct stm32_adc {
struct stm32_adc_common *common;
@@ -177,6 +187,11 @@ struct stm32_adc {
int bufi;
int num_conv;
enum stm32_adc_extenexten;
+   struct irq_work work;
+   struct dma_chan *dma_chan;
+   u8  *rx_buf;
+   dma_addr_t  rx_dma_buf;
+   int rx_buf_sz;
 };

 /**
@@ -332,10 +347,20 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc 
*adc)
 /**
  *

Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-22 Thread Jonathan Cameron 
On 19/01/17 13:34, Fabrice Gasnier wrote:
> Add optional DMA support to STM32 ADC.
> Use dma cyclic mode with at least two periods.
> 
> Signed-off-by: Fabrice Gasnier 
What is the point going forward in supporting non dma buffered reads at all?
Is there hardware that doesn't have DMA support?
Just strikes me that the driver would be slight simpler if we dropped that
support.

Various comments inline.  Mostly around crossing the boundary to fetch
from the IIO specific buffer (indio_dev->buffer).  That should never be
used directly as we can have multiple consumers of the datastream so
the numbers you get from that may represent only part of what is going on.


> ---
>  drivers/iio/adc/Kconfig  |   2 +
>  drivers/iio/adc/stm32-adc-core.c |   1 +
>  drivers/iio/adc/stm32-adc-core.h |   2 +
>  drivers/iio/adc/stm32-adc.c  | 209 
> ---
>  4 files changed, 202 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9a7b090..2a2ef78 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
>  config STM32_ADC_CORE
>   tristate "STMicroelectronics STM32 adc core"
>   depends on ARCH_STM32 || COMPILE_TEST
> + depends on HAS_DMA
>   depends on OF
>   depends on REGULATOR
>   select IIO_BUFFER
>   select MFD_STM32_TIMERS
>   select IIO_STM32_TIMER_TRIGGER
>   select IIO_TRIGGERED_BUFFER
> + select IRQ_WORK
>   help
> Select this option to enable the core driver for STMicroelectronics
> STM32 analog-to-digital converter (ADC).
> diff --git a/drivers/iio/adc/stm32-adc-core.c 
> b/drivers/iio/adc/stm32-adc-core.c
> index 4214b0c..22b7c93 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>   priv->common.base = devm_ioremap_resource(>dev, res);
>   if (IS_ERR(priv->common.base))
>   return PTR_ERR(priv->common.base);
> + priv->common.phys_base = res->start;
>  
>   priv->vref = devm_regulator_get(>dev, "vref");
>   if (IS_ERR(priv->vref)) {
> diff --git a/drivers/iio/adc/stm32-adc-core.h 
> b/drivers/iio/adc/stm32-adc-core.h
> index 081fa5f..2ec7abb 100644
> --- a/drivers/iio/adc/stm32-adc-core.h
> +++ b/drivers/iio/adc/stm32-adc-core.h
> @@ -42,10 +42,12 @@
>  /**
>   * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>   * @base:control registers base cpu addr
> + * @phys_base:   control registers base physical addr
>   * @vref_mv: vref voltage (mv)
>   */
>  struct stm32_adc_common {
>   void __iomem*base;
> + phys_addr_t phys_base;
>   int vref_mv;
>  };
>  
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9753c39..3439f4c 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -21,6 +21,8 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -29,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -69,6 +72,8 @@
>  #define STM32F4_EXTSEL_SHIFT 24
>  #define STM32F4_EXTSEL_MASK  GENMASK(27, 24)
>  #define STM32F4_EOCS BIT(10)
> +#define STM32F4_DDS  BIT(9)
> +#define STM32F4_DMA  BIT(8)
>  #define STM32F4_ADON BIT(0)
>  
>  /* STM32F4_ADC_SQR1 - bit fields */
> @@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
>   * @bufi:data buffer index
>   * @num_conv:expected number of scan conversions
>   * @exten:   external trigger config (enable/polarity)
> + * @work:irq work used to call trigger poll routine
> + * @dma_chan:dma channel
> + * @rx_buf:  dma rx buffer cpu address
> + * @rx_dma_buf:  dma rx buffer bus address
> + * @rx_buf_sz:   dma rx buffer size
>   */
>  struct stm32_adc {
>   struct stm32_adc_common *common;
> @@ -177,6 +187,11 @@ struct stm32_adc {
>   int bufi;
>   int num_conv;
>   enum stm32_adc_extenexten;
> + struct irq_work work;
> + struct dma_chan *dma_chan;
> + u8  *rx_buf;
> + dma_addr_t  rx_dma_buf;
> + int rx_buf_sz;
>  };
>  
>  /**
> @@ -332,10 +347,20 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc 
> *adc)
>  /**
>   * stm32_adc_start_conv() - Start conversions for regular channels.
>   * @adc: stm32 adc instance
> + *
> + * Start conversions for regular channels.
> + * Also take care of normal or DMA mode. DMA is used in circular mode for
> + * regular conversions, in IIO buffer

Re: [PATCH 5/7] iio: adc: stm32: add optional dma support

2017-01-22 Thread Jonathan Cameron 
On 19/01/17 13:34, Fabrice Gasnier wrote:
> Add optional DMA support to STM32 ADC.
> Use dma cyclic mode with at least two periods.
> 
> Signed-off-by: Fabrice Gasnier 
What is the point going forward in supporting non dma buffered reads at all?
Is there hardware that doesn't have DMA support?
Just strikes me that the driver would be slight simpler if we dropped that
support.

Various comments inline.  Mostly around crossing the boundary to fetch
from the IIO specific buffer (indio_dev->buffer).  That should never be
used directly as we can have multiple consumers of the datastream so
the numbers you get from that may represent only part of what is going on.


> ---
>  drivers/iio/adc/Kconfig  |   2 +
>  drivers/iio/adc/stm32-adc-core.c |   1 +
>  drivers/iio/adc/stm32-adc-core.h |   2 +
>  drivers/iio/adc/stm32-adc.c  | 209 
> ---
>  4 files changed, 202 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9a7b090..2a2ef78 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -444,12 +444,14 @@ config ROCKCHIP_SARADC
>  config STM32_ADC_CORE
>   tristate "STMicroelectronics STM32 adc core"
>   depends on ARCH_STM32 || COMPILE_TEST
> + depends on HAS_DMA
>   depends on OF
>   depends on REGULATOR
>   select IIO_BUFFER
>   select MFD_STM32_TIMERS
>   select IIO_STM32_TIMER_TRIGGER
>   select IIO_TRIGGERED_BUFFER
> + select IRQ_WORK
>   help
> Select this option to enable the core driver for STMicroelectronics
> STM32 analog-to-digital converter (ADC).
> diff --git a/drivers/iio/adc/stm32-adc-core.c 
> b/drivers/iio/adc/stm32-adc-core.c
> index 4214b0c..22b7c93 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -201,6 +201,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>   priv->common.base = devm_ioremap_resource(>dev, res);
>   if (IS_ERR(priv->common.base))
>   return PTR_ERR(priv->common.base);
> + priv->common.phys_base = res->start;
>  
>   priv->vref = devm_regulator_get(>dev, "vref");
>   if (IS_ERR(priv->vref)) {
> diff --git a/drivers/iio/adc/stm32-adc-core.h 
> b/drivers/iio/adc/stm32-adc-core.h
> index 081fa5f..2ec7abb 100644
> --- a/drivers/iio/adc/stm32-adc-core.h
> +++ b/drivers/iio/adc/stm32-adc-core.h
> @@ -42,10 +42,12 @@
>  /**
>   * struct stm32_adc_common - stm32 ADC driver common data (for all instances)
>   * @base:control registers base cpu addr
> + * @phys_base:   control registers base physical addr
>   * @vref_mv: vref voltage (mv)
>   */
>  struct stm32_adc_common {
>   void __iomem*base;
> + phys_addr_t phys_base;
>   int vref_mv;
>  };
>  
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 9753c39..3439f4c 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -21,6 +21,8 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -29,6 +31,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -69,6 +72,8 @@
>  #define STM32F4_EXTSEL_SHIFT 24
>  #define STM32F4_EXTSEL_MASK  GENMASK(27, 24)
>  #define STM32F4_EOCS BIT(10)
> +#define STM32F4_DDS  BIT(9)
> +#define STM32F4_DMA  BIT(8)
>  #define STM32F4_ADON BIT(0)
>  
>  /* STM32F4_ADC_SQR1 - bit fields */
> @@ -165,6 +170,11 @@ struct stm32_adc_trig_info {
>   * @bufi:data buffer index
>   * @num_conv:expected number of scan conversions
>   * @exten:   external trigger config (enable/polarity)
> + * @work:irq work used to call trigger poll routine
> + * @dma_chan:dma channel
> + * @rx_buf:  dma rx buffer cpu address
> + * @rx_dma_buf:  dma rx buffer bus address
> + * @rx_buf_sz:   dma rx buffer size
>   */
>  struct stm32_adc {
>   struct stm32_adc_common *common;
> @@ -177,6 +187,11 @@ struct stm32_adc {
>   int bufi;
>   int num_conv;
>   enum stm32_adc_extenexten;
> + struct irq_work work;
> + struct dma_chan *dma_chan;
> + u8  *rx_buf;
> + dma_addr_t  rx_dma_buf;
> + int rx_buf_sz;
>  };
>  
>  /**
> @@ -332,10 +347,20 @@ static void stm32_adc_conv_irq_disable(struct stm32_adc 
> *adc)
>  /**
>   * stm32_adc_start_conv() - Start conversions for regular channels.
>   * @adc: stm32 adc instance
> + *
> + * Start conversions for regular channels.
> + * Also take care of normal or DMA mode. DMA is used in circular mode for
> + * regular conversions, in IIO buffer modes. Rely on rx_buf