Re: [PATCH 1/1] iio: inkern: add helper to enable a channel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/10/2017 18:41, Jonathan Cameron wrote:
On Thu,  5 Oct 2017 10:37:46 +0800
Phil Reid <preid@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux