Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1645662 > unrolled thread
| Started by | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| First post | 2017-05-19 16:50 +0200 |
| Last post | 2017-05-22 19:30 +0200 |
| Articles | 12 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 1/4] iio: hi8435: add raw access Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-19 16:50 +0200
[PATCH 4/4] iio: hi8435: cleanup reset gpio Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-19 16:50 +0200
Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio Jonathan Cameron <jic23@kernel.org> - 2017-05-20 18:40 +0200
Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-23 10:20 +0200
Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio Vladimir Barinov <vladimir.barinov@cogentembedded.com> - 2017-05-24 13:30 +0200
Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio Jonathan Cameron <jic23@kernel.org> - 2017-05-24 21:40 +0200
[PATCH 2/4] iio: hi8435: avoid garbage event at first enable Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-19 16:50 +0200
Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable Jonathan Cameron <jic23@kernel.org> - 2017-05-20 18:40 +0200
Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable Vladimir Barinov <vladimir.barinov@cogentembedded.com> - 2017-05-22 20:30 +0200
Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-23 09:30 +0200
Re: [PATCH 1/4] iio: hi8435: add raw access Jonathan Cameron <jic23@kernel.org> - 2017-05-20 18:40 +0200
Re: [PATCH 1/4] iio: hi8435: add raw access Vladimir Barinov <vladimir.barinov@cogentembedded.com> - 2017-05-22 19:30 +0200
| From | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| Date | 2017-05-19 16:50 +0200 |
| Subject | [PATCH 1/4] iio: hi8435: add raw access |
| Message-ID | <tIRo5-3Bo-5@gated-at.bofh.it> |
With current event-only driver, it is not possible for user space
application to know current senses if they don't change since
application starts.
Address that by adding raw access to channels.
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
drivers/iio/adc/hi8435.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
index 678e8c7ea763..cb8e6342eddf 100644
--- a/drivers/iio/adc/hi8435.c
+++ b/drivers/iio/adc/hi8435.c
@@ -105,6 +105,26 @@ static int hi8435_writew(struct hi8435_priv *priv, u8 reg, u16 val)
return spi_write(priv->spi, priv->reg_buffer, 3);
}
+static int hi8435_read_raw(struct iio_dev *idev,
+ const struct iio_chan_spec *chan,
+ int *val, int *val2, long mask)
+{
+ struct hi8435_priv *priv = iio_priv(idev);
+ u32 tmp;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
+ if (ret < 0)
+ return ret;
+ *val = !!(tmp & BIT(chan->channel));
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
static int hi8435_read_event_config(struct iio_dev *idev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
@@ -333,6 +353,7 @@ static const struct iio_chan_spec_ext_info hi8435_ext_info[] = {
.type = IIO_VOLTAGE, \
.indexed = 1, \
.channel = num, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.event_spec = hi8435_events, \
.num_event_specs = ARRAY_SIZE(hi8435_events), \
.ext_info = hi8435_ext_info, \
@@ -376,6 +397,7 @@ static const struct iio_chan_spec hi8435_channels[] = {
static const struct iio_info hi8435_info = {
.driver_module = THIS_MODULE,
+ .read_raw = hi8435_read_raw,
.read_event_config = &hi8435_read_event_config,
.write_event_config = hi8435_write_event_config,
.read_event_value = &hi8435_read_event_value,
--
2.11.0
[toc] | [next] | [standalone]
| From | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| Date | 2017-05-19 16:50 +0200 |
| Subject | [PATCH 4/4] iio: hi8435: cleanup reset gpio |
| Message-ID | <tIRo6-3Bo-15@gated-at.bofh.it> |
| In reply to | #1645662 |
Reset GPIO is active low.
Currently driver uses gpiod_set_value(1) to clean reset, which depends
on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality.
This fixes driver to use _raw version of gpiod_set_value() to enforce
active-low semantics despite of what's written in device tree. Allowing
device tree to override that only opens possibility for errors and does
not add any value.
Additionally, use _cansleep version to make things work with i2c-gpio
and other sleeping gpio drivers.
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
drivers/iio/adc/hi8435.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
index d09cb6ff8044..ab59969b7c49 100644
--- a/drivers/iio/adc/hi8435.c
+++ b/drivers/iio/adc/hi8435.c
@@ -476,13 +476,15 @@ static int hi8435_probe(struct spi_device *spi)
priv->spi = spi;
reset_gpio = devm_gpiod_get(&spi->dev, NULL, GPIOD_OUT_LOW);
- if (IS_ERR(reset_gpio)) {
- /* chip s/w reset if h/w reset failed */
+ if (!IS_ERR(reset_gpio)) {
+ /* need >=100ns low pulse to reset chip */
+ gpiod_set_raw_value_cansleep(reset_gpio, 0);
+ udelay(1);
+ gpiod_set_raw_value_cansleep(reset_gpio, 1);
+ } else {
+ /* s/w reset chip if h/w reset is not available */
hi8435_writeb(priv, HI8435_CTRL_REG, HI8435_CTRL_SRST);
hi8435_writeb(priv, HI8435_CTRL_REG, 0);
- } else {
- udelay(5);
- gpiod_set_value(reset_gpio, 1);
}
spi_set_drvdata(spi, idev);
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2017-05-20 18:40 +0200 |
| Subject | Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio |
| Message-ID | <tJfA6-3RZ-19@gated-at.bofh.it> |
| In reply to | #1645663 |
On 19/05/17 15:48, Nikita Yushchenko wrote:
> Reset GPIO is active low.
>
> Currently driver uses gpiod_set_value(1) to clean reset, which depends
> on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality.
>
> This fixes driver to use _raw version of gpiod_set_value() to enforce
> active-low semantics despite of what's written in device tree. Allowing
> device tree to override that only opens possibility for errors and does
> not add any value.
>
> Additionally, use _cansleep version to make things work with i2c-gpio
> and other sleeping gpio drivers.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Hopefully we don't have any 'interesting' wiring schemes out there
that actually have a not gate on that wire.
Your argument seems sound to me so applied to the togreg branch
of iio.git and pushed out as testing for the autobuilders to play with
it.
Again, input from Vladimir would be particularly welcome!
Jonathan
> ---
> drivers/iio/adc/hi8435.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
> index d09cb6ff8044..ab59969b7c49 100644
> --- a/drivers/iio/adc/hi8435.c
> +++ b/drivers/iio/adc/hi8435.c
> @@ -476,13 +476,15 @@ static int hi8435_probe(struct spi_device *spi)
> priv->spi = spi;
>
> reset_gpio = devm_gpiod_get(&spi->dev, NULL, GPIOD_OUT_LOW);
> - if (IS_ERR(reset_gpio)) {
> - /* chip s/w reset if h/w reset failed */
> + if (!IS_ERR(reset_gpio)) {
> + /* need >=100ns low pulse to reset chip */
> + gpiod_set_raw_value_cansleep(reset_gpio, 0);
> + udelay(1);
> + gpiod_set_raw_value_cansleep(reset_gpio, 1);
> + } else {
> + /* s/w reset chip if h/w reset is not available */
> hi8435_writeb(priv, HI8435_CTRL_REG, HI8435_CTRL_SRST);
> hi8435_writeb(priv, HI8435_CTRL_REG, 0);
> - } else {
> - udelay(5);
> - gpiod_set_value(reset_gpio, 1);
> }
>
> spi_set_drvdata(spi, idev);
>
[toc] | [prev] | [next] | [standalone]
| From | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| Date | 2017-05-23 10:20 +0200 |
| Subject | Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio |
| Message-ID | <tKdcS-x7-17@gated-at.bofh.it> |
| In reply to | #1645663 |
>> Reset GPIO is active low. >> >> Currently driver uses gpiod_set_value(1) to clean reset, which depends >> on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality. >> >> This fixes driver to use _raw version of gpiod_set_value() to enforce >> active-low semantics despite of what's written in device tree. Allowing >> device tree to override that only opens possibility for errors and does >> not add any value. >> >> Additionally, use _cansleep version to make things work with i2c-gpio >> and other sleeping gpio drivers. > > The reset gpio comes from platform hence it should be handled by DTS. > > In driver the gpio should not be raw. > > Even the hi8435 is active low but platform may invert signal (f.e. by > adding trigger on the circuit path). I see. However - isn't this pure theoretic? Does such case exist? In vast majority of cases, GPIO polarity is chip-specific, not chip-use-specific. Thus this knowlege belongs to driver and not to device tree describing particular chip usage. Having this always defined at usage side is IMO major source of errors. In this particular case, we already have device trees in the wild with polarity set to active_high. If we now change that to require active_low, we break existing setups. Not sure this is acceptable. I'm unsure what is good way forward here. I believe it is at gpiolib level, which perhaps should change how "logical value" is defined, such that knowledge of chip-specific information is not forced to chip usage scope. Nikita
[toc] | [prev] | [next] | [standalone]
| From | Vladimir Barinov <vladimir.barinov@cogentembedded.com> |
|---|---|
| Date | 2017-05-24 13:30 +0200 |
| Subject | Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio |
| Message-ID | <tKCEi-1Hm-11@gated-at.bofh.it> |
| In reply to | #1647782 |
On 23.05.2017 11:18, Nikita Yushchenko wrote: >>> Reset GPIO is active low. >>> >>> Currently driver uses gpiod_set_value(1) to clean reset, which depends >>> on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality. >>> >>> This fixes driver to use _raw version of gpiod_set_value() to enforce >>> active-low semantics despite of what's written in device tree. Allowing >>> device tree to override that only opens possibility for errors and does >>> not add any value. >>> >>> Additionally, use _cansleep version to make things work with i2c-gpio >>> and other sleeping gpio drivers. >> The reset gpio comes from platform hence it should be handled by DTS. >> >> In driver the gpio should not be raw. >> >> Even the hi8435 is active low but platform may invert signal (f.e. by >> adding trigger on the circuit path). > I see. However - isn't this pure theoretic? Does such case exist? I assure you that this is frequently used. Simply search google for "simple voltage level shifter" It might be on PNP or NPN transistor, hence logic might be inverted. > > In vast majority of cases, GPIO polarity is chip-specific, not > chip-use-specific. Thus this knowlege belongs to driver and not to > device tree describing particular chip usage. Having this always > defined at usage side is IMO major source of errors. GPIO comes from SoC then "circuit path" and finally chip reset input. What do you propose if h/w circuit path has simple voltage level shifter on transistor. How to differentiate PNP and NPN cases? Regards, Vladimir
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2017-05-24 21:40 +0200 |
| Subject | Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio |
| Message-ID | <tKKit-6rO-1@gated-at.bofh.it> |
| In reply to | #1649488 |
On Wed, 24 May 2017 14:27:50 +0300 Vladimir Barinov <vladimir.barinov@cogentembedded.com> wrote: > On 23.05.2017 11:18, Nikita Yushchenko wrote: > >>> Reset GPIO is active low. > >>> > >>> Currently driver uses gpiod_set_value(1) to clean reset, which depends > >>> on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality. > >>> > >>> This fixes driver to use _raw version of gpiod_set_value() to enforce > >>> active-low semantics despite of what's written in device tree. Allowing > >>> device tree to override that only opens possibility for errors and does > >>> not add any value. > >>> > >>> Additionally, use _cansleep version to make things work with i2c-gpio > >>> and other sleeping gpio drivers. > >> The reset gpio comes from platform hence it should be handled by DTS. > >> > >> In driver the gpio should not be raw. > >> > >> Even the hi8435 is active low but platform may invert signal (f.e. by > >> adding trigger on the circuit path). > > I see. However - isn't this pure theoretic? Does such case exist? > I assure you that this is frequently used. > > Simply search google for "simple voltage level shifter" > It might be on PNP or NPN transistor, hence logic might be inverted. > > > > > In vast majority of cases, GPIO polarity is chip-specific, not > > chip-use-specific. Thus this knowlege belongs to driver and not to > > device tree describing particular chip usage. Having this always > > defined at usage side is IMO major source of errors. > GPIO comes from SoC then "circuit path" and finally chip reset input. > > What do you propose if h/w circuit path has simple voltage level shifter > on transistor. How to differentiate PNP and NPN cases? > > Regards, > Vladimir > Hmm. Ah well, I clearly jumped too fast on this set and should have left it for a while longer (I rushed a little as I'm away next weekend and the cycle is moving towards rc3) Sorry about that. Anyhow, I am tempted to queue a revert of this patch. The level shifting case hadn't occurred to me (oops). Thoughts? I'm travelling at the end of this week, so may be the middle of next before I can do anything about this one. Jonathan
[toc] | [prev] | [next] | [standalone]
| From | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| Date | 2017-05-19 16:50 +0200 |
| Subject | [PATCH 2/4] iio: hi8435: avoid garbage event at first enable |
| Message-ID | <tIRo6-3Bo-17@gated-at.bofh.it> |
| In reply to | #1645662 |
Currently, driver generates events for channels if new reading differs
from previous one. This "previous value" is initialized to zero, which
results into event if value is constant-one.
Fix that by initializing "previous value" by reading at event enable
time.
This provides reliable sequence for userspace:
- enable event,
- AFTER THAT read current value,
- AFTER THAT each event will correspond to change.
Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
---
drivers/iio/adc/hi8435.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
index cb8e6342eddf..45a92e3e8f2b 100644
--- a/drivers/iio/adc/hi8435.c
+++ b/drivers/iio/adc/hi8435.c
@@ -141,10 +141,21 @@ static int hi8435_write_event_config(struct iio_dev *idev,
enum iio_event_direction dir, int state)
{
struct hi8435_priv *priv = iio_priv(idev);
+ int ret;
+ u32 tmp;
+
+ if (state) {
+ ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
+ if (ret < 0)
+ return ret;
+ if (tmp & BIT(chan->channel))
+ priv->event_prev_val |= BIT(chan->channel);
+ else
+ priv->event_prev_val &= ~BIT(chan->channel);
- priv->event_scan_mask &= ~BIT(chan->channel);
- if (state)
priv->event_scan_mask |= BIT(chan->channel);
+ } else
+ priv->event_scan_mask &= ~BIT(chan->channel);
return 0;
}
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2017-05-20 18:40 +0200 |
| Subject | Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable |
| Message-ID | <tJfA6-3RZ-9@gated-at.bofh.it> |
| In reply to | #1645664 |
On 19/05/17 15:48, Nikita Yushchenko wrote:
> Currently, driver generates events for channels if new reading differs
> from previous one. This "previous value" is initialized to zero, which
> results into event if value is constant-one.
>
> Fix that by initializing "previous value" by reading at event enable
> time.
>
> This provides reliable sequence for userspace:
> - enable event,
> - AFTER THAT read current value,
> - AFTER THAT each event will correspond to change.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
I'm hoping there aren't any userspace apps out there relying on this
'unusual' behaviour. *cross fingers*
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/hi8435.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
> index cb8e6342eddf..45a92e3e8f2b 100644
> --- a/drivers/iio/adc/hi8435.c
> +++ b/drivers/iio/adc/hi8435.c
> @@ -141,10 +141,21 @@ static int hi8435_write_event_config(struct iio_dev *idev,
> enum iio_event_direction dir, int state)
> {
> struct hi8435_priv *priv = iio_priv(idev);
> + int ret;
> + u32 tmp;
> +
> + if (state) {
> + ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
> + if (ret < 0)
> + return ret;
> + if (tmp & BIT(chan->channel))
> + priv->event_prev_val |= BIT(chan->channel);
> + else
> + priv->event_prev_val &= ~BIT(chan->channel);
>
> - priv->event_scan_mask &= ~BIT(chan->channel);
> - if (state)
> priv->event_scan_mask |= BIT(chan->channel);
> + } else
> + priv->event_scan_mask &= ~BIT(chan->channel);
>
> return 0;
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Vladimir Barinov <vladimir.barinov@cogentembedded.com> |
|---|---|
| Date | 2017-05-22 20:30 +0200 |
| Subject | Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable |
| Message-ID | <tK0fE-HF-17@gated-at.bofh.it> |
| In reply to | #1645664 |
Hi Nikita,
On 19.05.2017 17:48, Nikita Yushchenko wrote:
> Currently, driver generates events for channels if new reading differs
> from previous one. This "previous value" is initialized to zero, which
> results into event if value is constant-one.
>
> Fix that by initializing "previous value" by reading at event enable
> time.
>
> This provides reliable sequence for userspace:
> - enable event,
> - AFTER THAT read current value,
> - AFTER THAT each event will correspond to change.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> ---
> drivers/iio/adc/hi8435.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
> index cb8e6342eddf..45a92e3e8f2b 100644
> --- a/drivers/iio/adc/hi8435.c
> +++ b/drivers/iio/adc/hi8435.c
> @@ -141,10 +141,21 @@ static int hi8435_write_event_config(struct iio_dev *idev,
> enum iio_event_direction dir, int state)
> {
> struct hi8435_priv *priv = iio_priv(idev);
> + int ret;
> + u32 tmp;
> +
> + if (state) {
> + ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
> + if (ret < 0)
> + return ret;
> + if (tmp & BIT(chan->channel))
> + priv->event_prev_val |= BIT(chan->channel);
> + else
> + priv->event_prev_val &= ~BIT(chan->channel);
>
> - priv->event_scan_mask &= ~BIT(chan->channel);
> - if (state)
> priv->event_scan_mask |= BIT(chan->channel);
> + } else
> + priv->event_scan_mask &= ~BIT(chan->channel);
>
This will race with hi8435_iio_push_event for priv->event_scan_mask.
Since you adding raw access then it is reasonable to initialize
priv->event_prev_val in hi8435_read_raw.
Then use the following sequence for your application:
1. read raw value
2. enable events
Regards,
Vladimir
[toc] | [prev] | [next] | [standalone]
| From | Nikita Yushchenko <nikita.yoush@cogentembedded.com> |
|---|---|
| Date | 2017-05-23 09:30 +0200 |
| Subject | Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable |
| Message-ID | <tKcqu-8q9-21@gated-at.bofh.it> |
| In reply to | #1647250 |
>> + int ret;
>> + u32 tmp;
>> +
>> + if (state) {
>> + ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
>> + if (ret < 0)
>> + return ret;
>> + if (tmp & BIT(chan->channel))
>> + priv->event_prev_val |= BIT(chan->channel);
>> + else
>> + priv->event_prev_val &= ~BIT(chan->channel);
>> - priv->event_scan_mask &= ~BIT(chan->channel);
>> - if (state)
>> priv->event_scan_mask |= BIT(chan->channel);
>> + } else
>> + priv->event_scan_mask &= ~BIT(chan->channel);
>>
>
> This will race with hi8435_iio_push_event for priv->event_scan_mask.
I was under impression that mutual-exclusion is provided by core. Now I
see it is not. Will submit a fix soon.
> Since you adding raw access then it is reasonable to initialize
> priv->event_prev_val in hi8435_read_raw.
This will cause complex interdependency between events and raw access.
E.g. should we generate event if change was discovered while processing
raw access from user space? If we do, then we break semantics "events
generated only under trigger". If we don't, we need complex state handling.
> Then use the following sequence for your application:
> 1. read raw value
> 2. enable events
So what if:
- one application calls read raw,
- then, far later, signal changes,
- then, far later, other application enables events?
Should second application get notification of change that happened long
before in enabled events? I think no.
There is fundamental race between start-of-monitoring and
change-of-signal-near-start-of-monitoring. To avoid it, reading signal
must be either atomic with start of monitoring (which is not possible
with IIO API), or done after start of monitoring (which moves handling
of this race to user side).
Nikita
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2017-05-20 18:40 +0200 |
| Message-ID | <tJfA6-3RZ-13@gated-at.bofh.it> |
| In reply to | #1645662 |
On 19/05/17 15:47, Nikita Yushchenko wrote:
> With current event-only driver, it is not possible for user space
> application to know current senses if they don't change since
> application starts.
>
> Address that by adding raw access to channels.
>
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Ideally I'd like Vladimir's ack on these, but I am going to guess
that he is fine with this and rely on him shouting if not ;)
Applied to the togreg branch of iio.git and pushed out as testing.
Thanks,
Jonathan
> ---
> drivers/iio/adc/hi8435.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/iio/adc/hi8435.c b/drivers/iio/adc/hi8435.c
> index 678e8c7ea763..cb8e6342eddf 100644
> --- a/drivers/iio/adc/hi8435.c
> +++ b/drivers/iio/adc/hi8435.c
> @@ -105,6 +105,26 @@ static int hi8435_writew(struct hi8435_priv *priv, u8 reg, u16 val)
> return spi_write(priv->spi, priv->reg_buffer, 3);
> }
>
> +static int hi8435_read_raw(struct iio_dev *idev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct hi8435_priv *priv = iio_priv(idev);
> + u32 tmp;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
> + if (ret < 0)
> + return ret;
> + *val = !!(tmp & BIT(chan->channel));
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static int hi8435_read_event_config(struct iio_dev *idev,
> const struct iio_chan_spec *chan,
> enum iio_event_type type,
> @@ -333,6 +353,7 @@ static const struct iio_chan_spec_ext_info hi8435_ext_info[] = {
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> .channel = num, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> .event_spec = hi8435_events, \
> .num_event_specs = ARRAY_SIZE(hi8435_events), \
> .ext_info = hi8435_ext_info, \
> @@ -376,6 +397,7 @@ static const struct iio_chan_spec hi8435_channels[] = {
>
> static const struct iio_info hi8435_info = {
> .driver_module = THIS_MODULE,
> + .read_raw = hi8435_read_raw,
> .read_event_config = &hi8435_read_event_config,
> .write_event_config = hi8435_write_event_config,
> .read_event_value = &hi8435_read_event_value,
>
[toc] | [prev] | [next] | [standalone]
| From | Vladimir Barinov <vladimir.barinov@cogentembedded.com> |
|---|---|
| Date | 2017-05-22 19:30 +0200 |
| Message-ID | <tJZjA-7i-11@gated-at.bofh.it> |
| In reply to | #1646153 |
On 20.05.2017 19:33, Jonathan Cameron wrote: > On 19/05/17 15:47, Nikita Yushchenko wrote: >> With current event-only driver, it is not possible for user space >> application to know current senses if they don't change since >> application starts. >> >> Address that by adding raw access to channels. >> >> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > Ideally I'd like Vladimir's ack on these, but I am going to guess > that he is fine with this and rely on him shouting if not ;) Acked-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> Originally it was a part of the patch for HI8435 buffer interface but removed during migration to event interface: http://marc.info/?l=linux-iio&m=143817462614547&w=4 This is a case when raw access is needed.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web