[PATCH 0/4] Add DMA support for ti_am335x_adc driver

[PATCH 0/4] Add DMA support for ti_am335x_adc driver

[Mugunthan V N]

The ADC has a 64 work depth fifo length which holds the ADC data
till the CPU reads. So when a user program needs a large ADC data
to operate on, then it has to do multiple reads to get its
buffer. Currently if the application asks for 4 samples per
channel with all 8 channels are enabled, kernel can provide only
3 samples per channel when all 8 channels are enabled (logs at
[1]). So with DMA support user can request for large number of
samples at a time (logs at [2]).

Tested the patch on AM437x-gp-evm and AM335x Boneblack with the
patch [3] to enable ADC and pushed a branch for testing [4]

[1] - http://pastebin.ubuntu.com/23211490/
[2] - http://pastebin.ubuntu.com/23211492/
[3] - http://pastebin.ubuntu.com/23211494/
[4] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git iio-dma

Mugunthan V N (4):
  mfd: ti_am335x_tscadc: store physical address
  drivers: iio: ti_am335x_adc: add dma support
  ARM: dts: am33xx: add DMA properties for tscadc
  ARM: dts: am4372: add DMA properties for tscadc

 arch/arm/boot/dts/am33xx.dtsi        |   2 +
 arch/arm/boot/dts/am4372.dtsi        |   2 +
 drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
 drivers/mfd/ti_am335x_tscadc.c       |   1 +
 include/linux/mfd/ti_am335x_tscadc.h |   8 ++
 5 files changed, 170 insertions(+), 3 deletions(-)

-- 
2.10.0.129.g35f6318

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Mugunthan V N]

store the physical address of the device in its priv to use it
for DMA addressing in the client drivers.

Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
---
 drivers/mfd/ti_am335x_tscadc.c       | 1 +
 include/linux/mfd/ti_am335x_tscadc.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
index c8f027b..0f3fab4 100644
--- a/drivers/mfd/ti_am335x_tscadc.c
+++ b/drivers/mfd/ti_am335x_tscadc.c
@@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
 		tscadc->irq = err;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	tscadc->tscadc_phys_base = res->start;
 	tscadc->tscadc_base = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(tscadc->tscadc_base))
 		return PTR_ERR(tscadc->tscadc_base);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index 7f55b8b..e45a208 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -155,6 +155,7 @@ struct ti_tscadc_dev {
 	struct device *dev;
 	struct regmap *regmap;
 	void __iomem *tscadc_base;
+	phys_addr_t tscadc_phys_base;
 	int irq;
 	int used_cells;	/* 1-2 */
 	int tsc_wires;
-- 
2.10.0.129.g35f6318

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Lee Jones]

On Wed, 21 Sep 2016, Mugunthan V N wrote:

> store the physical address of the device in its priv to use it
> for DMA addressing in the client drivers.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index c8f027b..0f3fab4 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>  		tscadc->irq = err;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	tscadc->tscadc_phys_base = res->start;

This is unusual.  Can't you use a virt_to_phys() variant instead?

>  	tscadc->tscadc_base = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(tscadc->tscadc_base))
>  		return PTR_ERR(tscadc->tscadc_base);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index 7f55b8b..e45a208 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -155,6 +155,7 @@ struct ti_tscadc_dev {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	void __iomem *tscadc_base;
> +	phys_addr_t tscadc_phys_base;
>  	int irq;
>  	int used_cells;	/* 1-2 */
>  	int tsc_wires;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Mugunthan V N]

On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
> On Wed, 21 Sep 2016, Mugunthan V N wrote:
> 
>> store the physical address of the device in its priv to use it
>> for DMA addressing in the client drivers.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
>>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>> index c8f027b..0f3fab4 100644
>> --- a/drivers/mfd/ti_am335x_tscadc.c
>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>  		tscadc->irq = err;
>>  
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	tscadc->tscadc_phys_base = res->start;
> 
> This is unusual.  Can't you use a virt_to_phys() variant instead?
> 

I tried using virt_to_phys(), but its not working for me.
Also saw many drivers uses like this to get physical address
("git grep -n " res->start;" drivers/*").

Regards
Mugunthan V N

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Lee Jones]

On Fri, 30 Sep 2016, Mugunthan V N wrote:

> On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
> > On Wed, 21 Sep 2016, Mugunthan V N wrote:
> > 
> >> store the physical address of the device in its priv to use it
> >> for DMA addressing in the client drivers.
> >>
> >> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> >> ---
> >>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
> >>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
> >>  2 files changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> >> index c8f027b..0f3fab4 100644
> >> --- a/drivers/mfd/ti_am335x_tscadc.c
> >> +++ b/drivers/mfd/ti_am335x_tscadc.c
> >> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> >>  		tscadc->irq = err;
> >>  
> >>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	tscadc->tscadc_phys_base = res->start;
> > 
> > This is unusual.  Can't you use a virt_to_phys() variant instead?
> > 
> 
> I tried using virt_to_phys(), but its not working for me.
> Also saw many drivers uses like this to get physical address
> ("git grep -n " res->start;" drivers/*").

Very well:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Let me know how you wish this set to be handled.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Jonathan Cameron]

On 26/10/16 13:17, Lee Jones wrote:
> On Fri, 30 Sep 2016, Mugunthan V N wrote:
> 
>> On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
>>> On Wed, 21 Sep 2016, Mugunthan V N wrote:
>>>
>>>> store the physical address of the device in its priv to use it
>>>> for DMA addressing in the client drivers.
>>>>
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> ---
>>>>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
>>>>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>>>> index c8f027b..0f3fab4 100644
>>>> --- a/drivers/mfd/ti_am335x_tscadc.c
>>>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>>>> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>>>  		tscadc->irq = err;
>>>>  
>>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	tscadc->tscadc_phys_base = res->start;
>>>
>>> This is unusual.  Can't you use a virt_to_phys() variant instead?
>>>
>>
>> I tried using virt_to_phys(), but its not working for me.
>> Also saw many drivers uses like this to get physical address
>> ("git grep -n " res->start;" drivers/*").
> 
> Very well:
> 
> For my own reference:
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> 
> Let me know how you wish this set to be handled.
I'm happy to pick up the whole series.  There are some more mfd
header changes in patch 2 but as they only add defines, I
don't mind that much if I don't an Ack from you on those
(btw this got to V3 but as patch 1 didn't change I'll carry
your ack forwards).

Do you want an immutable branch?  Seems unlikely to cause
much trouble even if there is a merge issue on all 10ish
lines of mfd code in the next merge window.

Jonathan
> 

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Lee Jones]

On Sun, 30 Oct 2016, Jonathan Cameron wrote:

> On 26/10/16 13:17, Lee Jones wrote:
> > On Fri, 30 Sep 2016, Mugunthan V N wrote:
> > 
> >> On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
> >>> On Wed, 21 Sep 2016, Mugunthan V N wrote:
> >>>
> >>>> store the physical address of the device in its priv to use it
> >>>> for DMA addressing in the client drivers.
> >>>>
> >>>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> >>>> ---
> >>>>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
> >>>>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
> >>>>  2 files changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> >>>> index c8f027b..0f3fab4 100644
> >>>> --- a/drivers/mfd/ti_am335x_tscadc.c
> >>>> +++ b/drivers/mfd/ti_am335x_tscadc.c
> >>>> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
> >>>>  		tscadc->irq = err;
> >>>>  
> >>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>> +	tscadc->tscadc_phys_base = res->start;
> >>>
> >>> This is unusual.  Can't you use a virt_to_phys() variant instead?
> >>>
> >>
> >> I tried using virt_to_phys(), but its not working for me.
> >> Also saw many drivers uses like this to get physical address
> >> ("git grep -n " res->start;" drivers/*").
> > 
> > Very well:
> > 
> > For my own reference:
> >   Acked-for-MFD-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > Let me know how you wish this set to be handled.
> I'm happy to pick up the whole series.  There are some more mfd
> header changes in patch 2 but as they only add defines, I
> don't mind that much if I don't an Ack from you on those
> (btw this got to V3 but as patch 1 didn't change I'll carry
> your ack forwards).
> 
> Do you want an immutable branch?  Seems unlikely to cause
> much trouble even if there is a merge issue on all 10ish
> lines of mfd code in the next merge window.

Not at the moment, but if you could set things up so it's possible to
create one at a later date if things go Pete Tong, that would be
great.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] mfd: ti_am335x_tscadc: store physical address

[Jonathan Cameron]

