| 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: <20230930160336.6a67765a@jic23-huawei>
Date: Sat, 30 Sep 2023 16:03:36 +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 14/27] staging: iio: resolver: ad2s1210: implement
hysteresis as channel attr
On Fri, 29 Sep 2023 12:23:19 -0500
David Lechner <dlechner@...libre.com> wrote:
> From: David Lechner <david@...hnology.com>
>
> From: David Lechner <dlechner@...libre.com>
>
> The AD2S1210 resolver has a hysteresis feature that can be used to
> prevent flicker in the LSB of the position register. This can be either
> enabled or disabled. Disabling hysteresis is useful for increasing
> precision by oversampling.
>
> Signed-off-by: David Lechner <dlechner@...libre.com>
One trivial thing inline.
Thanks,
Jonathan
> ---
>
> v3 changes:
> * Refactored into more functions to reduce complexity of switch statements.
> * Use early return instead of break in switch statements.
>
> drivers/staging/iio/resolver/ad2s1210.c | 86 +++++++++++++++++++++++++++++++--
> 1 file changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c b/drivers/staging/iio/resolver/ad2s1210.c
> index 0ec3598b600a..a82cb124a12f 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -78,7 +78,6 @@ struct ad2s1210_state {
> /** The external oscillator frequency in Hz. */
> unsigned long clkin_hz;
> unsigned int fexcit;
> - bool hysteresis;
> u8 resolution;
> /** For reading raw sample value via SPI. */
> __be16 sample __aligned(IIO_DMA_MINALIGN);
> @@ -430,6 +429,35 @@ static int ad2s1210_single_conversion(struct ad2s1210_state *st,
> return ret;
> }
>
> +static int ad2s1210_get_hysteresis(struct ad2s1210_state *st, int *val)
> +{
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_test_bits(st->regmap, AD2S1210_REG_CONTROL,
> + AD2S1210_ENABLE_HYSTERESIS);
> + mutex_unlock(&st->lock);
> +
> + if (ret < 0)
> + return ret;
> +
> + *val = !!ret;
regmap_test_bits() is documented as returning 1 or 0 anyway.
So this !! isn't necessary..
> + return IIO_VAL_INT;
> +}
> +
> +static int ad2s1210_set_hysteresis(struct ad2s1210_state *st, int val)
> +{
> + int ret;
> +
> + mutex_lock(&st->lock);
> + ret = regmap_update_bits(st->regmap, AD2S1210_REG_CONTROL,
> + AD2S1210_ENABLE_HYSTERESIS,
> + val ? AD2S1210_ENABLE_HYSTERESIS : 0);
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> static const int ad2s1210_velocity_scale[] = {
> 17089132, /* 8.192MHz / (2*pi * 2500 / 2^15) */
> 42722830, /* 8.192MHz / (2*pi * 1000 / 2^15) */
> @@ -462,7 +490,55 @@ static int ad2s1210_read_raw(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_CHAN_INFO_HYSTERESIS:
> + switch (chan->type) {
> + case IIO_ANGL:
> + return ad2s1210_get_hysteresis(st, val);
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ad2s1210_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type,
> + int *length, long mask)
> +{
> + static const int hysteresis_available[] = { 0, 1 };
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_HYSTERESIS:
> + switch (chan->type) {
> + case IIO_ANGL:
> + *vals = hysteresis_available;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(hysteresis_available);
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
>
> +static int ad2s1210_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct ad2s1210_state *st = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_HYSTERESIS:
> + switch (chan->type) {
> + case IIO_ANGL:
> + return ad2s1210_set_hysteresis(st, val);
> + default:
> + return -EINVAL;
> + }
> default:
> return -EINVAL;
> }
> @@ -503,7 +579,10 @@ static const struct iio_chan_spec ad2s1210_channels[] = {
> .indexed = 1,
> .channel = 0,
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> - BIT(IIO_CHAN_INFO_SCALE),
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_HYSTERESIS),
> + .info_mask_separate_available =
> + BIT(IIO_CHAN_INFO_HYSTERESIS),
> }, {
> .type = IIO_ANGL_VEL,
> .indexed = 1,
> @@ -581,6 +660,8 @@ static int ad2s1210_debugfs_reg_access(struct iio_dev *indio_dev,
>
> static const struct iio_info ad2s1210_info = {
> .read_raw = ad2s1210_read_raw,
> + .read_avail = ad2s1210_read_avail,
> + .write_raw = ad2s1210_write_raw,
> .attrs = &ad2s1210_attribute_group,
> .debugfs_reg_access = &ad2s1210_debugfs_reg_access,
> };
> @@ -696,7 +777,6 @@ static int ad2s1210_probe(struct spi_device *spi)
>
> mutex_init(&st->lock);
> st->sdev = spi;
> - st->hysteresis = true;
> st->resolution = 12;
> st->fexcit = AD2S1210_DEF_EXCIT;
>
>
Powered by blists - more mailing lists