On Tue, 7 May 2024 14:02:07 -0500
David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> This adds new fields to the iio_channel structure to support multiple
> scan types per channel. This is useful for devices that support multiple
> resolution modes or other modes that require different data formats of
> the raw data.
>
> To make use of this, drivers can still use the old scan_type field for
> the "default" scan type and use the new scan_type_ext field for any
> additional scan types.
Comment inline says that you should commit scan_type if scan_type_ext
is provided. That makes sense to me rather that a default no one reads.
The example that follows in patch 4 uses both the scan_type and
the scan_type_ext which is even more confusing.
> And they must implement the new callback
> get_current_scan_type() to return the current scan type based on the
> current state of the device.
>
> The buffer code is the only code in the IIO core code that is using the
> scan_type field. This patch updates the buffer code to use the new
> iio_channel_validate_scan_type() function to ensure it is returning the
> correct scan type for the current state of the device when reading the
> sysfs attributes. The buffer validation code is also update to validate
> any additional scan types that are set in the scan_type_ext field. Part
> of that code is refactored to a new function to avoid duplication.
>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
> ---
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 19de573a944a..66f0b4c68f53 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -205,6 +205,9 @@ struct iio_scan_type {
> * @scan_index: Monotonic index to give ordering in scans when read
> * from a buffer.
> * @scan_type: struct describing the scan type
> + * @ext_scan_type: Used in rare cases where there is more than one scan
> + * format for a channel. When this is used, omit scan_type.
Here is the disagreement with the patch description.
> + * @num_ext_scan_type: Number of elements in ext_scan_type.
> * @info_mask_separate: What information is to be exported that is specific to
> * this channel.
> * @info_mask_separate_available: What availability information is to be
> @@ -256,6 +259,8 @@ struct iio_chan_spec {
> unsigned long address;
> int scan_index;
> struct iio_scan_type scan_type;
> + const struct iio_scan_type *ext_scan_type;
> + unsigned int num_ext_scan_type;
Let's make it explicit that you can't do both.
union {
struct iio_scan_type scan_type;
struct {
const struct iio_scan_type *ext_scan_type;
unsigned int num_ext_scan_type;
};
};
should work for that I think.
However this is I think only used for validation. If that's the case
do we care about values not in use? Can we move the validation to
be runtime if the get_current_scan_type() callback is used.
> long info_mask_separate;
> long info_mask_separate_available;
> long info_mask_shared_by_type;
> @@ -435,6 +440,9 @@ struct iio_trigger; /* forward declaration */
> * for better event identification.
> * @validate_trigger: function to validate the trigger when the
> * current trigger gets changed.
> + * @get_current_scan_type: must be implemented by drivers that use ext_scan_type
> + * in the channel spec to return the currently active scan
> + * type based on the current state of the device.
> * @update_scan_mode: function to configure device and scan buffer when
> * channels have changed
> * @debugfs_reg_access: function to read or write register value of device
> @@ -519,6 +527,9 @@ struct iio_info {
>
> int (*validate_trigger)(struct iio_dev *indio_dev,
> struct iio_trigger *trig);
> + const struct iio_scan_type *(*get_current_scan_type)(
> + const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan);
> int (*update_scan_mode)(struct iio_dev *indio_dev,
> const unsigned long *scan_mask);
> int (*debugfs_reg_access)(struct iio_dev *indio_dev,
> @@ -804,6 +815,28 @@ static inline bool iio_read_acpi_mount_matrix(struct device *dev,
> }
> #endif
>
> +/**
> + * iio_get_current_scan_type - Get the current scan type for a channel
> + * @indio_dev: the IIO device to get the scan type for
> + * @chan: the channel to get the scan type for
> + *
> + * Most devices only have one scan type per channel and can just access it
> + * directly without calling this function. Core IIO code and drivers that
> + * implement ext_scan_type in the channel spec should use this function to
> + * get the current scan type for a channel.
> + *
> + * Returns: the current scan type for the channel
> + */
> +static inline const struct iio_scan_type *iio_get_current_scan_type(
> + const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + if (indio_dev->info->get_current_scan_type)
> + return indio_dev->info->get_current_scan_type(indio_dev, chan);
> +
> + return &chan->scan_type;
> +}
> +
> ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
>
> int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer,
>