On 31/10/16 08:16, Lee Jones wrote:
> On Sun, 30 Oct 2016, Jonathan Cameron wrote:
> 
>> On 26/10/16 13:17, Lee Jones wrote:
>>> On Fri, 30 Sep 2016, Mugunthan V N wrote:
>>>
>>>> On Wednesday 28 September 2016 01:10 AM, Lee Jones wrote:
>>>>> On Wed, 21 Sep 2016, Mugunthan V N wrote:
>>>>>
>>>>>> store the physical address of the device in its priv to use it
>>>>>> for DMA addressing in the client drivers.
>>>>>>
>>>>>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>>  drivers/mfd/ti_am335x_tscadc.c       | 1 +
>>>>>>  include/linux/mfd/ti_am335x_tscadc.h | 1 +
>>>>>>  2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
>>>>>> index c8f027b..0f3fab4 100644
>>>>>> --- a/drivers/mfd/ti_am335x_tscadc.c
>>>>>> +++ b/drivers/mfd/ti_am335x_tscadc.c
>>>>>> @@ -183,6 +183,7 @@ static	int ti_tscadc_probe(struct platform_device *pdev)
>>>>>>  		tscadc->irq = err;
>>>>>>  
>>>>>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>>> +	tscadc->tscadc_phys_base = res->start;
>>>>>
>>>>> This is unusual.  Can't you use a virt_to_phys() variant instead?
>>>>>
>>>>
>>>> I tried using virt_to_phys(), but its not working for me.
>>>> Also saw many drivers uses like this to get physical address
>>>> ("git grep -n " res->start;" drivers/*").
>>>
>>> Very well:
>>>
>>> For my own reference:
>>>   Acked-for-MFD-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>
>>> Let me know how you wish this set to be handled.
>> I'm happy to pick up the whole series.  There are some more mfd
>> header changes in patch 2 but as they only add defines, I
>> don't mind that much if I don't an Ack from you on those
>> (btw this got to V3 but as patch 1 didn't change I'll carry
>> your ack forwards).
>>
>> Do you want an immutable branch?  Seems unlikely to cause
>> much trouble even if there is a merge issue on all 10ish
>> lines of mfd code in the next merge window.
> 
> Not at the moment, but if you could set things up so it's possible to
> create one at a later date if things go Pete Tong, that would be
> great.
Couldn't think of an easy way to do this without creating a branch
and merging it into my normal branch.  I'll not push it out to
kernel.org though unless you tell me you need it.

Applied to the togreg branch (indirectly ;) of iio.git pushed out
as testing for the autobuilders to play with it.

Thanks,

Jonathan

> 

Re: [PATCH 0/4] Add DMA support for ti_am335x_adc driver

[Jonathan Cameron]

On 21/09/16 17:11, Mugunthan V N wrote:
> The ADC has a 64 work depth fifo length which holds the ADC data
> till the CPU reads. So when a user program needs a large ADC data
> to operate on, then it has to do multiple reads to get its
> buffer. Currently if the application asks for 4 samples per
> channel with all 8 channels are enabled, kernel can provide only
> 3 samples per channel when all 8 channels are enabled (logs at
> [1]). So with DMA support user can request for large number of
> samples at a time (logs at [2]).
> 
> Tested the patch on AM437x-gp-evm and AM335x Boneblack with the
> patch [3] to enable ADC and pushed a branch for testing [4]
> 
> [1] - http://pastebin.ubuntu.com/23211490/
> [2] - http://pastebin.ubuntu.com/23211492/
> [3] - http://pastebin.ubuntu.com/23211494/
> [4] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git iio-dma
Just curious.  How fast is the ADC sampling at in these?  Never that
obvious for this driver!

I'm also curious as to whether you started to hit the limits of the
kfifo based interface.  Might be worth considering adding alternative
support for the dma buffers interface which is obviously much lower
overhead.

Good to have this work prior to that as the kfifo stuff is somewhat
easier to use.

Jonathan
> 
> Mugunthan V N (4):
>   mfd: ti_am335x_tscadc: store physical address
>   drivers: iio: ti_am335x_adc: add dma support
>   ARM: dts: am33xx: add DMA properties for tscadc
>   ARM: dts: am4372: add DMA properties for tscadc
> 
>  arch/arm/boot/dts/am33xx.dtsi        |   2 +
>  arch/arm/boot/dts/am4372.dtsi        |   2 +
>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>  drivers/mfd/ti_am335x_tscadc.c       |   1 +
>  include/linux/mfd/ti_am335x_tscadc.h |   8 ++
>  5 files changed, 170 insertions(+), 3 deletions(-)
> 

Re: [PATCH 0/4] Add DMA support for ti_am335x_adc driver

[Mugunthan V N]

On Sunday 25 September 2016 03:11 PM, Jonathan Cameron wrote:
> On 21/09/16 17:11, Mugunthan V N wrote:
>> > The ADC has a 64 work depth fifo length which holds the ADC data
>> > till the CPU reads. So when a user program needs a large ADC data
>> > to operate on, then it has to do multiple reads to get its
>> > buffer. Currently if the application asks for 4 samples per
>> > channel with all 8 channels are enabled, kernel can provide only
>> > 3 samples per channel when all 8 channels are enabled (logs at
>> > [1]). So with DMA support user can request for large number of
>> > samples at a time (logs at [2]).
>> > 
>> > Tested the patch on AM437x-gp-evm and AM335x Boneblack with the
>> > patch [3] to enable ADC and pushed a branch for testing [4]
>> > 
>> > [1] - http://pastebin.ubuntu.com/23211490/
>> > [2] - http://pastebin.ubuntu.com/23211492/
>> > [3] - http://pastebin.ubuntu.com/23211494/
>> > [4] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git iio-dma
> Just curious.  How fast is the ADC sampling at in these?  Never that
> obvious for this driver!
> 
> I'm also curious as to whether you started to hit the limits of the
> kfifo based interface.  Might be worth considering adding alternative
> support for the dma buffers interface which is obviously much lower
> overhead.
> 
> Good to have this work prior to that as the kfifo stuff is somewhat
> easier to use.

Currently ADC clock is 3MHz, which can produce a data rate of 225KBps
per channel with no open delay and no averaging of samples. So when all
8 Channels are enables the data rate will be 1.75MBps

ADC can be operated at 24MHz, which can generate a data rate of 28MBps
with all 8 channels enabled and no open delay and averaging, but our
target is to get 800K samples per second per channel which has a data
rate of 12.5MBps

I think with this data rate, DMA will be the best option to implement
without any data loss and less cpu overload to read the ADC samples.

Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] Add DMA support for ti_am335x_adc driver

[John Syne]

I applied your patches to the following kernel:

github.com/RobertCNelson/bb-kernel.git 
branch am33x-v4.8

Changing 
#define REG_DMAENABLE_CLEAR	0x038
to
#define REG_DMAENABLE_CLEAR	0x03C

Also applying the following DTS changes:

		target = <&tscadc>;
		__overlay__ {

			status = "okay";
			adc {
				ti,adc-channels = <0 1 2 3 4 5 6>;
				ti,chan-step-avg = <0x1 0x1 0x1 0x1 0x1 0x1 0x1>;
				ti,chan-step-opendelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
				ti,chan-step-sampledelay = <0x0 0x0 0x0 0x0 0x0 0x0 0x0>;
			};
		};

Running on a BeagleBoneBlack, I was able to stream samples at 200ksps continuously and it looked stable. 

htop showed CPU utilization between 5% and 7%

Thank you for getting this to work.

Regards,
John




> On Sep 29, 2016, at 6:01 AM, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> 
> On Sunday 25 September 2016 03:11 PM, Jonathan Cameron wrote:
>> On 21/09/16 17:11, Mugunthan V N wrote:
>>>> The ADC has a 64 work depth fifo length which holds the ADC data
>>>> till the CPU reads. So when a user program needs a large ADC data
>>>> to operate on, then it has to do multiple reads to get its
>>>> buffer. Currently if the application asks for 4 samples per
>>>> channel with all 8 channels are enabled, kernel can provide only
>>>> 3 samples per channel when all 8 channels are enabled (logs at
>>>> [1]). So with DMA support user can request for large number of
>>>> samples at a time (logs at [2]).
>>>> 
>>>> Tested the patch on AM437x-gp-evm and AM335x Boneblack with the
>>>> patch [3] to enable ADC and pushed a branch for testing [4]
>>>> 
>>>> [1] - http://pastebin.ubuntu.com/23211490/
>>>> [2] - http://pastebin.ubuntu.com/23211492/
>>>> [3] - http://pastebin.ubuntu.com/23211494/
>>>> [4] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git iio-dma
>> Just curious.  How fast is the ADC sampling at in these?  Never that
>> obvious for this driver!
>> 
>> I'm also curious as to whether you started to hit the limits of the
>> kfifo based interface.  Might be worth considering adding alternative
>> support for the dma buffers interface which is obviously much lower
>> overhead.
>> 
>> Good to have this work prior to that as the kfifo stuff is somewhat
>> easier to use.
> 
> Currently ADC clock is 3MHz, which can produce a data rate of 225KBps
> per channel with no open delay and no averaging of samples. So when all
> 8 Channels are enables the data rate will be 1.75MBps
> 
> ADC can be operated at 24MHz, which can generate a data rate of 28MBps
> with all 8 channels enabled and no open delay and averaging, but our
> target is to get 800K samples per second per channel which has a data
> rate of 12.5MBps
> 
> I think with this data rate, DMA will be the best option to implement
> without any data loss and less cpu overload to read the ADC samples.
> 
> Regards
> Mugunthan V N
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Mugunthan V N]

