From a8b6d6708bb682108d8c899bc0cb7873240daf8a Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 7 Feb 2022 15:38:28 +0100 Subject: iio: core: Enhance the kernel doc of modes and currentmodes iio_dev entries Let's provide more details about these two variables because their understanding may not be straightforward for someone not used to the IIO subsystem internal logic. The different modes will soon be also be more documented for the same reason. Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220207143840.707510-2-miquel.raynal@bootlin.com Signed-off-by: Jonathan Cameron --- include/linux/iio/iio.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'include/linux/iio/iio.h') diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index faf00f2c0be6..f191b80466cd 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -488,8 +488,15 @@ struct iio_buffer_setup_ops { /** * struct iio_dev - industrial I/O device - * @modes: [DRIVER] operating modes supported by device - * @currentmode: [INTERN] current operating mode + * @modes: [DRIVER] bitmask listing all the operating modes + * supported by the IIO device. This list should be + * initialized before registering the IIO device. It can + * also be filed up by the IIO core, as a result of + * enabling particular features in the driver + * (see iio_triggered_event_setup()). + * @currentmode: [INTERN] operating mode currently in use, may be + * eventually checked by device drivers but should be + * considered read-only as this is a core internal bit * @dev: [DRIVER] device structure, should be assigned a parent * and owner * @buffer: [DRIVER] any buffer present -- cgit v1.2.3 From 2f53b4adfede66f1bc1c8bb7efd7ced2bad1191a Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 7 Feb 2022 15:38:36 +0100 Subject: iio: Un-inline iio_buffer_enabled() As we are going to hide the currentmode inside the opaque structure, this helper would soon need to call a non-inline function which would simply drop the benefit of having the helper defined inline in a header. One alternative is to move this helper in the core as there is no more interest in defining it inline in a header. We will pay the minor cost either way. Let's do like the iio_device_id() helper which also refers to the opaque structure and gets defined in the core. Suggested-by: Jonathan Cameron Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220207143840.707510-10-miquel.raynal@bootlin.com Signed-off-by: Jonathan Cameron --- include/linux/iio/iio.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'include/linux/iio/iio.h') diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index f191b80466cd..faabb852128a 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -550,6 +550,7 @@ struct iio_dev { }; int iio_device_id(struct iio_dev *indio_dev); +bool iio_buffer_enabled(struct iio_dev *indio_dev); const struct iio_chan_spec *iio_find_channel_from_si(struct iio_dev *indio_dev, int si); @@ -679,16 +680,6 @@ struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv); __printf(2, 3) struct iio_trigger *devm_iio_trigger_alloc(struct device *parent, const char *fmt, ...); -/** - * iio_buffer_enabled() - helper function to test if the buffer is enabled - * @indio_dev: IIO device structure for device - **/ -static inline bool iio_buffer_enabled(struct iio_dev *indio_dev) -{ - return indio_dev->currentmode - & (INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_HARDWARE | - INDIO_BUFFER_SOFTWARE); -} /** * iio_get_debugfs_dentry() - helper function to get the debugfs_dentry -- cgit v1.2.3 From 8c576f87ad7eb639b8bd4472a9bb830e0696dda5 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 7 Feb 2022 15:38:37 +0100 Subject: iio: core: Hide read accesses to iio_dev->currentmode In order to later move this variable within the opaque structure, let's create a helper for accessing it in read-only mode. This helper will be exposed to device drivers and kept accessible for the few that could need it. The write access to this variable however should be fully reserved to the core so in a second step we will hide this variable into the opaque structure. Cc: Eugen Hristev Cc: Nicolas Ferre Cc: Alexandre Belloni Cc: Ludovic Desroches Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220207143840.707510-11-miquel.raynal@bootlin.com Signed-off-by: Jonathan Cameron --- include/linux/iio/iio.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux/iio/iio.h') diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index faabb852128a..31098ffa7dc9 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -550,6 +550,7 @@ struct iio_dev { }; int iio_device_id(struct iio_dev *indio_dev); +int iio_device_get_current_mode(struct iio_dev *indio_dev); bool iio_buffer_enabled(struct iio_dev *indio_dev); const struct iio_chan_spec -- cgit v1.2.3 From 51570c9d4b3a678f77a50ac139f67290e946ec86 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 7 Feb 2022 15:38:38 +0100 Subject: iio: core: Move the currentmode entry to the opaque structure This entry should, under no situation, be modified by device drivers. Now that we have limited its read access to device drivers really needing it and did so through a dedicated helper, we can easily move this variable to the opaque structure in order to prevent any further modification from non-authorized code (out of the core, basically). Signed-off-by: Miquel Raynal Reviewed-by: Alexandru Ardelean Link: https://lore.kernel.org/r/20220207143840.707510-12-miquel.raynal@bootlin.com Signed-off-by: Jonathan Cameron --- include/linux/iio/iio.h | 4 ---- 1 file changed, 4 deletions(-) (limited to 'include/linux/iio/iio.h') diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index 31098ffa7dc9..85cb924debd9 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -494,9 +494,6 @@ struct iio_buffer_setup_ops { * also be filed up by the IIO core, as a result of * enabling particular features in the driver * (see iio_triggered_event_setup()). - * @currentmode: [INTERN] operating mode currently in use, may be - * eventually checked by device drivers but should be - * considered read-only as this is a core internal bit * @dev: [DRIVER] device structure, should be assigned a parent * and owner * @buffer: [DRIVER] any buffer present @@ -523,7 +520,6 @@ struct iio_buffer_setup_ops { */ struct iio_dev { int modes; - int currentmode; struct device dev; struct iio_buffer *buffer; -- cgit v1.2.3 From ac3e62f51b3fbda78a44fff62ece8f11d8f08a25 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 7 Feb 2022 15:38:40 +0100 Subject: iio: core: Clarify the modes As part of a previous discussion with Jonathan Cameron [1], it appeared necessary to clarify the meaning of each mode so that new developers could understand better what they should use or not use and when. The idea of renaming these modes as been let aside because naming is a big deal and requires a lot of thinking. So for now let's focus on correctly explaining what each mode implies. [1] https://lore.kernel.org/linux-iio/20210930165510.2295e6c4@jic23-huawei/ Suggested-by: Jonathan Cameron Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220207143840.707510-14-miquel.raynal@bootlin.com Signed-off-by: Jonathan Cameron --- include/linux/iio/iio.h | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) (limited to 'include/linux/iio/iio.h') diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index 85cb924debd9..233d2e6b7721 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -315,7 +315,54 @@ static inline bool iio_channel_has_available(const struct iio_chan_spec *chan, s64 iio_get_time_ns(const struct iio_dev *indio_dev); unsigned int iio_get_time_res(const struct iio_dev *indio_dev); -/* Device operating modes */ +/* + * Device operating modes + * @INDIO_DIRECT_MODE: There is an access to either: + * a) The last single value available for devices that do not provide + * on-demand reads. + * b) A new value after performing an on-demand read otherwise. + * On most devices, this is a single-shot read. On some devices with data + * streams without an 'on-demand' function, this might also be the 'last value' + * feature. Above all, this mode internally means that we are not in any of the + * other modes, and sysfs reads should work. + * Device drivers should inform the core if they support this mode. + * @INDIO_BUFFER_TRIGGERED: Common mode when dealing with kfifo buffers. + * It indicates that an explicit trigger is required. This requests the core to + * attach a poll function when enabling the buffer, which is indicated by the + * _TRIGGERED suffix. + * The core will ensure this mode is set when registering a triggered buffer + * with iio_triggered_buffer_setup(). + * @INDIO_BUFFER_SOFTWARE: Another kfifo buffer mode, but not event triggered. + * No poll function can be attached because there is no triggered infrastructure + * we can use to cause capture. There is a kfifo that the driver will fill, but + * not "only one scan at a time". Typically, hardware will have a buffer that + * can hold multiple scans. Software may read one or more scans at a single time + * and push the available data to a Kfifo. This means the core will not attach + * any poll function when enabling the buffer. + * The core will ensure this mode is set when registering a simple kfifo buffer + * with devm_iio_kfifo_buffer_setup(). + * @INDIO_BUFFER_HARDWARE: For specific hardware, if unsure do not use this mode. + * Same as above but this time the buffer is not a kfifo where we have direct + * access to the data. Instead, the consumer driver must access the data through + * non software visible channels (or DMA when there is no demux possible in + * software) + * The core will ensure this mode is set when registering a dmaengine buffer + * with devm_iio_dmaengine_buffer_setup(). + * @INDIO_EVENT_TRIGGERED: Very unusual mode. + * Triggers usually refer to an external event which will start data capture. + * Here it is kind of the opposite as, a particular state of the data might + * produce an event which can be considered as an event. We don't necessarily + * have access to the data itself, but to the event produced. For example, this + * can be a threshold detector. The internal path of this mode is very close to + * the INDIO_BUFFER_TRIGGERED mode. + * The core will ensure this mode is set when registering a triggered event. + * @INDIO_HARDWARE_TRIGGERED: Very unusual mode. + * Here, triggers can result in data capture and can be routed to multiple + * hardware components, which make them close to regular triggers in the way + * they must be managed by the core, but without the entire interrupts/poll + * functions burden. Interrupts are irrelevant as the data flow is hardware + * mediated and distributed. + */ #define INDIO_DIRECT_MODE 0x01 #define INDIO_BUFFER_TRIGGERED 0x02 #define INDIO_BUFFER_SOFTWARE 0x04 -- cgit v1.2.3