| 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: <20230930164732.34e62b20@jic23-huawei>
Date: Sat, 30 Sep 2023 16:47:32 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-staging@...ts.linux.dev,
David Lechner <david@...hnology.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Michael Hennerich <Michael.Hennerich@...log.com>,
Nuno Sá <nuno.sa@...log.com>,
Axel Haslam <ahaslam@...libre.com>,
Philip Molloy <pmolloy@...libre.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 25/27] staging: iio: resolver: ad2s1210: rename DOS
reset min/max attrs
On Fri, 29 Sep 2023 12:23:30 -0500
David Lechner <dlechner@...libre.com> wrote:
> From: David Lechner <david@...hnology.com>
>
> From: David Lechner <dlechner@...libre.com>
>
> The AD2S1210 has a programmable threshold for the degradation of signal
> (DOS) mismatch fault. This fault is triggered when the difference in
> amplitude between the sine and cosine inputs exceeds the threshold.
>
> The DOS reset min/max registers on the chip provide initial values
> for internal tracking of the min/max of the monitor signal after the
> fault register is cleared.
>
> This patch converts the custom device DOS reset min/max threshold
> attributes custom event attributes on the monitor signal channel.
>
> The attributes now use millivolts instead of the raw register value in
> accordance with the IIO ABI.
>
> Emitting the event will be implemented in a later patch.
>
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
>
> v3 changes: This is a new patch in v3
>
> .../Documentation/sysfs-bus-iio-resolver-ad2s1210 | 27 ++++++
> drivers/staging/iio/resolver/ad2s1210.c | 99 ++++++++++++----------
> 2 files changed, 82 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210 b/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210
> new file mode 100644
> index 000000000000..ea75881b0c77
> --- /dev/null
> +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-resolver-ad2s1210
> @@ -0,0 +1,27 @@
> +What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0-altvoltage1_thresh_rising_reset_max
Ah. So these are differential. But the mismatch channel value isn't?
I also got the format wrong for differential channels. Oops. Should
be the in_altvoltage0-altvoltage1 format for the previous suggestion
to change that channel type to differential.
This looks fine to me as new ABI.
Jonathan
> +KernelVersion: 6.7
> +Contact: linux-iio@...r.kernel.org
> +Description:
> + Reading returns the current Degradation of Signal Reset Maximum
> + Threshold value in millivolts. Writing sets the value.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0-altvoltage1_thresh_rising_reset_max_available
> +KernelVersion: 6.7
> +Contact: linux-iio@...r.kernel.org
> +Description:
> + Reading returns the allowable voltage range for
> + in_altvoltage0-altvoltage1_thresh_rising_reset_max.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0-altvoltage1_thresh_rising_reset_min
> +KernelVersion: 6.7
> +Contact: linux-iio@...r.kernel.org
> +Description:
> + Reading returns the current Degradation of Signal Reset Minimum
> + Threshold value in millivolts. Writing sets the value.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/events/in_altvoltage0-altvoltage1_thresh_rising_reset_min_available
> +KernelVersion: 6.7
> +Contact: linux-iio@...r.kernel.org
> +Description:
> + Reading returns the allowable voltage range for
> + in_altvoltage0-altvoltage1_thresh_rising_reset_min.
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index aa14edbe8a77..e1c95ec73545 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -283,41 +283,6 @@ static ssize_t ad2s1210_clear_fault(struct device *dev,
> return ret < 0 ? ret : len;
> }
>
> -static ssize_t ad2s1210_show_reg(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
> - struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
> - unsigned int value;
> - int ret;
> -
> - mutex_lock(&st->lock);
> - ret = regmap_read(st->regmap, iattr->address, &value);
> - mutex_unlock(&st->lock);
> -
> - return ret < 0 ? ret : sprintf(buf, "%d\n", value);
> -}
> -
> -static ssize_t ad2s1210_store_reg(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t len)
> -{
> - struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
> - unsigned char data;
> - int ret;
> - struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
> -
> - ret = kstrtou8(buf, 10, &data);
> - if (ret)
> - return -EINVAL;
> -
> - mutex_lock(&st->lock);
> - ret = regmap_write(st->regmap, iattr->address, data);
> - mutex_unlock(&st->lock);
> - return ret < 0 ? ret : len;
> -}
> -
> static int ad2s1210_single_conversion(struct ad2s1210_state *st,
> struct iio_chan_spec const *chan,
> int *val)
> @@ -743,13 +708,6 @@ static int ad2s1210_write_raw(struct iio_dev *indio_dev,
> static IIO_DEVICE_ATTR(fault, 0644,
> ad2s1210_show_fault, ad2s1210_clear_fault, 0);
>
> -static IIO_DEVICE_ATTR(dos_rst_max_thrd, 0644,
> - ad2s1210_show_reg, ad2s1210_store_reg,
> - AD2S1210_REG_DOS_RST_MAX_THRD);
> -static IIO_DEVICE_ATTR(dos_rst_min_thrd, 0644,
> - ad2s1210_show_reg, ad2s1210_store_reg,
> - AD2S1210_REG_DOS_RST_MIN_THRD);
> -
> static const struct iio_event_spec ad2s1210_position_event_spec[] = {
> {
> /* Tracking error exceeds LOT threshold fault. */
> @@ -867,8 +825,6 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
>
> static struct attribute *ad2s1210_attributes[] = {
> &iio_dev_attr_fault.dev_attr.attr,
> - &iio_dev_attr_dos_rst_max_thrd.dev_attr.attr,
> - &iio_dev_attr_dos_rst_min_thrd.dev_attr.attr,
> NULL,
> };
>
> @@ -876,6 +832,49 @@ static const struct attribute_group ad2s1210_attribute_group = {
> .attrs = ad2s1210_attributes,
> };
>
> +static ssize_t event_attr_voltage_reg_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
> + struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
> + unsigned int value;
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_read(st->regmap, iattr->address, &value);
> + mutex_unlock(&st->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + return sprintf(buf, "%d\n", value * THRESHOLD_MILLIVOLT_PER_LSB);
> +}
> +
> +static ssize_t event_attr_voltage_reg_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct ad2s1210_state *st = iio_priv(dev_to_iio_dev(dev));
> + struct iio_dev_attr *iattr = to_iio_dev_attr(attr);
> + u16 data;
> + int ret;
> +
> + ret = kstrtou16(buf, 10, &data);
> + if (ret)
> + return -EINVAL;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_write(st->regmap, iattr->address,
> + data / THRESHOLD_MILLIVOLT_PER_LSB);
> + mutex_unlock(&st->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + return len;
> +}
> +
> static ssize_t
> in_angl1_thresh_rising_value_available_show(struct device *dev,
> struct device_attribute *attr,
> @@ -906,6 +905,14 @@ IIO_CONST_ATTR(in_phase0_mag_value_available,
> IIO_CONST_ATTR(in_altvoltage0_thresh_falling_value_available, THRESHOLD_RANGE_STR);
> IIO_CONST_ATTR(in_altvoltage0_thresh_rising_value_available, THRESHOLD_RANGE_STR);
> IIO_CONST_ATTR(in_altvoltage0_mag_value_available, THRESHOLD_RANGE_STR);
> +IIO_DEVICE_ATTR(in_altvoltage0_mag_reset_max, 0644,
> + event_attr_voltage_reg_show, event_attr_voltage_reg_store,
> + AD2S1210_REG_DOS_RST_MAX_THRD);
> +IIO_CONST_ATTR(in_altvoltage0_mag_reset_max_available, THRESHOLD_RANGE_STR);
> +IIO_DEVICE_ATTR(in_altvoltage0_mag_reset_min, 0644,
> + event_attr_voltage_reg_show, event_attr_voltage_reg_store,
> + AD2S1210_REG_DOS_RST_MIN_THRD);
> +IIO_CONST_ATTR(in_altvoltage0_mag_reset_min_available, THRESHOLD_RANGE_STR);
> IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_value_available, 0);
> IIO_DEVICE_ATTR_RO(in_angl1_thresh_rising_hysteresis_available, 0);
>
> @@ -914,6 +921,10 @@ static struct attribute *ad2s1210_event_attributes[] = {
> &iio_const_attr_in_altvoltage0_thresh_falling_value_available.dev_attr.attr,
> &iio_const_attr_in_altvoltage0_thresh_rising_value_available.dev_attr.attr,
> &iio_const_attr_in_altvoltage0_mag_value_available.dev_attr.attr,
> + &iio_dev_attr_in_altvoltage0_mag_reset_max.dev_attr.attr,
> + &iio_const_attr_in_altvoltage0_mag_reset_max_available.dev_attr.attr,
> + &iio_dev_attr_in_altvoltage0_mag_reset_min.dev_attr.attr,
> + &iio_const_attr_in_altvoltage0_mag_reset_min_available.dev_attr.attr,
> &iio_dev_attr_in_angl1_thresh_rising_value_available.dev_attr.attr,
> &iio_dev_attr_in_angl1_thresh_rising_hysteresis_available.dev_attr.attr,
> NULL,
>
Powered by blists - more mailing lists