This patch adds the required pieces to ti_am335x_adc driver for
DMA support

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
 include/linux/mfd/ti_am335x_tscadc.h |   7 ++
 2 files changed, 164 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index c3cfacca..89d0b07 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -30,10 +30,32 @@
 #include <linux/iio/buffer.h>
 #include <linux/iio/kfifo_buf.h>
 
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+
+#define DMA_BUFFER_SIZE		SZ_2K
+
+struct tiadc_dma {
+	/* Filter function */
+	dma_filter_fn		fn;
+	/* Parameter to the filter function */
+	void			*param;
+	struct dma_slave_config	conf;
+	struct dma_chan		*chan;
+	dma_addr_t		addr;
+	dma_cookie_t		cookie;
+	u8			*buf;
+	bool			valid_buf_seg;
+	int			buf_offset;
+	u8			fifo_thresh;
+};
+
 struct tiadc_device {
 	struct ti_tscadc_dev *mfd_tscadc;
+	struct tiadc_dma dma;
 	struct mutex fifo1_lock; /* to protect fifo access */
 	int channels;
+	int total_ch_enabled;
 	u8 channel_line[8];
 	u8 channel_step[8];
 	int buffer_en_ch_steps;
@@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
 	u16 *data = adc_dev->data;
 
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+
 	for (k = 0; k < fifo1count; k = k + i) {
 		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
 			read = tiadc_readl(adc_dev, REG_FIFO1);
@@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
 	return IRQ_HANDLED;
 }
 
+static void tiadc_dma_rx_complete(void *param)
+{
+	struct iio_dev *indio_dev = param;
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct tiadc_dma *dma = &adc_dev->dma;
+	u8 *data;
+	int i;
+
+	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
+	dma->valid_buf_seg = !dma->valid_buf_seg;
+
+	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
+		iio_push_to_buffers(indio_dev, data);
+		data += indio_dev->scan_bytes;
+	}
+}
+
+static int tiadc_start_dma(struct iio_dev *indio_dev)
+{
+	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct tiadc_dma *dma = &adc_dev->dma;
+	struct dma_async_tx_descriptor *desc;
+
+	dma->valid_buf_seg = false;
+	dma->fifo_thresh = FIFO1_THRESHOLD;
+	/*
+	 * Make the fifo thresh as the multiple of total number of
+	 * channels enabled, so make sure that that cyclic DMA period
+	 * length is also a multiple of total number of channels
+	 * enabled. This ensures that no invalid data is reported
+	 * to the stack via iio_push_to_buffers().
+	 */
+	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
+	dma->buf_offset = DMA_BUFFER_SIZE / 2;
+	/* Make sure that period length is multiple of fifo thresh level */
+	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
+					      sizeof(u16));
+
+	dma->conf.src_maxburst = dma->fifo_thresh + 1;
+	dmaengine_slave_config(dma->chan, &dma->conf);
+
+	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
+					 dma->buf_offset * 2,
+					 dma->buf_offset, DMA_DEV_TO_MEM,
+					 DMA_PREP_INTERRUPT);
+	if (!desc)
+		return -EBUSY;
+
+	desc->callback = tiadc_dma_rx_complete;
+	desc->callback_param = indio_dev;
+
+	dma->cookie = dmaengine_submit(desc);
+
+	dma_async_issue_pending(dma->chan);
+
+	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
+	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
+	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
+
+	return 0;
+}
+
 static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
@@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
 static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct tiadc_dma *dma = &adc_dev->dma;
 	unsigned int enb = 0;
 	u8 bit;
+	u32 irq_enable;
 
 	tiadc_step_config(indio_dev);
-	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
+	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
 		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+		adc_dev->total_ch_enabled++;
+	}
 	adc_dev->buffer_en_ch_steps = enb;
 
+	if (dma->chan)
+		tiadc_start_dma(indio_dev);
+
 	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
 
 	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
 				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
-	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
-				| IRQENB_FIFO1OVRRUN);
+
+	irq_enable = IRQENB_FIFO1OVRRUN;
+	if (!dma->chan)
+		irq_enable |= IRQENB_FIFO1THRES;
+	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
 
 	return 0;
 }
@@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
 static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct tiadc_dma *dma = &adc_dev->dma;
 	int fifo1count, i, read;
 
 	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
 				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
 	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
 	adc_dev->buffer_en_ch_steps = 0;
+	adc_dev->total_ch_enabled = 0;
+	if (dma->chan) {
+		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
+		dmaengine_terminate_async(dma->chan);
+	}
 
 	/* Flush FIFO of leftover data in the time it takes to disable adc */
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
@@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
 	.driver_module = THIS_MODULE,
 };
 
+static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
+{
+	return false;
+}
+
+static int tiadc_request_dma(struct platform_device *pdev,
+			     struct tiadc_device *adc_dev)
+{
+	struct tiadc_dma	*dma = &adc_dev->dma;
+	dma_cap_mask_t		mask;
+
+	dma->fn = the_no_dma_filter_fn;
+
+	/* Default slave configuration parameters */
+	dma->conf.direction = DMA_DEV_TO_MEM;
+	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_CYCLIC, mask);
+
+	/* Get a channel for RX */
+	dma->chan = dma_request_slave_channel_compat(mask,
+						     dma->fn, dma->param,
+						     adc_dev->mfd_tscadc->dev,
+						     "fifo1");
+	if (!dma->chan)
+		return -ENODEV;
+
+	/* RX buffer */
+	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
+				      &dma->addr, GFP_KERNEL);
+	if (!dma->buf)
+		goto err;
+
+	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
+
+	return 0;
+err:
+	dma_release_channel(dma->chan);
+
+	return -ENOMEM;
+}
+
 static int tiadc_parse_dt(struct platform_device *pdev,
 			  struct tiadc_device *adc_dev)
 {
@@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, indio_dev);
 
+	err = tiadc_request_dma(pdev, adc_dev);
+	if (err && err != -ENODEV)
+		goto err_dma;
+
 	return 0;
 
+err_dma:
+	iio_device_unregister(indio_dev);
 err_buffer_unregister:
 	tiadc_iio_buffered_hardware_remove(indio_dev);
 err_free_channels:
@@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
+	struct tiadc_dma *dma = &adc_dev->dma;
 	u32 step_en;
 
+	if (dma->chan)
+		dma_release_channel(dma->chan);
 	iio_device_unregister(indio_dev);
 	tiadc_iio_buffered_hardware_remove(indio_dev);
 	tiadc_channels_remove(indio_dev);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index e45a208..fb9dc99 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -23,6 +23,8 @@
 #define REG_IRQENABLE		0x02C
 #define REG_IRQCLR		0x030
 #define REG_IRQWAKEUP		0x034
+#define REG_DMAENABLE_SET	0x038
+#define REG_DMAENABLE_CLEAR	0x038
 #define REG_CTRL		0x040
 #define REG_ADCFSM		0x044
 #define REG_CLKDIV		0x04C
@@ -36,6 +38,7 @@
 #define REG_FIFO0THR		0xE8
 #define REG_FIFO1CNT		0xF0
 #define REG_FIFO1THR		0xF4
+#define REG_DMA1REQ		0xF8
 #define REG_FIFO0		0x100
 #define REG_FIFO1		0x200
 
@@ -126,6 +129,10 @@
 #define FIFOREAD_DATA_MASK (0xfff << 0)
 #define FIFOREAD_CHNLID_MASK (0xf << 16)
 
+/* DMA ENABLE/CLEAR Register */
+#define DMA_FIFO0		BIT(0)
+#define DMA_FIFO1		BIT(1)
+
 /* Sequencer Status */
 #define SEQ_STATUS BIT(5)
 #define CHARGE_STEP		0x11
-- 
2.10.0.129.g35f6318

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Peter Meerwald-Stadler]

> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support

