Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 25/09/16 20:57, Quentin Schulz wrote:
> On 25/09/2016 11:10, Jonathan Cameron wrote:
>> On 24/09/16 18:40, Quentin Schulz wrote:
>>> Hi Jonathan,
>>>
>>> Sorry for the (long) delay, I did not have time to work on it. I'll
>>> mainly work in my free time now.
>>>
>>> Keep in mind this patch was proposed based on the v2 of the ADC patches.
>>> Since then, substantial changes have been made and I'm working on
>>> rebasing this series of patches on the v6, so comments here might
>>> include references to code parts added later in the ADC patch series.
>>>
>>> On 24/07/2016 13:03, Jonathan Cameron wrote:
On 20/07/16 09:29, Quentin Schulz wrote:
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and
> the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
That may be the bizarest hardware restriction I've heard of in a while! :)
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
Hmm. Might be possible to distinguish which consumer caused the start.
Thus, if the touchscreen is there we would know purely based on the
driver being the requester that we need to be in touchscreen mode.
>>>
>>> As of yet, can't see in which way I can retrieve the consumer in
>>> provider code. Maybe I'm missing something, I don't know?
>> I don't think we have a current way of doing this... Might be possible
>> to add one, but it would be a rather odd bit of reverse looking up.
> [...]
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
I was suprised to see this as normally it is cleaner to structure
the whole code to go in one direction through the structures (which is
why we don't provide a generic iio_device_from_priv bit of pointer magic).
Anyhow, don't htink you are actually using it ;)
>>>
>>> I'm using to push to buffers from the irq handler since I pass the local
>>> structure (sunxi_gpadc_dev) to the irq handler when registering it. But
>>> I guess I can pass the iio_dev instead and remove this from the local
>>> structure.
>> I'd prefer passing the iio_dev and keep all lookups in one direction.
>
> ACK.
>
>>>
>>> [...]
> static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> int *val)
> {
> struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
> + bool buffered = info->buffered;
Not worth the local version...
> int ret = 0;
> + unsigned int reg;
>
> mutex_lock(_dev->mlock);
>
> reinit_completion(>completion);
> +
> + reg = SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) | SUNXI_GPADC_TP_FIFO_FLUSH;
> + regmap_update_bits(info->regmap, SUNXI_GPADC_TP_INT_FIFOC, reg, reg);
I'd put it in directly rahter than having a reg local variable. To mind
mind that would be slightly easier to understand.
> +
> if (info->flags & SUNXI_GPADC_ARCH_SUN6I)
> regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>SUNXI_GPADC_SUN6I_TP_MODE_EN |
> @@ -153,9 +185,9 @@ static int sunxi_gpadc_adc_read(struct iio_dev
> *indio_dev, int channel,
>SUNXI_GPADC_TP_MODE_EN |
>SUNXI_GPADC_TP_ADC_SELECT |
>SUNXI_GPADC_ADC_CHAN_SELECT(channel));
> - regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
> - SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) |
> - SUNXI_GPADC_TP_FIFO_FLUSH);
Whole load of infrastructure in place to lock buffered mode out and
revent transitions when we can't have them.
iio_claim_direct_mode etc. I think you can just use that here?
If you need to do extra checks on it being
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 25/09/16 20:57, Quentin Schulz wrote:
> On 25/09/2016 11:10, Jonathan Cameron wrote:
>> On 24/09/16 18:40, Quentin Schulz wrote:
>>> Hi Jonathan,
>>>
>>> Sorry for the (long) delay, I did not have time to work on it. I'll
>>> mainly work in my free time now.
>>>
>>> Keep in mind this patch was proposed based on the v2 of the ADC patches.
>>> Since then, substantial changes have been made and I'm working on
>>> rebasing this series of patches on the v6, so comments here might
>>> include references to code parts added later in the ADC patch series.
>>>
>>> On 24/07/2016 13:03, Jonathan Cameron wrote:
On 20/07/16 09:29, Quentin Schulz wrote:
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and
> the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
That may be the bizarest hardware restriction I've heard of in a while! :)
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
Hmm. Might be possible to distinguish which consumer caused the start.
Thus, if the touchscreen is there we would know purely based on the
driver being the requester that we need to be in touchscreen mode.
>>>
>>> As of yet, can't see in which way I can retrieve the consumer in
>>> provider code. Maybe I'm missing something, I don't know?
>> I don't think we have a current way of doing this... Might be possible
>> to add one, but it would be a rather odd bit of reverse looking up.
> [...]
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
I was suprised to see this as normally it is cleaner to structure
the whole code to go in one direction through the structures (which is
why we don't provide a generic iio_device_from_priv bit of pointer magic).
Anyhow, don't htink you are actually using it ;)
>>>
>>> I'm using to push to buffers from the irq handler since I pass the local
>>> structure (sunxi_gpadc_dev) to the irq handler when registering it. But
>>> I guess I can pass the iio_dev instead and remove this from the local
>>> structure.
>> I'd prefer passing the iio_dev and keep all lookups in one direction.
>
> ACK.
>
>>>
>>> [...]
> static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
> int *val)
> {
> struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
> + bool buffered = info->buffered;
Not worth the local version...
> int ret = 0;
> + unsigned int reg;
>
> mutex_lock(_dev->mlock);
>
> reinit_completion(>completion);
> +
> + reg = SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) | SUNXI_GPADC_TP_FIFO_FLUSH;
> + regmap_update_bits(info->regmap, SUNXI_GPADC_TP_INT_FIFOC, reg, reg);
I'd put it in directly rahter than having a reg local variable. To mind
mind that would be slightly easier to understand.
> +
> if (info->flags & SUNXI_GPADC_ARCH_SUN6I)
> regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
>SUNXI_GPADC_SUN6I_TP_MODE_EN |
> @@ -153,9 +185,9 @@ static int sunxi_gpadc_adc_read(struct iio_dev
> *indio_dev, int channel,
>SUNXI_GPADC_TP_MODE_EN |
>SUNXI_GPADC_TP_ADC_SELECT |
>SUNXI_GPADC_ADC_CHAN_SELECT(channel));
> - regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
> - SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) |
> - SUNXI_GPADC_TP_FIFO_FLUSH);
Whole load of infrastructure in place to lock buffered mode out and
revent transitions when we can't have them.
iio_claim_direct_mode etc. I think you can just use that here?
If you need to do extra checks on it being
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 25/09/2016 11:10, Jonathan Cameron wrote:
> On 24/09/16 18:40, Quentin Schulz wrote:
>> Hi Jonathan,
>>
>> Sorry for the (long) delay, I did not have time to work on it. I'll
>> mainly work in my free time now.
>>
>> Keep in mind this patch was proposed based on the v2 of the ADC patches.
>> Since then, substantial changes have been made and I'm working on
>> rebasing this series of patches on the v6, so comments here might
>> include references to code parts added later in the ADC patch series.
>>
>> On 24/07/2016 13:03, Jonathan Cameron wrote:
>>> On 20/07/16 09:29, Quentin Schulz wrote:
This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
It also prepares the code which will be used by the touchscreen driver
named sunxi-gpadc-ts.
The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
conversion's data. The GPADC uses the same ADC channels for the ADC and the
touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
consumer which will be in charge of reading data from these channels for
the input framework.
The temperature can only be read when in touchscreen mode. This means if
the buffers are being used for the ADC, the temperature sensor cannot be
read.
>>> That may be the bizarest hardware restriction I've heard of in a while! :)
When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
and fill a buffer before sending it to the consumers which registered in
IIO for the ADC channels.
When a consumer starts buffering ADC channels,
sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
irq and select the mode in which the GPADC should run (ADC or touchscreen)
depending on a property of the DT ("allwinner,ts-attached").
When the consumer stops buffering, it disables the same irq.
>>> Hmm. Might be possible to distinguish which consumer caused the start.
>>> Thus, if the touchscreen is there we would know purely based on the
>>> driver being the requester that we need to be in touchscreen mode.
>>>
>>
>> As of yet, can't see in which way I can retrieve the consumer in
>> provider code. Maybe I'm missing something, I don't know?
> I don't think we have a current way of doing this... Might be possible
> to add one, but it would be a rather odd bit of reverse looking up.
[...]
@@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
unsigned intfifo_data_irq;
unsigned inttemp_data_irq;
unsigned intflags;
+ struct iio_dev *indio_dev;
>>> I was suprised to see this as normally it is cleaner to structure
>>> the whole code to go in one direction through the structures (which is
>>> why we don't provide a generic iio_device_from_priv bit of pointer magic).
>>>
>>> Anyhow, don't htink you are actually using it ;)
>>>
>>
>> I'm using to push to buffers from the irq handler since I pass the local
>> structure (sunxi_gpadc_dev) to the irq handler when registering it. But
>> I guess I can pass the iio_dev instead and remove this from the local
>> structure.
> I'd prefer passing the iio_dev and keep all lookups in one direction.
ACK.
>>
>> [...]
static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
int *val)
{
struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
+ bool buffered = info->buffered;
>>> Not worth the local version...
int ret = 0;
+ unsigned int reg;
mutex_lock(_dev->mlock);
reinit_completion(>completion);
+
+ reg = SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) | SUNXI_GPADC_TP_FIFO_FLUSH;
+ regmap_update_bits(info->regmap, SUNXI_GPADC_TP_INT_FIFOC, reg, reg);
>>> I'd put it in directly rahter than having a reg local variable. To mind
>>> mind that would be slightly easier to understand.
+
if (info->flags & SUNXI_GPADC_ARCH_SUN6I)
regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
SUNXI_GPADC_SUN6I_TP_MODE_EN |
@@ -153,9 +185,9 @@ static int sunxi_gpadc_adc_read(struct iio_dev
*indio_dev, int channel,
SUNXI_GPADC_TP_MODE_EN |
SUNXI_GPADC_TP_ADC_SELECT |
SUNXI_GPADC_ADC_CHAN_SELECT(channel));
- regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
- SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) |
- SUNXI_GPADC_TP_FIFO_FLUSH);
>>> Whole load of infrastructure in place to lock buffered mode out and
>>> revent transitions when we can't have them.
>>>
>>> iio_claim_direct_mode etc. I think you can just use that here?
>>> If you need to do extra checks on it being enabled that should be
>>> fine too.
>>>
>>
>> Yes, way better with iio_device_claim_direct_mode and iio_buffer_enabled!
>>
>>> As a
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 25/09/2016 11:10, Jonathan Cameron wrote:
> On 24/09/16 18:40, Quentin Schulz wrote:
>> Hi Jonathan,
>>
>> Sorry for the (long) delay, I did not have time to work on it. I'll
>> mainly work in my free time now.
>>
>> Keep in mind this patch was proposed based on the v2 of the ADC patches.
>> Since then, substantial changes have been made and I'm working on
>> rebasing this series of patches on the v6, so comments here might
>> include references to code parts added later in the ADC patch series.
>>
>> On 24/07/2016 13:03, Jonathan Cameron wrote:
>>> On 20/07/16 09:29, Quentin Schulz wrote:
This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
It also prepares the code which will be used by the touchscreen driver
named sunxi-gpadc-ts.
The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
conversion's data. The GPADC uses the same ADC channels for the ADC and the
touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
consumer which will be in charge of reading data from these channels for
the input framework.
The temperature can only be read when in touchscreen mode. This means if
the buffers are being used for the ADC, the temperature sensor cannot be
read.
>>> That may be the bizarest hardware restriction I've heard of in a while! :)
When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
and fill a buffer before sending it to the consumers which registered in
IIO for the ADC channels.
When a consumer starts buffering ADC channels,
sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
irq and select the mode in which the GPADC should run (ADC or touchscreen)
depending on a property of the DT ("allwinner,ts-attached").
When the consumer stops buffering, it disables the same irq.
>>> Hmm. Might be possible to distinguish which consumer caused the start.
>>> Thus, if the touchscreen is there we would know purely based on the
>>> driver being the requester that we need to be in touchscreen mode.
>>>
>>
>> As of yet, can't see in which way I can retrieve the consumer in
>> provider code. Maybe I'm missing something, I don't know?
> I don't think we have a current way of doing this... Might be possible
> to add one, but it would be a rather odd bit of reverse looking up.
[...]
@@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
unsigned intfifo_data_irq;
unsigned inttemp_data_irq;
unsigned intflags;
+ struct iio_dev *indio_dev;
>>> I was suprised to see this as normally it is cleaner to structure
>>> the whole code to go in one direction through the structures (which is
>>> why we don't provide a generic iio_device_from_priv bit of pointer magic).
>>>
>>> Anyhow, don't htink you are actually using it ;)
>>>
>>
>> I'm using to push to buffers from the irq handler since I pass the local
>> structure (sunxi_gpadc_dev) to the irq handler when registering it. But
>> I guess I can pass the iio_dev instead and remove this from the local
>> structure.
> I'd prefer passing the iio_dev and keep all lookups in one direction.
ACK.
>>
>> [...]
static int sunxi_gpadc_adc_read(struct iio_dev *indio_dev, int channel,
int *val)
{
struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
+ bool buffered = info->buffered;
>>> Not worth the local version...
int ret = 0;
+ unsigned int reg;
mutex_lock(_dev->mlock);
reinit_completion(>completion);
+
+ reg = SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) | SUNXI_GPADC_TP_FIFO_FLUSH;
+ regmap_update_bits(info->regmap, SUNXI_GPADC_TP_INT_FIFOC, reg, reg);
>>> I'd put it in directly rahter than having a reg local variable. To mind
>>> mind that would be slightly easier to understand.
+
if (info->flags & SUNXI_GPADC_ARCH_SUN6I)
regmap_write(info->regmap, SUNXI_GPADC_TP_CTRL1,
SUNXI_GPADC_SUN6I_TP_MODE_EN |
@@ -153,9 +185,9 @@ static int sunxi_gpadc_adc_read(struct iio_dev
*indio_dev, int channel,
SUNXI_GPADC_TP_MODE_EN |
SUNXI_GPADC_TP_ADC_SELECT |
SUNXI_GPADC_ADC_CHAN_SELECT(channel));
- regmap_write(info->regmap, SUNXI_GPADC_TP_INT_FIFOC,
- SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(1) |
- SUNXI_GPADC_TP_FIFO_FLUSH);
>>> Whole load of infrastructure in place to lock buffered mode out and
>>> revent transitions when we can't have them.
>>>
>>> iio_claim_direct_mode etc. I think you can just use that here?
>>> If you need to do extra checks on it being enabled that should be
>>> fine too.
>>>
>>
>> Yes, way better with iio_device_claim_direct_mode and iio_buffer_enabled!
>>
>>> As a
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 24/09/16 18:40, Quentin Schulz wrote:
> Hi Jonathan,
>
> Sorry for the (long) delay, I did not have time to work on it. I'll
> mainly work in my free time now.
>
> Keep in mind this patch was proposed based on the v2 of the ADC patches.
> Since then, substantial changes have been made and I'm working on
> rebasing this series of patches on the v6, so comments here might
> include references to code parts added later in the ADC patch series.
>
> On 24/07/2016 13:03, Jonathan Cameron wrote:
>> On 20/07/16 09:29, Quentin Schulz wrote:
>>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>>> It also prepares the code which will be used by the touchscreen driver
>>> named sunxi-gpadc-ts.
>>>
>>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>>> consumer which will be in charge of reading data from these channels for
>>> the input framework.
>>>
>>> The temperature can only be read when in touchscreen mode. This means if
>>> the buffers are being used for the ADC, the temperature sensor cannot be
>>> read.
>> That may be the bizarest hardware restriction I've heard of in a while! :)
>>>
>>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>>> and fill a buffer before sending it to the consumers which registered in
>>> IIO for the ADC channels.
>>>
>>> When a consumer starts buffering ADC channels,
>>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>>> depending on a property of the DT ("allwinner,ts-attached").
>>> When the consumer stops buffering, it disables the same irq.
>> Hmm. Might be possible to distinguish which consumer caused the start.
>> Thus, if the touchscreen is there we would know purely based on the
>> driver being the requester that we need to be in touchscreen mode.
>>
>
> As of yet, can't see in which way I can retrieve the consumer in
> provider code. Maybe I'm missing something, I don't know?
I don't think we have a current way of doing this... Might be possible
to add one, but it would be a rather odd bit of reverse looking up.
>
>>>
>>> Signed-off-by: Quentin Schulz
>> You are moving fast on this - I'd have been tempted to do a mega
>> series with the updated version of the basic support and this on top
>> rather than a new unconnected series.
>>
>> (I'd forgotten that was still under review so got confused when I
>> went to look something up in the files you are modifying!).
>>> ---
>>> drivers/iio/adc/Kconfig | 1 +
>>> drivers/iio/adc/sunxi-gpadc-iio.c | 153
>>> ++
>>> 2 files changed, 138 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 184856f..15e3b08 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -342,6 +342,7 @@ config SUNXI_ADC
>>> tristate "ADC driver for sunxi platforms"
>>> depends on IIO
>>> depends on MFD_SUNXI_ADC
>>> + depends on IIO_BUFFER_CB
>>> help
>>> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
>>> ADC. This ADC provides 4 channels which can be used as an ADC or as a
>>> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
>>> b/drivers/iio/adc/sunxi-gpadc-iio.c
>>> index 87cc913..2e44ca7 100644
>>> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
>>> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
>>> @@ -16,8 +16,9 @@
>>> #include
>>> #include
>>>
>>> -#include
>>> +#include
>>> #include
>>> +#include
>> Can't say I'm a particular fan of reordering headers to be in alphabetical
>> order, but I suppose it doesn't really matter if you want to do it.
>> (to my mind there is a tree structure implicit in these headers with iio.h
>> at the top for generic support, then the various sub elements below).
>>
>>> #include
>>> #include
>>>
>>> @@ -71,6 +72,7 @@
>>> #define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
>>> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
>>> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
>>> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING
>>> interrupts */
>> Sounds like you learned that one the hard way ;)
>>> #define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
>>> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
>>> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
>>> @@ -79,6 +81,7 @@
>>> #define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
>>> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
>>> #define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
>>> +#define SUNXI_GPADC_RXA_CNTGENMASK(12, 8)
>>> #define SUNXI_GPADC_TP_IDLE_FLGBIT(2)
>>> #define
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 24/09/16 18:40, Quentin Schulz wrote:
> Hi Jonathan,
>
> Sorry for the (long) delay, I did not have time to work on it. I'll
> mainly work in my free time now.
>
> Keep in mind this patch was proposed based on the v2 of the ADC patches.
> Since then, substantial changes have been made and I'm working on
> rebasing this series of patches on the v6, so comments here might
> include references to code parts added later in the ADC patch series.
>
> On 24/07/2016 13:03, Jonathan Cameron wrote:
>> On 20/07/16 09:29, Quentin Schulz wrote:
>>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>>> It also prepares the code which will be used by the touchscreen driver
>>> named sunxi-gpadc-ts.
>>>
>>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>>> consumer which will be in charge of reading data from these channels for
>>> the input framework.
>>>
>>> The temperature can only be read when in touchscreen mode. This means if
>>> the buffers are being used for the ADC, the temperature sensor cannot be
>>> read.
>> That may be the bizarest hardware restriction I've heard of in a while! :)
>>>
>>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>>> and fill a buffer before sending it to the consumers which registered in
>>> IIO for the ADC channels.
>>>
>>> When a consumer starts buffering ADC channels,
>>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>>> depending on a property of the DT ("allwinner,ts-attached").
>>> When the consumer stops buffering, it disables the same irq.
>> Hmm. Might be possible to distinguish which consumer caused the start.
>> Thus, if the touchscreen is there we would know purely based on the
>> driver being the requester that we need to be in touchscreen mode.
>>
>
> As of yet, can't see in which way I can retrieve the consumer in
> provider code. Maybe I'm missing something, I don't know?
I don't think we have a current way of doing this... Might be possible
to add one, but it would be a rather odd bit of reverse looking up.
>
>>>
>>> Signed-off-by: Quentin Schulz
>> You are moving fast on this - I'd have been tempted to do a mega
>> series with the updated version of the basic support and this on top
>> rather than a new unconnected series.
>>
>> (I'd forgotten that was still under review so got confused when I
>> went to look something up in the files you are modifying!).
>>> ---
>>> drivers/iio/adc/Kconfig | 1 +
>>> drivers/iio/adc/sunxi-gpadc-iio.c | 153
>>> ++
>>> 2 files changed, 138 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 184856f..15e3b08 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -342,6 +342,7 @@ config SUNXI_ADC
>>> tristate "ADC driver for sunxi platforms"
>>> depends on IIO
>>> depends on MFD_SUNXI_ADC
>>> + depends on IIO_BUFFER_CB
>>> help
>>> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
>>> ADC. This ADC provides 4 channels which can be used as an ADC or as a
>>> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
>>> b/drivers/iio/adc/sunxi-gpadc-iio.c
>>> index 87cc913..2e44ca7 100644
>>> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
>>> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
>>> @@ -16,8 +16,9 @@
>>> #include
>>> #include
>>>
>>> -#include
>>> +#include
>>> #include
>>> +#include
>> Can't say I'm a particular fan of reordering headers to be in alphabetical
>> order, but I suppose it doesn't really matter if you want to do it.
>> (to my mind there is a tree structure implicit in these headers with iio.h
>> at the top for generic support, then the various sub elements below).
>>
>>> #include
>>> #include
>>>
>>> @@ -71,6 +72,7 @@
>>> #define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
>>> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
>>> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
>>> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING
>>> interrupts */
>> Sounds like you learned that one the hard way ;)
>>> #define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
>>> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
>>> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
>>> @@ -79,6 +81,7 @@
>>> #define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
>>> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
>>> #define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
>>> +#define SUNXI_GPADC_RXA_CNTGENMASK(12, 8)
>>> #define SUNXI_GPADC_TP_IDLE_FLGBIT(2)
>>> #define SUNXI_GPADC_TP_UP_PENDING BIT(1)
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
Hi Jonathan,
Sorry for the (long) delay, I did not have time to work on it. I'll
mainly work in my free time now.
Keep in mind this patch was proposed based on the v2 of the ADC patches.
Since then, substantial changes have been made and I'm working on
rebasing this series of patches on the v6, so comments here might
include references to code parts added later in the ADC patch series.
On 24/07/2016 13:03, Jonathan Cameron wrote:
> On 20/07/16 09:29, Quentin Schulz wrote:
>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>> It also prepares the code which will be used by the touchscreen driver
>> named sunxi-gpadc-ts.
>>
>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>> consumer which will be in charge of reading data from these channels for
>> the input framework.
>>
>> The temperature can only be read when in touchscreen mode. This means if
>> the buffers are being used for the ADC, the temperature sensor cannot be
>> read.
> That may be the bizarest hardware restriction I've heard of in a while! :)
>>
>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>> and fill a buffer before sending it to the consumers which registered in
>> IIO for the ADC channels.
>>
>> When a consumer starts buffering ADC channels,
>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>> depending on a property of the DT ("allwinner,ts-attached").
>> When the consumer stops buffering, it disables the same irq.
> Hmm. Might be possible to distinguish which consumer caused the start.
> Thus, if the touchscreen is there we would know purely based on the
> driver being the requester that we need to be in touchscreen mode.
>
As of yet, can't see in which way I can retrieve the consumer in
provider code. Maybe I'm missing something, I don't know?
>>
>> Signed-off-by: Quentin Schulz
> You are moving fast on this - I'd have been tempted to do a mega
> series with the updated version of the basic support and this on top
> rather than a new unconnected series.
>
> (I'd forgotten that was still under review so got confused when I
> went to look something up in the files you are modifying!).
>> ---
>> drivers/iio/adc/Kconfig | 1 +
>> drivers/iio/adc/sunxi-gpadc-iio.c | 153
>> ++
>> 2 files changed, 138 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 184856f..15e3b08 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -342,6 +342,7 @@ config SUNXI_ADC
>> tristate "ADC driver for sunxi platforms"
>> depends on IIO
>> depends on MFD_SUNXI_ADC
>> +depends on IIO_BUFFER_CB
>> help
>>Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
>>ADC. This ADC provides 4 channels which can be used as an ADC or as a
>> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
>> b/drivers/iio/adc/sunxi-gpadc-iio.c
>> index 87cc913..2e44ca7 100644
>> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
>> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
>> @@ -16,8 +16,9 @@
>> #include
>> #include
>>
>> -#include
>> +#include
>> #include
>> +#include
> Can't say I'm a particular fan of reordering headers to be in alphabetical
> order, but I suppose it doesn't really matter if you want to do it.
> (to my mind there is a tree structure implicit in these headers with iio.h
> at the top for generic support, then the various sub elements below).
>
>> #include
>> #include
>>
>> @@ -71,6 +72,7 @@
>> #define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
>> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
>> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
>> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING
>> interrupts */
> Sounds like you learned that one the hard way ;)
>> #define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
>> #define SUNXI_GPADC_TP_UP_IRQ_ENBIT(1)
>> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
>> @@ -79,6 +81,7 @@
>> #define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
>> #define SUNXI_GPADC_FIFO_OVERRUN_PENDINGBIT(17)
>> #define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
>> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
>> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
>> #define SUNXI_GPADC_TP_UP_PENDING BIT(1)
>> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>> unsigned intfifo_data_irq;
>> unsigned inttemp_data_irq;
>> unsigned int
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
Hi Jonathan,
Sorry for the (long) delay, I did not have time to work on it. I'll
mainly work in my free time now.
Keep in mind this patch was proposed based on the v2 of the ADC patches.
Since then, substantial changes have been made and I'm working on
rebasing this series of patches on the v6, so comments here might
include references to code parts added later in the ADC patch series.
On 24/07/2016 13:03, Jonathan Cameron wrote:
> On 20/07/16 09:29, Quentin Schulz wrote:
>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>> It also prepares the code which will be used by the touchscreen driver
>> named sunxi-gpadc-ts.
>>
>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>> consumer which will be in charge of reading data from these channels for
>> the input framework.
>>
>> The temperature can only be read when in touchscreen mode. This means if
>> the buffers are being used for the ADC, the temperature sensor cannot be
>> read.
> That may be the bizarest hardware restriction I've heard of in a while! :)
>>
>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>> and fill a buffer before sending it to the consumers which registered in
>> IIO for the ADC channels.
>>
>> When a consumer starts buffering ADC channels,
>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>> depending on a property of the DT ("allwinner,ts-attached").
>> When the consumer stops buffering, it disables the same irq.
> Hmm. Might be possible to distinguish which consumer caused the start.
> Thus, if the touchscreen is there we would know purely based on the
> driver being the requester that we need to be in touchscreen mode.
>
As of yet, can't see in which way I can retrieve the consumer in
provider code. Maybe I'm missing something, I don't know?
>>
>> Signed-off-by: Quentin Schulz
> You are moving fast on this - I'd have been tempted to do a mega
> series with the updated version of the basic support and this on top
> rather than a new unconnected series.
>
> (I'd forgotten that was still under review so got confused when I
> went to look something up in the files you are modifying!).
>> ---
>> drivers/iio/adc/Kconfig | 1 +
>> drivers/iio/adc/sunxi-gpadc-iio.c | 153
>> ++
>> 2 files changed, 138 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 184856f..15e3b08 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -342,6 +342,7 @@ config SUNXI_ADC
>> tristate "ADC driver for sunxi platforms"
>> depends on IIO
>> depends on MFD_SUNXI_ADC
>> +depends on IIO_BUFFER_CB
>> help
>>Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
>>ADC. This ADC provides 4 channels which can be used as an ADC or as a
>> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
>> b/drivers/iio/adc/sunxi-gpadc-iio.c
>> index 87cc913..2e44ca7 100644
>> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
>> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
>> @@ -16,8 +16,9 @@
>> #include
>> #include
>>
>> -#include
>> +#include
>> #include
>> +#include
> Can't say I'm a particular fan of reordering headers to be in alphabetical
> order, but I suppose it doesn't really matter if you want to do it.
> (to my mind there is a tree structure implicit in these headers with iio.h
> at the top for generic support, then the various sub elements below).
>
>> #include
>> #include
>>
>> @@ -71,6 +72,7 @@
>> #define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
>> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
>> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
>> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING
>> interrupts */
> Sounds like you learned that one the hard way ;)
>> #define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
>> #define SUNXI_GPADC_TP_UP_IRQ_ENBIT(1)
>> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
>> @@ -79,6 +81,7 @@
>> #define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
>> #define SUNXI_GPADC_FIFO_OVERRUN_PENDINGBIT(17)
>> #define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
>> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
>> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
>> #define SUNXI_GPADC_TP_UP_PENDING BIT(1)
>> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>> unsigned intfifo_data_irq;
>> unsigned inttemp_data_irq;
>> unsigned intflags;
>> +struct iio_dev
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 20/07/16 09:29, Quentin Schulz wrote:
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
That may be the bizarest hardware restriction I've heard of in a while! :)
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
Hmm. Might be possible to distinguish which consumer caused the start.
Thus, if the touchscreen is there we would know purely based on the
driver being the requester that we need to be in touchscreen mode.
>
> Signed-off-by: Quentin Schulz
You are moving fast on this - I'd have been tempted to do a mega
series with the updated version of the basic support and this on top
rather than a new unconnected series.
(I'd forgotten that was still under review so got confused when I
went to look something up in the files you are modifying!).
> ---
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/sunxi-gpadc-iio.c | 153
> ++
> 2 files changed, 138 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 184856f..15e3b08 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -342,6 +342,7 @@ config SUNXI_ADC
> tristate "ADC driver for sunxi platforms"
> depends on IIO
> depends on MFD_SUNXI_ADC
> + depends on IIO_BUFFER_CB
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> ADC. This ADC provides 4 channels which can be used as an ADC or as a
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> index 87cc913..2e44ca7 100644
> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -16,8 +16,9 @@
> #include
> #include
>
> -#include
> +#include
> #include
> +#include
Can't say I'm a particular fan of reordering headers to be in alphabetical
order, but I suppose it doesn't really matter if you want to do it.
(to my mind there is a tree structure implicit in these headers with iio.h
at the top for generic support, then the various sub elements below).
> #include
> #include
>
> @@ -71,6 +72,7 @@
> #define SUNXI_GPADC_TP_DATA_XY_CHANGEBIT(13)
> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x)((x) << 8) /* 5 bits */
> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts
> */
Sounds like you learned that one the hard way ;)
> #define SUNXI_GPADC_TP_FIFO_FLUSHBIT(4)
> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
> @@ -79,6 +81,7 @@
> #define SUNXI_GPADC_TEMP_DATA_PENDINGBIT(18)
> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
> #define SUNXI_GPADC_FIFO_DATA_PENDINGBIT(16)
> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
> #define SUNXI_GPADC_TP_UP_PENDINGBIT(1)
> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
I was suprised to see this as normally it is cleaner to structure
the whole code to go in one direction through the structures (which is
why we don't provide a generic iio_device_from_priv bit of pointer magic).
Anyhow, don't htink you are actually using it ;)
> + struct sunxi_gpadc_buffer buffer;
> + boolts_attached;
> + boolbuffered;
> };
>
> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
> .type = IIO_VOLTAGE,
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 20/07/16 09:29, Quentin Schulz wrote:
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
That may be the bizarest hardware restriction I've heard of in a while! :)
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
Hmm. Might be possible to distinguish which consumer caused the start.
Thus, if the touchscreen is there we would know purely based on the
driver being the requester that we need to be in touchscreen mode.
>
> Signed-off-by: Quentin Schulz
You are moving fast on this - I'd have been tempted to do a mega
series with the updated version of the basic support and this on top
rather than a new unconnected series.
(I'd forgotten that was still under review so got confused when I
went to look something up in the files you are modifying!).
> ---
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/sunxi-gpadc-iio.c | 153
> ++
> 2 files changed, 138 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 184856f..15e3b08 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -342,6 +342,7 @@ config SUNXI_ADC
> tristate "ADC driver for sunxi platforms"
> depends on IIO
> depends on MFD_SUNXI_ADC
> + depends on IIO_BUFFER_CB
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> ADC. This ADC provides 4 channels which can be used as an ADC or as a
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> index 87cc913..2e44ca7 100644
> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -16,8 +16,9 @@
> #include
> #include
>
> -#include
> +#include
> #include
> +#include
Can't say I'm a particular fan of reordering headers to be in alphabetical
order, but I suppose it doesn't really matter if you want to do it.
(to my mind there is a tree structure implicit in these headers with iio.h
at the top for generic support, then the various sub elements below).
> #include
> #include
>
> @@ -71,6 +72,7 @@
> #define SUNXI_GPADC_TP_DATA_XY_CHANGEBIT(13)
> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x)((x) << 8) /* 5 bits */
> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts
> */
Sounds like you learned that one the hard way ;)
> #define SUNXI_GPADC_TP_FIFO_FLUSHBIT(4)
> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
> @@ -79,6 +81,7 @@
> #define SUNXI_GPADC_TEMP_DATA_PENDINGBIT(18)
> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
> #define SUNXI_GPADC_FIFO_DATA_PENDINGBIT(16)
> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
> #define SUNXI_GPADC_TP_UP_PENDINGBIT(1)
> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
I was suprised to see this as normally it is cleaner to structure
the whole code to go in one direction through the structures (which is
why we don't provide a generic iio_device_from_priv bit of pointer magic).
Anyhow, don't htink you are actually using it ;)
> + struct sunxi_gpadc_buffer buffer;
> + boolts_attached;
> + boolbuffered;
> };
>
> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
> .type = IIO_VOLTAGE,\
>
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 20/07/2016 10:38, Peter Meerwald-Stadler wrote:
>
>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>> It also prepares the code which will be used by the touchscreen driver
>> named sunxi-gpadc-ts.
>>
>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>> consumer which will be in charge of reading data from these channels for
>> the input framework.
>>
>> The temperature can only be read when in touchscreen mode. This means if
>> the buffers are being used for the ADC, the temperature sensor cannot be
>> read.
>>
>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>> and fill a buffer before sending it to the consumers which registered in
>> IIO for the ADC channels.
>>
>> When a consumer starts buffering ADC channels,
>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>> depending on a property of the DT ("allwinner,ts-attached").
>> When the consumer stops buffering, it disables the same irq.
>
> comments below
>
[...]
>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>> unsigned intfifo_data_irq;
>> unsigned inttemp_data_irq;
>> unsigned intflags;
>> +struct iio_dev *indio_dev;
>> +struct sunxi_gpadc_buffer buffer;
>> +boolts_attached;
>> +boolbuffered;
>
> why add buffered, duplicate state and not query iio_buffer_enabled()?
>
>> };
>>
>> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
>> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
>> .type = IIO_VOLTAGE,\
>> .indexed = 1, \
>> .channel = _channel,\
>> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> .datasheet_name = _name,\
>> +.scan_index = _index, \
>> +.scan_type = { \
>> +.sign = 'u',\
>> +.realbits = 12, \
>> +.storagebits = 16, \
>> +.shift = 0, \
>
> shift not strictly needed
>
ACK.
[...]
>> static int sunxi_gpadc_read_raw(struct iio_dev *indio_dev,
>> @@ -219,15 +253,22 @@ static int sunxi_gpadc_read_raw(struct iio_dev
>> *indio_dev,
>> int *val, int *val2, long mask)
>> {
>> int ret;
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>>
>> switch (mask) {
>> case IIO_CHAN_INFO_PROCESSED:
>> +if (info->buffered && !info->ts_attached)
>> +return -EBUSY;
>
> there would be iio_device_claim_direct_mode()
>
OK, iio_device_claim_direct_mode() and iio_device_release_direct_mode()
are new functions which are not yet in the Linux Cross Reference
(http://lxr.free-electrons.com/), I didn't know they existed. I'll use
that, iio_buffer_enabled() when needed and get rid of the buffered
boolean variable.
[...]
Thanks.
Quentin
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
On 20/07/2016 10:38, Peter Meerwald-Stadler wrote:
>
>> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
>> It also prepares the code which will be used by the touchscreen driver
>> named sunxi-gpadc-ts.
>>
>> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
>> conversion's data. The GPADC uses the same ADC channels for the ADC and the
>> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
>> consumer which will be in charge of reading data from these channels for
>> the input framework.
>>
>> The temperature can only be read when in touchscreen mode. This means if
>> the buffers are being used for the ADC, the temperature sensor cannot be
>> read.
>>
>> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
>> and fill a buffer before sending it to the consumers which registered in
>> IIO for the ADC channels.
>>
>> When a consumer starts buffering ADC channels,
>> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
>> irq and select the mode in which the GPADC should run (ADC or touchscreen)
>> depending on a property of the DT ("allwinner,ts-attached").
>> When the consumer stops buffering, it disables the same irq.
>
> comments below
>
[...]
>> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
>> unsigned intfifo_data_irq;
>> unsigned inttemp_data_irq;
>> unsigned intflags;
>> +struct iio_dev *indio_dev;
>> +struct sunxi_gpadc_buffer buffer;
>> +boolts_attached;
>> +boolbuffered;
>
> why add buffered, duplicate state and not query iio_buffer_enabled()?
>
>> };
>>
>> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
>> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
>> .type = IIO_VOLTAGE,\
>> .indexed = 1, \
>> .channel = _channel,\
>> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> .datasheet_name = _name,\
>> +.scan_index = _index, \
>> +.scan_type = { \
>> +.sign = 'u',\
>> +.realbits = 12, \
>> +.storagebits = 16, \
>> +.shift = 0, \
>
> shift not strictly needed
>
ACK.
[...]
>> static int sunxi_gpadc_read_raw(struct iio_dev *indio_dev,
>> @@ -219,15 +253,22 @@ static int sunxi_gpadc_read_raw(struct iio_dev
>> *indio_dev,
>> int *val, int *val2, long mask)
>> {
>> int ret;
>> +struct sunxi_gpadc_dev *info = iio_priv(indio_dev);
>>
>> switch (mask) {
>> case IIO_CHAN_INFO_PROCESSED:
>> +if (info->buffered && !info->ts_attached)
>> +return -EBUSY;
>
> there would be iio_device_claim_direct_mode()
>
OK, iio_device_claim_direct_mode() and iio_device_release_direct_mode()
are new functions which are not yet in the Linux Cross Reference
(http://lxr.free-electrons.com/), I didn't know they existed. I'll use
that, iio_buffer_enabled() when needed and get rid of the buffered
boolean variable.
[...]
Thanks.
Quentin
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
comments below
> Signed-off-by: Quentin Schulz
> ---
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/sunxi-gpadc-iio.c | 153
> ++
> 2 files changed, 138 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 184856f..15e3b08 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -342,6 +342,7 @@ config SUNXI_ADC
> tristate "ADC driver for sunxi platforms"
> depends on IIO
> depends on MFD_SUNXI_ADC
> + depends on IIO_BUFFER_CB
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> ADC. This ADC provides 4 channels which can be used as an ADC or as a
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> index 87cc913..2e44ca7 100644
> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -16,8 +16,9 @@
> #include
> #include
>
> -#include
> +#include
> #include
> +#include
> #include
> #include
>
> @@ -71,6 +72,7 @@
> #define SUNXI_GPADC_TP_DATA_XY_CHANGEBIT(13)
> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x)((x) << 8) /* 5 bits */
> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts
> */
> #define SUNXI_GPADC_TP_FIFO_FLUSHBIT(4)
> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
> @@ -79,6 +81,7 @@
> #define SUNXI_GPADC_TEMP_DATA_PENDINGBIT(18)
> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
> #define SUNXI_GPADC_FIFO_DATA_PENDINGBIT(16)
> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
> #define SUNXI_GPADC_TP_UP_PENDINGBIT(1)
> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
> + struct sunxi_gpadc_buffer buffer;
> + boolts_attached;
> + boolbuffered;
why add buffered, duplicate state and not query iio_buffer_enabled()?
> };
>
> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
> .type = IIO_VOLTAGE,\
> .indexed = 1, \
> .channel = _channel,\
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> .datasheet_name = _name,\
> + .scan_index = _index, \
> + .scan_type = { \
> + .sign = 'u',\
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 0, \
shift not strictly needed
> + .endianness = IIO_LE, \
> + }, \
> }
>
> static struct iio_map sunxi_gpadc_hwmon_maps[] = {
> {
> + .adc_channel_label = "adc_chan0",
> + .consumer_dev_name = "sunxi-gpadc-ts.0",
> + }, {
> +
Re: [PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
> This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
> It also prepares the code which will be used by the touchscreen driver
> named sunxi-gpadc-ts.
>
> The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
> conversion's data. The GPADC uses the same ADC channels for the ADC and the
> touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
> consumer which will be in charge of reading data from these channels for
> the input framework.
>
> The temperature can only be read when in touchscreen mode. This means if
> the buffers are being used for the ADC, the temperature sensor cannot be
> read.
>
> When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
> and fill a buffer before sending it to the consumers which registered in
> IIO for the ADC channels.
>
> When a consumer starts buffering ADC channels,
> sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
> irq and select the mode in which the GPADC should run (ADC or touchscreen)
> depending on a property of the DT ("allwinner,ts-attached").
> When the consumer stops buffering, it disables the same irq.
comments below
> Signed-off-by: Quentin Schulz
> ---
> drivers/iio/adc/Kconfig | 1 +
> drivers/iio/adc/sunxi-gpadc-iio.c | 153
> ++
> 2 files changed, 138 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 184856f..15e3b08 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -342,6 +342,7 @@ config SUNXI_ADC
> tristate "ADC driver for sunxi platforms"
> depends on IIO
> depends on MFD_SUNXI_ADC
> + depends on IIO_BUFFER_CB
> help
> Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
> ADC. This ADC provides 4 channels which can be used as an ADC or as a
> diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
> b/drivers/iio/adc/sunxi-gpadc-iio.c
> index 87cc913..2e44ca7 100644
> --- a/drivers/iio/adc/sunxi-gpadc-iio.c
> +++ b/drivers/iio/adc/sunxi-gpadc-iio.c
> @@ -16,8 +16,9 @@
> #include
> #include
>
> -#include
> +#include
> #include
> +#include
> #include
> #include
>
> @@ -71,6 +72,7 @@
> #define SUNXI_GPADC_TP_DATA_XY_CHANGEBIT(13)
> #define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x)((x) << 8) /* 5 bits */
> #define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
> +/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts
> */
> #define SUNXI_GPADC_TP_FIFO_FLUSHBIT(4)
> #define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
> #define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
> @@ -79,6 +81,7 @@
> #define SUNXI_GPADC_TEMP_DATA_PENDINGBIT(18)
> #define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
> #define SUNXI_GPADC_FIFO_DATA_PENDINGBIT(16)
> +#define SUNXI_GPADC_RXA_CNT GENMASK(12, 8)
> #define SUNXI_GPADC_TP_IDLE_FLG BIT(2)
> #define SUNXI_GPADC_TP_UP_PENDINGBIT(1)
> #define SUNXI_GPADC_TP_DOWN_PENDING BIT(0)
> @@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
> unsigned intfifo_data_irq;
> unsigned inttemp_data_irq;
> unsigned intflags;
> + struct iio_dev *indio_dev;
> + struct sunxi_gpadc_buffer buffer;
> + boolts_attached;
> + boolbuffered;
why add buffered, duplicate state and not query iio_buffer_enabled()?
> };
>
> -#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
> +#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
> .type = IIO_VOLTAGE,\
> .indexed = 1, \
> .channel = _channel,\
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> .datasheet_name = _name,\
> + .scan_index = _index, \
> + .scan_type = { \
> + .sign = 'u',\
> + .realbits = 12, \
> + .storagebits = 16, \
> + .shift = 0, \
shift not strictly needed
> + .endianness = IIO_LE, \
> + }, \
> }
>
> static struct iio_map sunxi_gpadc_hwmon_maps[] = {
> {
> + .adc_channel_label = "adc_chan0",
> + .consumer_dev_name = "sunxi-gpadc-ts.0",
> + }, {
> + .adc_channel_label = "adc_chan1",
> +
[PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
It also prepares the code which will be used by the touchscreen driver
named sunxi-gpadc-ts.
The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
conversion's data. The GPADC uses the same ADC channels for the ADC and the
touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
consumer which will be in charge of reading data from these channels for
the input framework.
The temperature can only be read when in touchscreen mode. This means if
the buffers are being used for the ADC, the temperature sensor cannot be
read.
When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
and fill a buffer before sending it to the consumers which registered in
IIO for the ADC channels.
When a consumer starts buffering ADC channels,
sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
irq and select the mode in which the GPADC should run (ADC or touchscreen)
depending on a property of the DT ("allwinner,ts-attached").
When the consumer stops buffering, it disables the same irq.
Signed-off-by: Quentin Schulz
---
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/sunxi-gpadc-iio.c | 153 ++
2 files changed, 138 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 184856f..15e3b08 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -342,6 +342,7 @@ config SUNXI_ADC
tristate "ADC driver for sunxi platforms"
depends on IIO
depends on MFD_SUNXI_ADC
+ depends on IIO_BUFFER_CB
help
Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
ADC. This ADC provides 4 channels which can be used as an ADC or as a
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
b/drivers/iio/adc/sunxi-gpadc-iio.c
index 87cc913..2e44ca7 100644
--- a/drivers/iio/adc/sunxi-gpadc-iio.c
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -16,8 +16,9 @@
#include
#include
-#include
+#include
#include
+#include
#include
#include
@@ -71,6 +72,7 @@
#define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
#define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
#define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
+/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts */
#define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
#define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
#define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
@@ -79,6 +81,7 @@
#define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
#define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
#define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
+#define SUNXI_GPADC_RXA_CNTGENMASK(12, 8)
#define SUNXI_GPADC_TP_IDLE_FLGBIT(2)
#define SUNXI_GPADC_TP_UP_PENDING BIT(1)
#define SUNXI_GPADC_TP_DOWN_PENDINGBIT(0)
@@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
unsigned intfifo_data_irq;
unsigned inttemp_data_irq;
unsigned intflags;
+ struct iio_dev *indio_dev;
+ struct sunxi_gpadc_buffer buffer;
+ boolts_attached;
+ boolbuffered;
};
-#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
+#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
.type = IIO_VOLTAGE,\
.indexed = 1, \
.channel = _channel,\
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
.datasheet_name = _name,\
+ .scan_index = _index, \
+ .scan_type = { \
+ .sign = 'u',\
+ .realbits = 12, \
+ .storagebits = 16, \
+ .shift = 0, \
+ .endianness = IIO_LE, \
+ }, \
}
static struct iio_map sunxi_gpadc_hwmon_maps[] = {
{
+ .adc_channel_label = "adc_chan0",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan1",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan2",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan3",
[PATCH 3/5] iio: adc: sunxi-gpadc-iio: enable iio_buffers
This enables the use of buffers on ADC channels of sunxi-gpadc-iio driver.
It also prepares the code which will be used by the touchscreen driver
named sunxi-gpadc-ts.
The GPADC on Allwinner SoCs (A10, A13 and A31) has a 12 bits register for
conversion's data. The GPADC uses the same ADC channels for the ADC and the
touchscreen therefore exposes these channels to the sunxi-gpadc-ts iio
consumer which will be in charge of reading data from these channels for
the input framework.
The temperature can only be read when in touchscreen mode. This means if
the buffers are being used for the ADC, the temperature sensor cannot be
read.
When a FIFO_DATA_PENDING irq occurs, its handler will read the entire FIFO
and fill a buffer before sending it to the consumers which registered in
IIO for the ADC channels.
When a consumer starts buffering ADC channels,
sunxi_gpadc_buffer_postenable is called and will enable FIFO_DATA_PENDING
irq and select the mode in which the GPADC should run (ADC or touchscreen)
depending on a property of the DT ("allwinner,ts-attached").
When the consumer stops buffering, it disables the same irq.
Signed-off-by: Quentin Schulz
---
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/sunxi-gpadc-iio.c | 153 ++
2 files changed, 138 insertions(+), 16 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 184856f..15e3b08 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -342,6 +342,7 @@ config SUNXI_ADC
tristate "ADC driver for sunxi platforms"
depends on IIO
depends on MFD_SUNXI_ADC
+ depends on IIO_BUFFER_CB
help
Say yes here to build support for Allwinner (A10, A13 and A31) SoCs
ADC. This ADC provides 4 channels which can be used as an ADC or as a
diff --git a/drivers/iio/adc/sunxi-gpadc-iio.c
b/drivers/iio/adc/sunxi-gpadc-iio.c
index 87cc913..2e44ca7 100644
--- a/drivers/iio/adc/sunxi-gpadc-iio.c
+++ b/drivers/iio/adc/sunxi-gpadc-iio.c
@@ -16,8 +16,9 @@
#include
#include
-#include
+#include
#include
+#include
#include
#include
@@ -71,6 +72,7 @@
#define SUNXI_GPADC_TP_DATA_XY_CHANGE BIT(13)
#define SUNXI_GPADC_TP_FIFO_TRIG_LEVEL(x) ((x) << 8) /* 5 bits */
#define SUNXI_GPADC_TP_DATA_DRQ_EN BIT(7)
+/* Be careful, flushing FIFO spawns SUNXI_GPADC_FIFO_DATA_PENDING interrupts */
#define SUNXI_GPADC_TP_FIFO_FLUSH BIT(4)
#define SUNXI_GPADC_TP_UP_IRQ_EN BIT(1)
#define SUNXI_GPADC_TP_DOWN_IRQ_EN BIT(0)
@@ -79,6 +81,7 @@
#define SUNXI_GPADC_TEMP_DATA_PENDING BIT(18)
#define SUNXI_GPADC_FIFO_OVERRUN_PENDING BIT(17)
#define SUNXI_GPADC_FIFO_DATA_PENDING BIT(16)
+#define SUNXI_GPADC_RXA_CNTGENMASK(12, 8)
#define SUNXI_GPADC_TP_IDLE_FLGBIT(2)
#define SUNXI_GPADC_TP_UP_PENDING BIT(1)
#define SUNXI_GPADC_TP_DOWN_PENDINGBIT(0)
@@ -101,19 +104,43 @@ struct sunxi_gpadc_dev {
unsigned intfifo_data_irq;
unsigned inttemp_data_irq;
unsigned intflags;
+ struct iio_dev *indio_dev;
+ struct sunxi_gpadc_buffer buffer;
+ boolts_attached;
+ boolbuffered;
};
-#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name) { \
+#define SUNXI_GPADC_ADC_CHANNEL(_channel, _name, _index) { \
.type = IIO_VOLTAGE,\
.indexed = 1, \
.channel = _channel,\
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
.datasheet_name = _name,\
+ .scan_index = _index, \
+ .scan_type = { \
+ .sign = 'u',\
+ .realbits = 12, \
+ .storagebits = 16, \
+ .shift = 0, \
+ .endianness = IIO_LE, \
+ }, \
}
static struct iio_map sunxi_gpadc_hwmon_maps[] = {
{
+ .adc_channel_label = "adc_chan0",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan1",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan2",
+ .consumer_dev_name = "sunxi-gpadc-ts.0",
+ }, {
+ .adc_channel_label = "adc_chan3",
+ .consumer_dev_name
