| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4c4c70d9-88d8-433d-a8d0-804b3c718a2b@baylibre.com>
Date: Thu, 8 May 2025 11:18:13 -0500
From: David Lechner <dlechner@...libre.com>
To: Brajesh Patil <brajeshpatil11@...il.com>, jic23@...nel.org,
lars@...afoo.de
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
marcelo.schmitt1@...il.com
Subject: Re: [PATCH v2 4/5] iio: magnetometer: qmc5883l: add extended sysfs
attributes and configuration options
On 5/8/25 7:08 AM, Brajesh Patil wrote:
> Signed-off-by: Brajesh Patil <brajeshpatil11@...il.com>
> ---
...
> +
> +static const struct iio_chan_spec_ext_info qmc5883l_ext_info[] = {
> + IIO_ENUM("mode", IIO_SHARED_BY_TYPE, &qmc5883l_mode_enum),
> + IIO_ENUM_AVAILABLE("mode", IIO_SHARED_BY_TYPE, &qmc5883l_mode_enum),
> + IIO_ENUM("oversampling_ratio", IIO_SHARED_BY_TYPE, &qmc5883l_osr_enum),
There is already IIO_CHAN_INFO_OVERSAMPLING_RATIO so we don't need to make this
a custom attribute.
> + IIO_ENUM_AVAILABLE("oversampling_ratio", IIO_SHARED_BY_TYPE, &qmc5883l_osr_enum),
> + { }
> +};
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(qmc5883l_show_odr_avail);
> +static IIO_DEVICE_ATTR(scale_available, 0444, qmc5883l_show_scale_avail, NULL, 0);
> +static IIO_DEVICE_ATTR(data_ready, 0444, qmc5883l_show_status, NULL, 0);
> +static IIO_DEVICE_ATTR(overflow, 0444, qmc5883l_show_status, NULL, 0);
As far as I can tell, mode, data_ready and overflow are not standard attributes.
I don't see them in Documentation/ABI/testing/sysfs-bus-iio*. So if these really
are needed, we will need a justification of why these don't fit into any
existing attributes.
In the previous patch standby/continuous and data ready were handled internally
already so I don't think need those.
Overflow sounds like it could possibly be an event, but I didn't really look in
to it.
> +
> +static ssize_t qmc5883l_show_odr_avail(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "10 50 100 200\n");
> +}
> +
> +static ssize_t qmc5883l_show_scale_avail(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "2 8\n");
> +}
> +
> +static ssize_t qmc5883l_show_status(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(data->regmap, QMC5883L_STATUS_REG, &val);
> + if (ret < 0)
> + return ret;
> +
> + if (attr == (struct device_attribute *)&iio_dev_attr_data_ready.dev_attr.attr)
> + return sprintf(buf, "%d\n", !!(val & QMC5883L_DRDY));
> + else if (attr == (struct device_attribute *)&iio_dev_attr_overflow.dev_attr.attr)
> + return sprintf(buf, "%d\n", !!(val & QMC5883L_OVL));
> +
> + return -EINVAL;
> +}
> +
> static int qmc5883l_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int *val, int *val2, long mask)
> {
> @@ -275,6 +483,54 @@ static int qmc5883l_read_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +static int qmc5883l_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + int odr, range;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
Since the format for this is set to IIO_VAL_INT, val2 will always be 0 and can
be dropped. Then we can turn this into a switch statemnt.
> + if (val == 10 && val2 == 0)
> + odr = QMC5883L_ODR_10HZ;
> + else if (val == 50 && val2 == 0)
> + odr = QMC5883L_ODR_50HZ;
> + else if (val == 100 && val2 == 0)
> + odr = QMC5883L_ODR_100HZ;
> + else if (val == 200 && val2 == 0)
> + odr = QMC5883L_ODR_200HZ;
> + else
> + return -EINVAL;
> +
> + return qmc5883l_set_odr(data, odr);
> + case IIO_CHAN_INFO_SCALE:
If scale is always an integer value, then we should set that in
qmc5883l_write_raw_get_fmt() and we don't have to check val2 here either.
> + if (val == 2 && val2 == 0)
> + range = QMC5883L_RNG_2G;
> + else if (val == 8 && val2 == 0)
> + range = QMC5883L_RNG_8G;
> + else
> + return -EINVAL;
> +
> + return qmc5883l_set_rng(data, range << QMC5883L_RNG_SHIFT);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int qmc5883l_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + return IIO_VAL_INT_PLUS_NANO;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> static irqreturn_t qmc5883l_trigger_handler(int irq, void *p)
> {
> struct iio_poll_func *pf = p;
> @@ -321,6 +577,7 @@ static irqreturn_t qmc5883l_trigger_handler(int irq, void *p)
> .storagebits = 16, \
> .endianness = IIO_LE, \
> }, \
> + .ext_info = qmc5883l_ext_info, \
> }
>
> static const struct iio_chan_spec qmc5883l_channels[] = {
> @@ -337,6 +594,18 @@ static const struct iio_chan_spec qmc5883l_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> +static struct attribute *qmc5883l_attributes[] = {
> + &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> + &iio_dev_attr_scale_available.dev_attr.attr,
> + &iio_dev_attr_data_ready.dev_attr.attr,
> + &iio_dev_attr_overflow.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group qmc5883l_attribute_group = {
> + .attrs = qmc5883l_attributes,
> +};
> +
> static int qmc5883l_init(struct qmc5883l_data *data)
> {
> int ret;
> @@ -382,7 +651,10 @@ static int qmc5883l_init(struct qmc5883l_data *data)
> }
>
> static const struct iio_info qmc5883l_info = {
> + .attrs = &qmc5883l_attribute_group,
> .read_raw = &qmc5883l_read_raw,
We can implement .read_avail here to avoid needing custom _available attributes.
> + .write_raw = &qmc5883l_write_raw,
> + .write_raw_get_fmt = &qmc5883l_write_raw_get_fmt,
> };
>
> static const unsigned long qmc5883l_scan_masks[] = {0x7, 0};
> --
> 2.39.5
>
Powered by blists - more mailing lists