nitpicking below
 
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>  2 files changed, 164 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/kfifo_buf.h>
>  
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE		SZ_2K
> +
> +struct tiadc_dma {
> +	/* Filter function */
> +	dma_filter_fn		fn;
> +	/* Parameter to the filter function */
> +	void			*param;
> +	struct dma_slave_config	conf;
> +	struct dma_chan		*chan;
> +	dma_addr_t		addr;
> +	dma_cookie_t		cookie;
> +	u8			*buf;
> +	bool			valid_buf_seg;
> +	int			buf_offset;
> +	u8			fifo_thresh;
> +};
> +
>  struct tiadc_device {
>  	struct ti_tscadc_dev *mfd_tscadc;
> +	struct tiadc_dma dma;
>  	struct mutex fifo1_lock; /* to protect fifo access */
>  	int channels;
> +	int total_ch_enabled;
>  	u8 channel_line[8];
>  	u8 channel_step[8];
>  	int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	u16 *data = adc_dev->data;
>  
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +

extra newline added

>  	for (k = 0; k < fifo1count; k = k + i) {
>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>  			read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	return IRQ_HANDLED;
>  }
>  
> +static void tiadc_dma_rx_complete(void *param)
> +{
> +	struct iio_dev *indio_dev = param;
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	u8 *data;
> +	int i;
> +
> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> +	dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> +		iio_push_to_buffers(indio_dev, data);
> +		data += indio_dev->scan_bytes;
> +	}
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	struct dma_async_tx_descriptor *desc;
> +
> +	dma->valid_buf_seg = false;
> +	dma->fifo_thresh = FIFO1_THRESHOLD;
> +	/*
> +	 * Make the fifo thresh as the multiple of total number of
> +	 * channels enabled, so make sure that that cyclic DMA period

that that

> +	 * length is also a multiple of total number of channels
> +	 * enabled. This ensures that no invalid data is reported
> +	 * to the stack via iio_push_to_buffers().
> +	 */
> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
> +	/* Make sure that period length is multiple of fifo thresh level */
> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> +					      sizeof(u16));
> +
> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
> +	dmaengine_slave_config(dma->chan, &dma->conf);
> +
> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> +					 dma->buf_offset * 2,
> +					 dma->buf_offset, DMA_DEV_TO_MEM,
> +					 DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		return -EBUSY;
> +
> +	desc->callback = tiadc_dma_rx_complete;
> +	desc->callback_param = indio_dev;
> +
> +	dma->cookie = dmaengine_submit(desc);
> +
> +	dma_async_issue_pending(dma->chan);
> +
> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> +	return 0;
> +}
> +
>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	unsigned int enb = 0;
>  	u8 bit;
> +	u32 irq_enable;

why u32 and not unsigned int?

>  
>  	tiadc_step_config(indio_dev);
> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> +		adc_dev->total_ch_enabled++;
> +	}
>  	adc_dev->buffer_en_ch_steps = enb;
>  
> +	if (dma->chan)
> +		tiadc_start_dma(indio_dev);
> +
>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>  
>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
> -				| IRQENB_FIFO1OVRRUN);
> +
> +	irq_enable = IRQENB_FIFO1OVRRUN;
> +	if (!dma->chan)
> +		irq_enable |= IRQENB_FIFO1THRES;
> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>  
>  	return 0;
>  }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	int fifo1count, i, read;
>  
>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>  	adc_dev->buffer_en_ch_steps = 0;
> +	adc_dev->total_ch_enabled = 0;
> +	if (dma->chan) {
> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> +		dmaengine_terminate_async(dma->chan);
> +	}
>  
>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> +	return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> +			     struct tiadc_device *adc_dev)
> +{
> +	struct tiadc_dma	*dma = &adc_dev->dma;
> +	dma_cap_mask_t		mask;
> +
> +	dma->fn = the_no_dma_filter_fn;
> +
> +	/* Default slave configuration parameters */
> +	dma->conf.direction = DMA_DEV_TO_MEM;
> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_CYCLIC, mask);
> +
> +	/* Get a channel for RX */
> +	dma->chan = dma_request_slave_channel_compat(mask,
> +						     dma->fn, dma->param,
> +						     adc_dev->mfd_tscadc->dev,
> +						     "fifo1");
> +	if (!dma->chan)
> +		return -ENODEV;
> +
> +	/* RX buffer */
> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> +				      &dma->addr, GFP_KERNEL);
> +	if (!dma->buf)
> +		goto err;
> +
> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> +	return 0;
> +err:
> +	dma_release_channel(dma->chan);
> +
> +	return -ENOMEM;
> +}
> +
>  static int tiadc_parse_dt(struct platform_device *pdev,
>  			  struct tiadc_device *adc_dev)
>  {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> +	err = tiadc_request_dma(pdev, adc_dev);
> +	if (err && err != -ENODEV)
> +		goto err_dma;
> +
>  	return 0;
>  
> +err_dma:
> +	iio_device_unregister(indio_dev);
>  err_buffer_unregister:
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	u32 step_en;
>  
> +	if (dma->chan)
> +		dma_release_channel(dma->chan);
>  	iio_device_unregister(indio_dev);
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  	tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
>  #define REG_IRQENABLE		0x02C
>  #define REG_IRQCLR		0x030
>  #define REG_IRQWAKEUP		0x034
> +#define REG_DMAENABLE_SET	0x038
> +#define REG_DMAENABLE_CLEAR	0x038

same reg for set and clear?

>  #define REG_CTRL		0x040
>  #define REG_ADCFSM		0x044
>  #define REG_CLKDIV		0x04C
> @@ -36,6 +38,7 @@
>  #define REG_FIFO0THR		0xE8
>  #define REG_FIFO1CNT		0xF0
>  #define REG_FIFO1THR		0xF4
> +#define REG_DMA1REQ		0xF8
>  #define REG_FIFO0		0x100
>  #define REG_FIFO1		0x200
>  
> @@ -126,6 +129,10 @@
>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>  
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0		BIT(0)
> +#define DMA_FIFO1		BIT(1)
> +
>  /* Sequencer Status */
>  #define SEQ_STATUS BIT(5)
>  #define CHARGE_STEP		0x11
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Mugunthan V N]

