* [PATCH 1/1] iio: inkern: add helper to enable a channel
@ 2017-10-05 2:37 Phil Reid
2017-10-08 10:36 ` Jonathan Cameron
2017-10-08 10:41 ` Jonathan Cameron
0 siblings, 2 replies; 6+ messages in thread
From: Phil Reid @ 2017-10-05 2:37 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, preid, linux-iio
This is done by calling a generic write attribute helper similar
to the existing read attribute helper. Update write raw helper
to use new write attribute function.
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/iio/inkern.c | 17 +++++++++++++++--
include/linux/iio/consumer.h | 9 +++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 069defc..51c5c967 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -850,7 +850,9 @@ static int iio_channel_write(struct iio_channel *chan, int val, int val2,
chan->channel, val, val2, info);
}
-int iio_write_channel_raw(struct iio_channel *chan, int val)
+static int iio_write_channel_attribute(struct iio_channel *chan,
+ int val, int val2,
+ enum iio_chan_info_enum attribute)
{
int ret;
@@ -860,14 +862,25 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
goto err_unlock;
}
- ret = iio_channel_write(chan, val, 0, IIO_CHAN_INFO_RAW);
+ ret = iio_channel_write(chan, val, val2, attribute);
err_unlock:
mutex_unlock(&chan->indio_dev->info_exist_lock);
return ret;
}
+
+int iio_write_channel_raw(struct iio_channel *chan, int val)
+{
+ return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_RAW);
+}
EXPORT_SYMBOL_GPL(iio_write_channel_raw);
+int iio_write_channel_enable(struct iio_channel *chan, int val)
+{
+ return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_ENABLE);
+}
+EXPORT_SYMBOL_GPL(iio_write_channel_enable);
+
unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
{
const struct iio_chan_spec_ext_info *ext_info;
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 5e347a9..4a5a90d 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -226,6 +226,15 @@ int iio_read_channel_raw(struct iio_channel *chan,
int iio_write_channel_raw(struct iio_channel *chan, int val);
/**
+ * iio_write_channel_enable() - enable a given channel
+ * @chan: The channel being queried.
+ * @val: Value being written.
+ *
+ * Enable / disable the channel.
+ */
+int iio_write_channel_enable(struct iio_channel *chan, int val);
+
+/**
* iio_read_max_channel_raw() - read maximum available raw value from a given
* channel, i.e. the maximum possible value.
* @chan: The channel being queried.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] iio: inkern: add helper to enable a channel
2017-10-05 2:37 [PATCH 1/1] iio: inkern: add helper to enable a channel Phil Reid
@ 2017-10-08 10:36 ` Jonathan Cameron
2017-10-08 10:41 ` Jonathan Cameron
1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2017-10-08 10:36 UTC (permalink / raw)
To: Phil Reid; +Cc: knaack.h, lars, pmeerw, linux-iio
On Thu, 5 Oct 2017 10:37:46 +0800
Phil Reid <preid@electromag.com.au> wrote:
> This is done by calling a generic write attribute helper similar
> to the existing read attribute helper. Update write raw helper
> to use new write attribute function.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
> drivers/iio/inkern.c | 17 +++++++++++++++--
> include/linux/iio/consumer.h | 9 +++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 069defc..51c5c967 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -850,7 +850,9 @@ static int iio_channel_write(struct iio_channel *chan, int val, int val2,
> chan->channel, val, val2, info);
> }
>
> -int iio_write_channel_raw(struct iio_channel *chan, int val)
> +static int iio_write_channel_attribute(struct iio_channel *chan,
> + int val, int val2,
> + enum iio_chan_info_enum attribute)
> {
> int ret;
>
> @@ -860,14 +862,25 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
> goto err_unlock;
> }
>
> - ret = iio_channel_write(chan, val, 0, IIO_CHAN_INFO_RAW);
> + ret = iio_channel_write(chan, val, val2, attribute);
> err_unlock:
> mutex_unlock(&chan->indio_dev->info_exist_lock);
>
> return ret;
> }
> +
> +int iio_write_channel_raw(struct iio_channel *chan, int val)
> +{
> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_RAW);
> +}
> EXPORT_SYMBOL_GPL(iio_write_channel_raw);
>
> +int iio_write_channel_enable(struct iio_channel *chan, int val)
> +{
> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_ENABLE);
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_enable);
> +
> unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
> {
> const struct iio_chan_spec_ext_info *ext_info;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 5e347a9..4a5a90d 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -226,6 +226,15 @@ int iio_read_channel_raw(struct iio_channel *chan,
> int iio_write_channel_raw(struct iio_channel *chan, int val);
>
> /**
> + * iio_write_channel_enable() - enable a given channel
> + * @chan: The channel being queried.
> + * @val: Value being written.
> + *
> + * Enable / disable the channel.
> + */
> +int iio_write_channel_enable(struct iio_channel *chan, int val);
> +
> +/**
> * iio_read_max_channel_raw() - read maximum available raw value from a given
> * channel, i.e. the maximum possible value.
> * @chan: The channel being queried.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] iio: inkern: add helper to enable a channel
2017-10-05 2:37 [PATCH 1/1] iio: inkern: add helper to enable a channel Phil Reid
2017-10-08 10:36 ` Jonathan Cameron
@ 2017-10-08 10:41 ` Jonathan Cameron
2017-10-16 1:50 ` Phil Reid
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2017-10-08 10:41 UTC (permalink / raw)
To: Phil Reid; +Cc: knaack.h, lars, pmeerw, linux-iio
On Thu, 5 Oct 2017 10:37:46 +0800
Phil Reid <preid@electromag.com.au> wrote:
> This is done by calling a generic write attribute helper similar
> to the existing read attribute helper. Update write raw helper
> to use new write attribute function.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
Hi Phil,
In principle this is fine but there are a few subtle details that need
tidying up...
Also - normally a core ABI patch like this would only be applied as
part of a series that is using the change. Is such a series on its
way? We would want to see a usecase to evaluate the additional
interface.
Thanks,
Jonathan
> ---
> drivers/iio/inkern.c | 17 +++++++++++++++--
> include/linux/iio/consumer.h | 9 +++++++++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 069defc..51c5c967 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -850,7 +850,9 @@ static int iio_channel_write(struct iio_channel *chan, int val, int val2,
> chan->channel, val, val2, info);
> }
>
> -int iio_write_channel_raw(struct iio_channel *chan, int val)
> +static int iio_write_channel_attribute(struct iio_channel *chan,
> + int val, int val2,
> + enum iio_chan_info_enum attribute)
Hmm. Val2 only has meaning if we know the type of the write value
- i.e. how the driver is going to interpret val and val2. I would
suggest that we also need a function to allow the consumer driver to access
that information.
In this patch you aren't actually using val2 anyway so I'm not sure why
this is here.
> {
> int ret;
>
> @@ -860,14 +862,25 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
> goto err_unlock;
> }
>
> - ret = iio_channel_write(chan, val, 0, IIO_CHAN_INFO_RAW);
> + ret = iio_channel_write(chan, val, val2, attribute);
> err_unlock:
> mutex_unlock(&chan->indio_dev->info_exist_lock);
>
> return ret;
> }
> +
> +int iio_write_channel_raw(struct iio_channel *chan, int val)
> +{
> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_RAW);
> +}
> EXPORT_SYMBOL_GPL(iio_write_channel_raw);
>
> +int iio_write_channel_enable(struct iio_channel *chan, int val)
> +{
> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_ENABLE);
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_enable);
> +
> unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
> {
> const struct iio_chan_spec_ext_info *ext_info;
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 5e347a9..4a5a90d 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -226,6 +226,15 @@ int iio_read_channel_raw(struct iio_channel *chan,
> int iio_write_channel_raw(struct iio_channel *chan, int val);
>
> /**
> + * iio_write_channel_enable() - enable a given channel
> + * @chan: The channel being queried.
> + * @val: Value being written.
> + *
> + * Enable / disable the channel.
> + */
> +int iio_write_channel_enable(struct iio_channel *chan, int val);
> +
> +/**
> * iio_read_max_channel_raw() - read maximum available raw value from a given
> * channel, i.e. the maximum possible value.
> * @chan: The channel being queried.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] iio: inkern: add helper to enable a channel
2017-10-08 10:41 ` Jonathan Cameron
@ 2017-10-16 1:50 ` Phil Reid
2017-10-21 16:39 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Phil Reid @ 2017-10-16 1:50 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: knaack.h, lars, pmeerw, linux-iio
On 8/10/2017 18:41, Jonathan Cameron wrote:
> On Thu, 5 Oct 2017 10:37:46 +0800
> Phil Reid <preid@electromag.com.au> wrote:
>
>> This is done by calling a generic write attribute helper similar
>> to the existing read attribute helper. Update write raw helper
>> to use new write attribute function.
>>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>
> Hi Phil,
>
> In principle this is fine but there are a few subtle details that need
> tidying up...
>
> Also - normally a core ABI patch like this would only be applied as
> part of a series that is using the change. Is such a series on its
> way? We would want to see a usecase to evaluate the additional
> interface.
G'day Jonathan,
Not really. The hardware for the driver is custom fpga core and external hardware device.
No reason it can't be up-streamed but I doubt it'd be of use to anyone else and I haven't
made much effort to write the driver to meet the kernel coding standards.
Driver is basically a posix clock using an OCXO that is tuned using a DAC.
Don't know if there's any interest in such a thing.
>
> Thanks,
>
> Jonathan
>> ---
>> drivers/iio/inkern.c | 17 +++++++++++++++--
>> include/linux/iio/consumer.h | 9 +++++++++
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index 069defc..51c5c967 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -850,7 +850,9 @@ static int iio_channel_write(struct iio_channel *chan, int val, int val2,
>> chan->channel, val, val2, info);
>> }
>>
>> -int iio_write_channel_raw(struct iio_channel *chan, int val)
>> +static int iio_write_channel_attribute(struct iio_channel *chan,
>> + int val, int val2,
>> + enum iio_chan_info_enum attribute)
>
> Hmm. Val2 only has meaning if we know the type of the write value
> - i.e. how the driver is going to interpret val and val2. I would
> suggest that we also need a function to allow the consumer driver to access
> that information.
>
> In this patch you aren't actually using val2 anyway so I'm not sure why
> this is here.
Yes this is true but the intention was that it is a mutex locked wrapper for the existing
iio_channel_write function that has val / val2. So my thought was including it would make it
easy to add other helpers in the future that use val2.
>
>> {
>> int ret;
>>
>> @@ -860,14 +862,25 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
>> goto err_unlock;
>> }
>>
>> - ret = iio_channel_write(chan, val, 0, IIO_CHAN_INFO_RAW);
>> + ret = iio_channel_write(chan, val, val2, attribute);
>> err_unlock:
>> mutex_unlock(&chan->indio_dev->info_exist_lock);
>>
>> return ret;
>> }
>> +
>> +int iio_write_channel_raw(struct iio_channel *chan, int val)
>> +{
>> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_RAW);
>> +}
>> EXPORT_SYMBOL_GPL(iio_write_channel_raw);
>>
>> +int iio_write_channel_enable(struct iio_channel *chan, int val)
>> +{
>> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_ENABLE);
>> +}
>> +EXPORT_SYMBOL_GPL(iio_write_channel_enable);
>> +
>> unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
>> {
>> const struct iio_chan_spec_ext_info *ext_info;
>> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
>> index 5e347a9..4a5a90d 100644
>> --- a/include/linux/iio/consumer.h
>> +++ b/include/linux/iio/consumer.h
>> @@ -226,6 +226,15 @@ int iio_read_channel_raw(struct iio_channel *chan,
>> int iio_write_channel_raw(struct iio_channel *chan, int val);
>>
>> /**
>> + * iio_write_channel_enable() - enable a given channel
>> + * @chan: The channel being queried.
>> + * @val: Value being written.
>> + *
>> + * Enable / disable the channel.
>> + */
>> +int iio_write_channel_enable(struct iio_channel *chan, int val);
>> +
>> +/**
>> * iio_read_max_channel_raw() - read maximum available raw value from a given
>> * channel, i.e. the maximum possible value.
>> * @chan: The channel being queried.
>
>
>
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] iio: inkern: add helper to enable a channel
2017-10-16 1:50 ` Phil Reid
@ 2017-10-21 16:39 ` Jonathan Cameron
2017-10-25 3:43 ` Phil Reid
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2017-10-21 16:39 UTC (permalink / raw)
To: Phil Reid; +Cc: knaack.h, lars, pmeerw, linux-iio
On Mon, 16 Oct 2017 09:50:34 +0800
Phil Reid <preid@electromag.com.au> wrote:
> On 8/10/2017 18:41, Jonathan Cameron wrote:
> > On Thu, 5 Oct 2017 10:37:46 +0800
> > Phil Reid <preid@electromag.com.au> wrote:
> >
> >> This is done by calling a generic write attribute helper similar
> >> to the existing read attribute helper. Update write raw helper
> >> to use new write attribute function.
> >>
> >> Signed-off-by: Phil Reid <preid@electromag.com.au>
> >
> > Hi Phil,
> >
> > In principle this is fine but there are a few subtle details that need
> > tidying up...
> >
> > Also - normally a core ABI patch like this would only be applied as
> > part of a series that is using the change. Is such a series on its
> > way? We would want to see a usecase to evaluate the additional
> > interface.
>
> G'day Jonathan,
>
> Not really. The hardware for the driver is custom fpga core and external hardware device.
> No reason it can't be up-streamed but I doubt it'd be of use to anyone else and I haven't
> made much effort to write the driver to meet the kernel coding standards.
>
> Driver is basically a posix clock using an OCXO that is tuned using a DAC.
> Don't know if there's any interest in such a thing.
Yeah - that one is a little random ;)
Hohum. Lets make an exception - with the tidy ups below I'll take this
even though we don't have an in kernel user for now...
Jonathan
>
>
> >
> > Thanks,
> >
> > Jonathan
> >> ---
> >> drivers/iio/inkern.c | 17 +++++++++++++++--
> >> include/linux/iio/consumer.h | 9 +++++++++
> >> 2 files changed, 24 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> >> index 069defc..51c5c967 100644
> >> --- a/drivers/iio/inkern.c
> >> +++ b/drivers/iio/inkern.c
> >> @@ -850,7 +850,9 @@ static int iio_channel_write(struct iio_channel *chan, int val, int val2,
> >> chan->channel, val, val2, info);
> >> }
> >>
> >> -int iio_write_channel_raw(struct iio_channel *chan, int val)
> >> +static int iio_write_channel_attribute(struct iio_channel *chan,
> >> + int val, int val2,
> >> + enum iio_chan_info_enum attribute)
> >
> > Hmm. Val2 only has meaning if we know the type of the write value
> > - i.e. how the driver is going to interpret val and val2. I would
> > suggest that we also need a function to allow the consumer driver to access
> > that information.
> >
> > In this patch you aren't actually using val2 anyway so I'm not sure why
> > this is here.
>
> Yes this is true but the intention was that it is a mutex locked wrapper for the existing
> iio_channel_write function that has val / val2. So my thought was including it would make it
> easy to add other helpers in the future that use val2.
>
> >
> >> {
> >> int ret;
> >>
> >> @@ -860,14 +862,25 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
> >> goto err_unlock;
> >> }
> >>
> >> - ret = iio_channel_write(chan, val, 0, IIO_CHAN_INFO_RAW);
> >> + ret = iio_channel_write(chan, val, val2, attribute);
> >> err_unlock:
> >> mutex_unlock(&chan->indio_dev->info_exist_lock);
> >>
> >> return ret;
> >> }
> >> +
> >> +int iio_write_channel_raw(struct iio_channel *chan, int val)
> >> +{
> >> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_RAW);
> >> +}
> >> EXPORT_SYMBOL_GPL(iio_write_channel_raw);
> >>
> >> +int iio_write_channel_enable(struct iio_channel *chan, int val)
> >> +{
> >> + return iio_write_channel_attribute(chan, val, 0, IIO_CHAN_INFO_ENABLE);
> >> +}
> >> +EXPORT_SYMBOL_GPL(iio_write_channel_enable);
> >> +
> >> unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
> >> {
> >> const struct iio_chan_spec_ext_info *ext_info;
> >> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> >> index 5e347a9..4a5a90d 100644
> >> --- a/include/linux/iio/consumer.h
> >> +++ b/include/linux/iio/consumer.h
> >> @@ -226,6 +226,15 @@ int iio_read_channel_raw(struct iio_channel *chan,
> >> int iio_write_channel_raw(struct iio_channel *chan, int val);
> >>
> >> /**
> >> + * iio_write_channel_enable() - enable a given channel
> >> + * @chan: The channel being queried.
> >> + * @val: Value being written.
> >> + *
> >> + * Enable / disable the channel.
> >> + */
> >> +int iio_write_channel_enable(struct iio_channel *chan, int val);
> >> +
> >> +/**
> >> * iio_read_max_channel_raw() - read maximum available raw value from a given
> >> * channel, i.e. the maximum possible value.
> >> * @chan: The channel being queried.
> >
> >
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] iio: inkern: add helper to enable a channel
2017-10-21 16:39 ` Jonathan Cameron
@ 2017-10-25 3:43 ` Phil Reid
0 siblings, 0 replies; 6+ messages in thread
From: Phil Reid @ 2017-10-25 3:43 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: knaack.h, lars, pmeerw, linux-iio
On 22/10/2017 00:39, Jonathan Cameron wrote:
> On Mon, 16 Oct 2017 09:50:34 +0800
> Phil Reid <preid@electromag.com.au> wrote:
>
>> On 8/10/2017 18:41, Jonathan Cameron wrote:
>>> On Thu, 5 Oct 2017 10:37:46 +0800
>>> Phil Reid <preid@electromag.com.au> wrote:
>>>
>>>> This is done by calling a generic write attribute helper similar
>>>> to the existing read attribute helper. Update write raw helper
>>>> to use new write attribute function.
>>>>
>>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>>>
>>> Hi Phil,
>>>
>>> In principle this is fine but there are a few subtle details that need
>>> tidying up...
>>>
>>> Also - normally a core ABI patch like this would only be applied as
>>> part of a series that is using the change. Is such a series on its
>>> way? We would want to see a usecase to evaluate the additional
>>> interface.
>>
>> G'day Jonathan,
>>
>> Not really. The hardware for the driver is custom fpga core and external hardware device.
>> No reason it can't be up-streamed but I doubt it'd be of use to anyone else and I haven't
>> made much effort to write the driver to meet the kernel coding standards.
>>
>> Driver is basically a posix clock using an OCXO that is tuned using a DAC.
>> Don't know if there's any interest in such a thing.
> Yeah - that one is a little random ;)
>
> Hohum. Lets make an exception - with the tidy ups below I'll take this
> even though we don't have an in kernel user for now...
>
Thanks, I'll send v2 soon.
--
Regards
Phil Reid
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-25 3:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 2:37 [PATCH 1/1] iio: inkern: add helper to enable a channel Phil Reid
2017-10-08 10:36 ` Jonathan Cameron
2017-10-08 10:41 ` Jonathan Cameron
2017-10-16 1:50 ` Phil Reid
2017-10-21 16:39 ` Jonathan Cameron
2017-10-25 3:43 ` Phil Reid
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.