On Wednesday 21 September 2016 11:14 PM, Peter Meerwald-Stadler wrote:
> 
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
> 
> nitpicking below
>  
>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>> ---
>>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>>  2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>>  #include <linux/iio/buffer.h>
>>  #include <linux/iio/kfifo_buf.h>
>>  
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE		SZ_2K
>> +
>> +struct tiadc_dma {
>> +	/* Filter function */
>> +	dma_filter_fn		fn;
>> +	/* Parameter to the filter function */
>> +	void			*param;
>> +	struct dma_slave_config	conf;
>> +	struct dma_chan		*chan;
>> +	dma_addr_t		addr;
>> +	dma_cookie_t		cookie;
>> +	u8			*buf;
>> +	bool			valid_buf_seg;
>> +	int			buf_offset;
>> +	u8			fifo_thresh;
>> +};
>> +
>>  struct tiadc_device {
>>  	struct ti_tscadc_dev *mfd_tscadc;
>> +	struct tiadc_dma dma;
>>  	struct mutex fifo1_lock; /* to protect fifo access */
>>  	int channels;
>> +	int total_ch_enabled;
>>  	u8 channel_line[8];
>>  	u8 channel_step[8];
>>  	int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	u16 *data = adc_dev->data;
>>  
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
> 
> extra newline added
> 
>>  	for (k = 0; k < fifo1count; k = k + i) {
>>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>>  			read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> +	struct iio_dev *indio_dev = param;
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	u8 *data;
>> +	int i;
>> +
>> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> +	dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> +		iio_push_to_buffers(indio_dev, data);
>> +		data += indio_dev->scan_bytes;
>> +	}
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	struct dma_async_tx_descriptor *desc;
>> +
>> +	dma->valid_buf_seg = false;
>> +	dma->fifo_thresh = FIFO1_THRESHOLD;
>> +	/*
>> +	 * Make the fifo thresh as the multiple of total number of
>> +	 * channels enabled, so make sure that that cyclic DMA period
> 
> that that
> 
>> +	 * length is also a multiple of total number of channels
>> +	 * enabled. This ensures that no invalid data is reported
>> +	 * to the stack via iio_push_to_buffers().
>> +	 */
>> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> +	/* Make sure that period length is multiple of fifo thresh level */
>> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> +					      sizeof(u16));
>> +
>> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> +	dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> +					 dma->buf_offset * 2,
>> +					 dma->buf_offset, DMA_DEV_TO_MEM,
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = tiadc_dma_rx_complete;
>> +	desc->callback_param = indio_dev;
>> +
>> +	dma->cookie = dmaengine_submit(desc);
>> +
>> +	dma_async_issue_pending(dma->chan);
>> +
>> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	unsigned int enb = 0;
>>  	u8 bit;
>> +	u32 irq_enable;
> 
> why u32 and not unsigned int?
> 
>>  
>>  	tiadc_step_config(indio_dev);
>> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> +		adc_dev->total_ch_enabled++;
>> +	}
>>  	adc_dev->buffer_en_ch_steps = enb;
>>  
>> +	if (dma->chan)
>> +		tiadc_start_dma(indio_dev);
>> +
>>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>  
>>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
>> -				| IRQENB_FIFO1OVRRUN);
>> +
>> +	irq_enable = IRQENB_FIFO1OVRRUN;
>> +	if (!dma->chan)
>> +		irq_enable |= IRQENB_FIFO1THRES;
>> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>>  
>>  	return 0;
>>  }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	int fifo1count, i, read;
>>  
>>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>>  	adc_dev->buffer_en_ch_steps = 0;
>> +	adc_dev->total_ch_enabled = 0;
>> +	if (dma->chan) {
>> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> +		dmaengine_terminate_async(dma->chan);
>> +	}
>>  
>>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> +	return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> +			     struct tiadc_device *adc_dev)
>> +{
>> +	struct tiadc_dma	*dma = &adc_dev->dma;
>> +	dma_cap_mask_t		mask;
>> +
>> +	dma->fn = the_no_dma_filter_fn;
>> +
>> +	/* Default slave configuration parameters */
>> +	dma->conf.direction = DMA_DEV_TO_MEM;
>> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> +	dma_cap_zero(mask);
>> +	dma_cap_set(DMA_CYCLIC, mask);
>> +
>> +	/* Get a channel for RX */
>> +	dma->chan = dma_request_slave_channel_compat(mask,
>> +						     dma->fn, dma->param,
>> +						     adc_dev->mfd_tscadc->dev,
>> +						     "fifo1");
>> +	if (!dma->chan)
>> +		return -ENODEV;
>> +
>> +	/* RX buffer */
>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> +				      &dma->addr, GFP_KERNEL);
>> +	if (!dma->buf)
>> +		goto err;
>> +
>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> +	return 0;
>> +err:
>> +	dma_release_channel(dma->chan);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>>  static int tiadc_parse_dt(struct platform_device *pdev,
>>  			  struct tiadc_device *adc_dev)
>>  {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> +	err = tiadc_request_dma(pdev, adc_dev);
>> +	if (err && err != -ENODEV)
>> +		goto err_dma;
>> +
>>  	return 0;
>>  
>> +err_dma:
>> +	iio_device_unregister(indio_dev);
>>  err_buffer_unregister:
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>>  {
>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	u32 step_en;
>>  
>> +	if (dma->chan)
>> +		dma_release_channel(dma->chan);
>>  	iio_device_unregister(indio_dev);
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  	tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>>  #define REG_IRQENABLE		0x02C
>>  #define REG_IRQCLR		0x030
>>  #define REG_IRQWAKEUP		0x034
>> +#define REG_DMAENABLE_SET	0x038
>> +#define REG_DMAENABLE_CLEAR	0x038
> 
> same reg for set and clear?

Oops, will fix this and all above.

Regards
Mugunthan V N

> 
>>  #define REG_CTRL		0x040
>>  #define REG_ADCFSM		0x044
>>  #define REG_CLKDIV		0x04C
>> @@ -36,6 +38,7 @@
>>  #define REG_FIFO0THR		0xE8
>>  #define REG_FIFO1CNT		0xF0
>>  #define REG_FIFO1THR		0xF4
>> +#define REG_DMA1REQ		0xF8
>>  #define REG_FIFO0		0x100
>>  #define REG_FIFO1		0x200
>>  
>> @@ -126,6 +129,10 @@
>>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>  
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0		BIT(0)
>> +#define DMA_FIFO1		BIT(1)
>> +
>>  /* Sequencer Status */
>>  #define SEQ_STATUS BIT(5)
>>  #define CHARGE_STEP		0x11
>>
> 

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Vignesh R]

On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>  2 files changed, 164 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/kfifo_buf.h>
>  
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE		SZ_2K
> +
> +struct tiadc_dma {
> +	/* Filter function */
> +	dma_filter_fn		fn;
> +	/* Parameter to the filter function */
> +	void			*param;

These will not be needed with newer APIs (see below)

> +	struct dma_slave_config	conf;
> +	struct dma_chan		*chan;
> +	dma_addr_t		addr;
> +	dma_cookie_t		cookie;
> +	u8			*buf;
> +	bool			valid_buf_seg;
> +	int			buf_offset;
> +	u8			fifo_thresh;
> +};
> +
>  struct tiadc_device {
>  	struct ti_tscadc_dev *mfd_tscadc;
> +	struct tiadc_dma dma;
>  	struct mutex fifo1_lock; /* to protect fifo access */
>  	int channels;
> +	int total_ch_enabled;
>  	u8 channel_line[8];
>  	u8 channel_step[8];
>  	int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	u16 *data = adc_dev->data;
>  
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
>  	for (k = 0; k < fifo1count; k = k + i) {
>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>  			read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	return IRQ_HANDLED;
>  }
>  
> +static void tiadc_dma_rx_complete(void *param)
> +{
> +	struct iio_dev *indio_dev = param;
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	u8 *data;
> +	int i;
> +
> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> +	dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> +		iio_push_to_buffers(indio_dev, data);
> +		data += indio_dev->scan_bytes;
> +	}
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	struct dma_async_tx_descriptor *desc;
> +
> +	dma->valid_buf_seg = false;
> +	dma->fifo_thresh = FIFO1_THRESHOLD;
> +	/*
> +	 * Make the fifo thresh as the multiple of total number of
> +	 * channels enabled, so make sure that that cyclic DMA period
> +	 * length is also a multiple of total number of channels
> +	 * enabled. This ensures that no invalid data is reported
> +	 * to the stack via iio_push_to_buffers().
> +	 */
> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;

Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?

> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
> +	/* Make sure that period length is multiple of fifo thresh level */
> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> +					      sizeof(u16));
> +

Can we use rounddown()?

> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
> +	dmaengine_slave_config(dma->chan, &dma->conf);
> +
> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> +					 dma->buf_offset * 2,
> +					 dma->buf_offset, DMA_DEV_TO_MEM,
> +					 DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		return -EBUSY;
> +
> +	desc->callback = tiadc_dma_rx_complete;
> +	desc->callback_param = indio_dev;
> +
> +	dma->cookie = dmaengine_submit(desc);
> +
> +	dma_async_issue_pending(dma->chan);
> +
> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> +	return 0;
> +}
> +
>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	unsigned int enb = 0;
>  	u8 bit;
> +	u32 irq_enable;
>  
>  	tiadc_step_config(indio_dev);
> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> +		adc_dev->total_ch_enabled++;
> +	}
>  	adc_dev->buffer_en_ch_steps = enb;
>  
> +	if (dma->chan)
> +		tiadc_start_dma(indio_dev);
> +
>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>  
>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
> -				| IRQENB_FIFO1OVRRUN);
> +
> +	irq_enable = IRQENB_FIFO1OVRRUN;
> +	if (!dma->chan)
> +		irq_enable |= IRQENB_FIFO1THRES;
> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>  
>  	return 0;
>  }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	int fifo1count, i, read;
>  
>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>  	adc_dev->buffer_en_ch_steps = 0;
> +	adc_dev->total_ch_enabled = 0;
> +	if (dma->chan) {
> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> +		dmaengine_terminate_async(dma->chan);
> +	}
>  
>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> +	return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> +			     struct tiadc_device *adc_dev)
> +{
> +	struct tiadc_dma	*dma = &adc_dev->dma;
> +	dma_cap_mask_t		mask;
> +
> +	dma->fn = the_no_dma_filter_fn;
> +
> +	/* Default slave configuration parameters */
> +	dma->conf.direction = DMA_DEV_TO_MEM;
> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_CYCLIC, mask);
> +
> +	/* Get a channel for RX */
> +	dma->chan = dma_request_slave_channel_compat(mask,
> +						     dma->fn, dma->param,
> +						     adc_dev->mfd_tscadc->dev,
> +						     "fifo1");

Please use dma_request_chan() API instead, this does not need
dma_filter_fn and probe defer can be handled.

> +	if (!dma->chan)
> +		return -ENODEV;
> +
> +	/* RX buffer */
> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> +				      &dma->addr, GFP_KERNEL);
> +	if (!dma->buf)
> +		goto err;
> +
> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");

Do we need _ratelimited? AFAICS, this print is called only once.

> +
> +	return 0;
> +err:
> +	dma_release_channel(dma->chan);
> +
> +	return -ENOMEM;
> +}
> +
>  static int tiadc_parse_dt(struct platform_device *pdev,
>  			  struct tiadc_device *adc_dev)
>  {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> +	err = tiadc_request_dma(pdev, adc_dev);
> +	if (err && err != -ENODEV)
> +		goto err_dma;
> +
>  	return 0;
>  
> +err_dma:
> +	iio_device_unregister(indio_dev);
>  err_buffer_unregister:
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	u32 step_en;
>  
> +	if (dma->chan)
> +		dma_release_channel(dma->chan);

dma_free_coherent() for dma->buf?

>  	iio_device_unregister(indio_dev);
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  	tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
>  #define REG_IRQENABLE		0x02C
>  #define REG_IRQCLR		0x030
>  #define REG_IRQWAKEUP		0x034
> +#define REG_DMAENABLE_SET	0x038
> +#define REG_DMAENABLE_CLEAR	0x038
>  #define REG_CTRL		0x040
>  #define REG_ADCFSM		0x044
>  #define REG_CLKDIV		0x04C
> @@ -36,6 +38,7 @@
>  #define REG_FIFO0THR		0xE8
>  #define REG_FIFO1CNT		0xF0
>  #define REG_FIFO1THR		0xF4
> +#define REG_DMA1REQ		0xF8
>  #define REG_FIFO0		0x100
>  #define REG_FIFO1		0x200
>  
> @@ -126,6 +129,10 @@
>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>  
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0		BIT(0)
> +#define DMA_FIFO1		BIT(1)
> +
>  /* Sequencer Status */
>  #define SEQ_STATUS BIT(5)
>  #define CHARGE_STEP		0x11
> 

-- 
Regards
Vignesh

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Mugunthan V N]

On Thursday 22 September 2016 11:48 AM, Vignesh R wrote:
> 
> 
> On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>>  2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>>  #include <linux/iio/buffer.h>
>>  #include <linux/iio/kfifo_buf.h>
>>  
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE		SZ_2K
>> +
>> +struct tiadc_dma {
>> +	/* Filter function */
>> +	dma_filter_fn		fn;
>> +	/* Parameter to the filter function */
>> +	void			*param;
> 
> These will not be needed with newer APIs (see below)
> 
>> +	struct dma_slave_config	conf;
>> +	struct dma_chan		*chan;
>> +	dma_addr_t		addr;
>> +	dma_cookie_t		cookie;
>> +	u8			*buf;
>> +	bool			valid_buf_seg;
>> +	int			buf_offset;
>> +	u8			fifo_thresh;
>> +};
>> +
>>  struct tiadc_device {
>>  	struct ti_tscadc_dev *mfd_tscadc;
>> +	struct tiadc_dma dma;
>>  	struct mutex fifo1_lock; /* to protect fifo access */
>>  	int channels;
>> +	int total_ch_enabled;
>>  	u8 channel_line[8];
>>  	u8 channel_step[8];
>>  	int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	u16 *data = adc_dev->data;
>>  
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>>  	for (k = 0; k < fifo1count; k = k + i) {
>>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>>  			read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> +	struct iio_dev *indio_dev = param;
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	u8 *data;
>> +	int i;
>> +
>> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> +	dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> +		iio_push_to_buffers(indio_dev, data);
>> +		data += indio_dev->scan_bytes;
>> +	}
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	struct dma_async_tx_descriptor *desc;
>> +
>> +	dma->valid_buf_seg = false;
>> +	dma->fifo_thresh = FIFO1_THRESHOLD;
>> +	/*
>> +	 * Make the fifo thresh as the multiple of total number of
>> +	 * channels enabled, so make sure that that cyclic DMA period
>> +	 * length is also a multiple of total number of channels
>> +	 * enabled. This ensures that no invalid data is reported
>> +	 * to the stack via iio_push_to_buffers().
>> +	 */
>> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> 
> Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
> 
>> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> +	/* Make sure that period length is multiple of fifo thresh level */
>> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> +					      sizeof(u16));
>> +
> 
> Can we use rounddown()?

Will change this in next version.

> 
>> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> +	dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> +					 dma->buf_offset * 2,
>> +					 dma->buf_offset, DMA_DEV_TO_MEM,
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = tiadc_dma_rx_complete;
>> +	desc->callback_param = indio_dev;
>> +
>> +	dma->cookie = dmaengine_submit(desc);
>> +
>> +	dma_async_issue_pending(dma->chan);
>> +
>> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	unsigned int enb = 0;
>>  	u8 bit;
>> +	u32 irq_enable;
>>  
>>  	tiadc_step_config(indio_dev);
>> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> +		adc_dev->total_ch_enabled++;
>> +	}
>>  	adc_dev->buffer_en_ch_steps = enb;
>>  
>> +	if (dma->chan)
>> +		tiadc_start_dma(indio_dev);
>> +
>>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>  
>>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
>> -				| IRQENB_FIFO1OVRRUN);
>> +
>> +	irq_enable = IRQENB_FIFO1OVRRUN;
>> +	if (!dma->chan)
>> +		irq_enable |= IRQENB_FIFO1THRES;
>> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>>  
>>  	return 0;
>>  }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	int fifo1count, i, read;
>>  
>>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>>  	adc_dev->buffer_en_ch_steps = 0;
>> +	adc_dev->total_ch_enabled = 0;
>> +	if (dma->chan) {
>> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> +		dmaengine_terminate_async(dma->chan);
>> +	}
>>  
>>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> +	return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> +			     struct tiadc_device *adc_dev)
>> +{
>> +	struct tiadc_dma	*dma = &adc_dev->dma;
>> +	dma_cap_mask_t		mask;
>> +
>> +	dma->fn = the_no_dma_filter_fn;
>> +
>> +	/* Default slave configuration parameters */
>> +	dma->conf.direction = DMA_DEV_TO_MEM;
>> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> +	dma_cap_zero(mask);
>> +	dma_cap_set(DMA_CYCLIC, mask);
>> +
>> +	/* Get a channel for RX */
>> +	dma->chan = dma_request_slave_channel_compat(mask,
>> +						     dma->fn, dma->param,
>> +						     adc_dev->mfd_tscadc->dev,
>> +						     "fifo1");
> 
> Please use dma_request_chan() API instead, this does not need
> dma_filter_fn and probe defer can be handled.

Okay

> 
>> +	if (!dma->chan)
>> +		return -ENODEV;
>> +
>> +	/* RX buffer */
>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> +				      &dma->addr, GFP_KERNEL);
>> +	if (!dma->buf)
>> +		goto err;
>> +
>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> 
> Do we need _ratelimited? AFAICS, this print is called only once.

True, will change to dev_dbg.

> 
>> +
>> +	return 0;
>> +err:
>> +	dma_release_channel(dma->chan);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>>  static int tiadc_parse_dt(struct platform_device *pdev,
>>  			  struct tiadc_device *adc_dev)
>>  {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> +	err = tiadc_request_dma(pdev, adc_dev);
>> +	if (err && err != -ENODEV)
>> +		goto err_dma;
>> +
>>  	return 0;
>>  
>> +err_dma:
>> +	iio_device_unregister(indio_dev);
>>  err_buffer_unregister:
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>>  {
>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	u32 step_en;
>>  
>> +	if (dma->chan)
>> +		dma_release_channel(dma->chan);
> 
> dma_free_coherent() for dma->buf?

Oops missed, will fix in next version.

Regards
Mugunthan V N

> 
>>  	iio_device_unregister(indio_dev);
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  	tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>>  #define REG_IRQENABLE		0x02C
>>  #define REG_IRQCLR		0x030
>>  #define REG_IRQWAKEUP		0x034
>> +#define REG_DMAENABLE_SET	0x038
>> +#define REG_DMAENABLE_CLEAR	0x038
>>  #define REG_CTRL		0x040
>>  #define REG_ADCFSM		0x044
>>  #define REG_CLKDIV		0x04C
>> @@ -36,6 +38,7 @@
>>  #define REG_FIFO0THR		0xE8
>>  #define REG_FIFO1CNT		0xF0
>>  #define REG_FIFO1THR		0xF4
>> +#define REG_DMA1REQ		0xF8
>>  #define REG_FIFO0		0x100
>>  #define REG_FIFO1		0x200
>>  
>> @@ -126,6 +129,10 @@
>>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>  
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0		BIT(0)
>> +#define DMA_FIFO1		BIT(1)
>> +
>>  /* Sequencer Status */
>>  #define SEQ_STATUS BIT(5)
>>  #define CHARGE_STEP		0x11
>>
> 

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Peter Ujfalusi]

On 09/22/16 13:45, Mugunthan V N wrote:
>>> +	if (!dma->chan)
>>> +		return -ENODEV;
>>> +
>>> +	/* RX buffer */
>>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>>> +				      &dma->addr, GFP_KERNEL);
>>> +	if (!dma->buf)
>>> +		goto err;
>>> +
>>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>>
>> Do we need _ratelimited? AFAICS, this print is called only once.
> 
> True, will change to dev_dbg.

It would be better to remove it. It gives no useful debuggin information apart
from the fact that the driver did not failed to probe.

-- 
Péter

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Peter Ujfalusi]

On 09/21/16 19:11, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>  2 files changed, 164 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/kfifo_buf.h>
>  
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE		SZ_2K
> +
> +struct tiadc_dma {
> +	/* Filter function */
> +	dma_filter_fn		fn;
> +	/* Parameter to the filter function */
> +	void			*param;
> +	struct dma_slave_config	conf;
> +	struct dma_chan		*chan;
> +	dma_addr_t		addr;
> +	dma_cookie_t		cookie;
> +	u8			*buf;
> +	bool			valid_buf_seg;
> +	int			buf_offset;
> +	u8			fifo_thresh;
> +};
> +
>  struct tiadc_device {
>  	struct ti_tscadc_dev *mfd_tscadc;
> +	struct tiadc_dma dma;
>  	struct mutex fifo1_lock; /* to protect fifo access */
>  	int channels;
> +	int total_ch_enabled;
>  	u8 channel_line[8];
>  	u8 channel_step[8];
>  	int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	u16 *data = adc_dev->data;
>  
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
>  	for (k = 0; k < fifo1count; k = k + i) {
>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>  			read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>  	return IRQ_HANDLED;
>  }
>  
> +static void tiadc_dma_rx_complete(void *param)
> +{
> +	struct iio_dev *indio_dev = param;
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	u8 *data;
> +	int i;
> +
> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;

bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
segment is valid when valid_buf_seg is true?

I know what this is doing, but it is logically not correct.

Instead you could have:

int current_period; /* The period the DMA is working on */

When you start the DMA:
dma->current_period = 0; /* We start to fill period 0 */

In here:

u8 *data = dma->buf + (dma->current_period * dma_period_size);

> +	dma->valid_buf_seg = !dma->valid_buf_seg;

/* Currently we have only two periods, so we can just */
dma->current_period = !dma->current_period;
/* or */
dma->current_period = 1 - dma->current_period; /* swap the buffer ID */

I think this would make it better to follow and if later you figure that you
want three or more periods instead of the two, it is going to be easier to add
that.

> +
> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> +		iio_push_to_buffers(indio_dev, data);
> +		data += indio_dev->scan_bytes;
> +	}
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
> +	struct dma_async_tx_descriptor *desc;
> +
> +	dma->valid_buf_seg = false;
> +	dma->fifo_thresh = FIFO1_THRESHOLD;

FIFO1_THRESHOLD is defined as 19 in the global header file, so
if you have 2 channels enabled the trigger comes when the FIFO have 10
samples/ch, in case of all 8 channels enabled you will round it down the
trigger comes when you have 2 samples/ch.

Would not be better to try to use high threshold for the FIFO to reduce the
number of DMA requests needed runtime?

It is OK as it is at the moment, but I think it would make sense to try to
maximize the FIFO usage.

> +	/*
> +	 * Make the fifo thresh as the multiple of total number of
> +	 * channels enabled, so make sure that that cyclic DMA period
> +	 * length is also a multiple of total number of channels
> +	 * enabled. This ensures that no invalid data is reported
> +	 * to the stack via iio_push_to_buffers().
> +	 */
> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
> +	/* Make sure that period length is multiple of fifo thresh level */
> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> +					      sizeof(u16));
> +
> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
> +	dmaengine_slave_config(dma->chan, &dma->conf);
> +
> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> +					 dma->buf_offset * 2,
> +					 dma->buf_offset, DMA_DEV_TO_MEM,

I find the buf_offset name and it's use really confusing. It should be called
period_size, really.

> +					 DMA_PREP_INTERRUPT);
> +	if (!desc)
> +		return -EBUSY;
> +
> +	desc->callback = tiadc_dma_rx_complete;
> +	desc->callback_param = indio_dev;
> +
> +	dma->cookie = dmaengine_submit(desc);
> +
> +	dma_async_issue_pending(dma->chan);
> +
> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> +	return 0;
> +}
> +
>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	unsigned int enb = 0;
>  	u8 bit;
> +	u32 irq_enable;
>  
>  	tiadc_step_config(indio_dev);
> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> +		adc_dev->total_ch_enabled++;
> +	}
>  	adc_dev->buffer_en_ch_steps = enb;
>  
> +	if (dma->chan)
> +		tiadc_start_dma(indio_dev);
> +
>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>  
>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
> -				| IRQENB_FIFO1OVRRUN);
> +
> +	irq_enable = IRQENB_FIFO1OVRRUN;
> +	if (!dma->chan)
> +		irq_enable |= IRQENB_FIFO1THRES;
> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>  
>  	return 0;
>  }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>  {
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	int fifo1count, i, read;
>  
>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>  	adc_dev->buffer_en_ch_steps = 0;
> +	adc_dev->total_ch_enabled = 0;
> +	if (dma->chan) {
> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> +		dmaengine_terminate_async(dma->chan);
> +	}
>  
>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> +	return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> +			     struct tiadc_device *adc_dev)
> +{
> +	struct tiadc_dma	*dma = &adc_dev->dma;
> +	dma_cap_mask_t		mask;
> +
> +	dma->fn = the_no_dma_filter_fn;
> +
> +	/* Default slave configuration parameters */
> +	dma->conf.direction = DMA_DEV_TO_MEM;
> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_CYCLIC, mask);
> +
> +	/* Get a channel for RX */
> +	dma->chan = dma_request_slave_channel_compat(mask,
> +						     dma->fn, dma->param,
> +						     adc_dev->mfd_tscadc->dev,
> +						     "fifo1");

dma_request_chan()

> +	if (!dma->chan)
> +		return -ENODEV;
> +
> +	/* RX buffer */
> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> +				      &dma->addr, GFP_KERNEL);
> +	if (!dma->buf)
> +		goto err;
> +
> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> +	return 0;
> +err:
> +	dma_release_channel(dma->chan);
> +
> +	return -ENOMEM;
> +}
> +
>  static int tiadc_parse_dt(struct platform_device *pdev,
>  			  struct tiadc_device *adc_dev)
>  {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> +	err = tiadc_request_dma(pdev, adc_dev);
> +	if (err && err != -ENODEV)
> +		goto err_dma;
> +
>  	return 0;
>  
> +err_dma:
> +	iio_device_unregister(indio_dev);
>  err_buffer_unregister:
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
> +	struct tiadc_dma *dma = &adc_dev->dma;
>  	u32 step_en;
>  
> +	if (dma->chan)
> +		dma_release_channel(dma->chan);
>  	iio_device_unregister(indio_dev);
>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>  	tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
>  #define REG_IRQENABLE		0x02C
>  #define REG_IRQCLR		0x030
>  #define REG_IRQWAKEUP		0x034
> +#define REG_DMAENABLE_SET	0x038
> +#define REG_DMAENABLE_CLEAR	0x038
>  #define REG_CTRL		0x040
>  #define REG_ADCFSM		0x044
>  #define REG_CLKDIV		0x04C
> @@ -36,6 +38,7 @@
>  #define REG_FIFO0THR		0xE8
>  #define REG_FIFO1CNT		0xF0
>  #define REG_FIFO1THR		0xF4
> +#define REG_DMA1REQ		0xF8
>  #define REG_FIFO0		0x100
>  #define REG_FIFO1		0x200
>  
> @@ -126,6 +129,10 @@
>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>  
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0		BIT(0)
> +#define DMA_FIFO1		BIT(1)
> +
>  /* Sequencer Status */
>  #define SEQ_STATUS BIT(5)
>  #define CHARGE_STEP		0x11
> 


-- 
Péter

Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support

[Mugunthan V N]

On Thursday 22 September 2016 12:50 PM, Peter Ujfalusi wrote:
> On 09/21/16 19:11, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>> ---
>>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>>  2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>>  #include <linux/iio/buffer.h>
>>  #include <linux/iio/kfifo_buf.h>
>>  
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE		SZ_2K
>> +
>> +struct tiadc_dma {
>> +	/* Filter function */
>> +	dma_filter_fn		fn;
>> +	/* Parameter to the filter function */
>> +	void			*param;
>> +	struct dma_slave_config	conf;
>> +	struct dma_chan		*chan;
>> +	dma_addr_t		addr;
>> +	dma_cookie_t		cookie;
>> +	u8			*buf;
>> +	bool			valid_buf_seg;
>> +	int			buf_offset;
>> +	u8			fifo_thresh;
>> +};
>> +
>>  struct tiadc_device {
>>  	struct ti_tscadc_dev *mfd_tscadc;
>> +	struct tiadc_dma dma;
>>  	struct mutex fifo1_lock; /* to protect fifo access */
>>  	int channels;
>> +	int total_ch_enabled;
>>  	u8 channel_line[8];
>>  	u8 channel_step[8];
>>  	int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	u16 *data = adc_dev->data;
>>  
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>>  	for (k = 0; k < fifo1count; k = k + i) {
>>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>>  			read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> +	struct iio_dev *indio_dev = param;
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	u8 *data;
>> +	int i;
>> +
>> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> 
> bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
> segment is valid when valid_buf_seg is true?
> 
> I know what this is doing, but it is logically not correct.
> 
> Instead you could have:
> 
> int current_period; /* The period the DMA is working on */
> 
> When you start the DMA:
> dma->current_period = 0; /* We start to fill period 0 */
> 
> In here:
> 
> u8 *data = dma->buf + (dma->current_period * dma_period_size);
> 
>> +	dma->valid_buf_seg = !dma->valid_buf_seg;
> 
> /* Currently we have only two periods, so we can just */
> dma->current_period = !dma->current_period;
> /* or */
> dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
> 
> I think this would make it better to follow and if later you figure that you
> want three or more periods instead of the two, it is going to be easier to add
> that.

Okay, will change this in next version patch.

> 
>> +
>> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> +		iio_push_to_buffers(indio_dev, data);
>> +		data += indio_dev->scan_bytes;
>> +	}
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	struct dma_async_tx_descriptor *desc;
>> +
>> +	dma->valid_buf_seg = false;
>> +	dma->fifo_thresh = FIFO1_THRESHOLD;
> 
> FIFO1_THRESHOLD is defined as 19 in the global header file, so
> if you have 2 channels enabled the trigger comes when the FIFO have 10
> samples/ch, in case of all 8 channels enabled you will round it down the
> trigger comes when you have 2 samples/ch.
> 
> Would not be better to try to use high threshold for the FIFO to reduce the
> number of DMA requests needed runtime?
> 
> It is OK as it is at the moment, but I think it would make sense to try to
> maximize the FIFO usage.

I had already tried with higher values, It works fine. Just don't want
to change the non-DMA implementation as I am not aware of any
limitations why at the first place they have used fifo threshold as 19.

> 
>> +	/*
>> +	 * Make the fifo thresh as the multiple of total number of
>> +	 * channels enabled, so make sure that that cyclic DMA period
>> +	 * length is also a multiple of total number of channels
>> +	 * enabled. This ensures that no invalid data is reported
>> +	 * to the stack via iio_push_to_buffers().
>> +	 */
>> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> +	/* Make sure that period length is multiple of fifo thresh level */
>> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> +					      sizeof(u16));
>> +
>> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> +	dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> +					 dma->buf_offset * 2,
>> +					 dma->buf_offset, DMA_DEV_TO_MEM,
> 
> I find the buf_offset name and it's use really confusing. It should be called
> period_size, really.

Yes, will fix this.

> 
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = tiadc_dma_rx_complete;
>> +	desc->callback_param = indio_dev;
>> +
>> +	dma->cookie = dmaengine_submit(desc);
>> +
>> +	dma_async_issue_pending(dma->chan);
>> +
>> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	unsigned int enb = 0;
>>  	u8 bit;
>> +	u32 irq_enable;
>>  
>>  	tiadc_step_config(indio_dev);
>> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> +		adc_dev->total_ch_enabled++;
>> +	}
>>  	adc_dev->buffer_en_ch_steps = enb;
>>  
>> +	if (dma->chan)
>> +		tiadc_start_dma(indio_dev);
>> +
>>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>  
>>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
>> -				| IRQENB_FIFO1OVRRUN);
>> +
>> +	irq_enable = IRQENB_FIFO1OVRRUN;
>> +	if (!dma->chan)
>> +		irq_enable |= IRQENB_FIFO1THRES;
>> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>>  
>>  	return 0;
>>  }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	int fifo1count, i, read;
>>  
>>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>>  	adc_dev->buffer_en_ch_steps = 0;
>> +	adc_dev->total_ch_enabled = 0;
>> +	if (dma->chan) {
>> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> +		dmaengine_terminate_async(dma->chan);
>> +	}
>>  
>>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> +	return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> +			     struct tiadc_device *adc_dev)
>> +{
>> +	struct tiadc_dma	*dma = &adc_dev->dma;
>> +	dma_cap_mask_t		mask;
>> +
>> +	dma->fn = the_no_dma_filter_fn;
>> +
>> +	/* Default slave configuration parameters */
>> +	dma->conf.direction = DMA_DEV_TO_MEM;
>> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> +	dma_cap_zero(mask);
>> +	dma_cap_set(DMA_CYCLIC, mask);
>> +
>> +	/* Get a channel for RX */
>> +	dma->chan = dma_request_slave_channel_compat(mask,
>> +						     dma->fn, dma->param,
>> +						     adc_dev->mfd_tscadc->dev,
>> +						     "fifo1");
> 
> dma_request_chan()

Will fix this.

> 
>> +	if (!dma->chan)
>> +		return -ENODEV;
>> +
>> +	/* RX buffer */
>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> +				      &dma->addr, GFP_KERNEL);
>> +	if (!dma->buf)
>> +		goto err;
>> +
>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> +	return 0;
>> +err:
>> +	dma_release_channel(dma->chan);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>>  static int tiadc_parse_dt(struct platform_device *pdev,
>>  			  struct tiadc_device *adc_dev)
>>  {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> +	err = tiadc_request_dma(pdev, adc_dev);
>> +	if (err && err != -ENODEV)
>> +		goto err_dma;
>> +
>>  	return 0;
>>  
>> +err_dma:
>> +	iio_device_unregister(indio_dev);
>>  err_buffer_unregister:
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>>  {
>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	u32 step_en;
>>  
>> +	if (dma->chan)
>> +		dma_release_channel(dma->chan);
>>  	iio_device_unregister(indio_dev);
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  	tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>>  #define REG_IRQENABLE		0x02C
>>  #define REG_IRQCLR		0x030
>>  #define REG_IRQWAKEUP		0x034
>> +#define REG_DMAENABLE_SET	0x038
>> +#define REG_DMAENABLE_CLEAR	0x038
>>  #define REG_CTRL		0x040
>>  #define REG_ADCFSM		0x044
>>  #define REG_CLKDIV		0x04C
>> @@ -36,6 +38,7 @@
>>  #define REG_FIFO0THR		0xE8
>>  #define REG_FIFO1CNT		0xF0
>>  #define REG_FIFO1THR		0xF4
>> +#define REG_DMA1REQ		0xF8
>>  #define REG_FIFO0		0x100
>>  #define REG_FIFO1		0x200
>>  
>> @@ -126,6 +129,10 @@
>>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>  
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0		BIT(0)
>> +#define DMA_FIFO1		BIT(1)
>> +
>>  /* Sequencer Status */
>>  #define SEQ_STATUS BIT(5)
>>  #define CHARGE_STEP		0x11
>>
> 
> 

--
Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: dts: am33xx: add DMA properties for tscadc

[Mugunthan V N]

Add DMA properties for tscadc

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 98748c6..6d607b8 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -917,6 +917,8 @@
 			interrupts = <16>;
 			ti,hwmods = "adc_tsc";
 			status = "disabled";
+			dmas = <&edma 53 0>, <&edma 57 0>;
+			dma-names = "fifo0", "fifo1";
 
 			tsc {
 				compatible = "ti,am3359-tsc";
-- 
2.10.0.129.g35f6318

[PATCH 4/4] ARM: dts: am4372: add DMA properties for tscadc

[Mugunthan V N]

Add DMA properties for tscadc

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 arch/arm/boot/dts/am4372.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 0fadae5..6094d17 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -867,6 +867,8 @@
 			clocks = <&adc_tsc_fck>;
 			clock-names = "fck";
 			status = "disabled";
+			dmas = <&edma 53 0>, <&edma 57 0>;
+			dma-names = "fifo0", "fifo1";
 
 			tsc {
 				compatible = "ti,am3359-tsc";
-- 
2.10.0.129.g35f6318