| 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: <CAMknhBH8UewSoZu-V6ERda=q8wO2NYg8RinnC2+RMF3LhKwtUg@mail.gmail.com>
Date: Sat, 13 Apr 2024 15:31:13 -0500
From: David Lechner <dlechner@...libre.com>
To: Alisa-Dariana Roman <alisadariana@...il.com>
Cc: michael.hennerich@...log.com, linux-iio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
alexandru.tachici@...log.com, lars@...afoo.de, jic23@...nel.org,
robh@...nel.org, krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
lgirdwood@...il.com, broonie@...nel.org, andy@...nel.org, nuno.sa@...log.com,
marcelo.schmitt@...log.com, bigunclemax@...il.com, okan.sahin@...log.com,
fr0st61te@...il.com, alisa.roman@...log.com, marcus.folkesson@...il.com,
schnelle@...ux.ibm.com, liambeguin@...il.com
Subject: Re: [PATCH v5 1/5] iio: adc: ad7192: Use standard attribute
On Sat, Apr 13, 2024 at 10:12 AM Alisa-Dariana Roman
<alisadariana@...il.com> wrote:
>
> Replace custom attribute filter_low_pass_3db_frequency_available with
> standard attribute.
>
> Store the available values in ad7192_state struct.
>
> The function that used to compute those values replaced by
> ad7192_update_filter_freq_avail().
>
> Function ad7192_show_filter_avail() is no longer needed.
>
> Note that the initial available values are hardcoded.
>
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@...log.com>
> ---
With the question below addressed:
Reviewed-by: David Lechner <dlechner@...libre.com>
> drivers/iio/adc/ad7192.c | 67 ++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 7bcc7e2aa2a2..ac737221beae 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -190,6 +190,7 @@ struct ad7192_state {
> u32 mode;
> u32 conf;
> u32 scale_avail[8][2];
> + u32 filter_freq_avail[4][2];
> u32 oversampling_ratio_avail[4];
> u8 gpocon;
> u8 clock_sel;
> @@ -473,6 +474,16 @@ static int ad7192_setup(struct iio_dev *indio_dev, struct device *dev)
> st->oversampling_ratio_avail[2] = 8;
> st->oversampling_ratio_avail[3] = 16;
>
> + st->filter_freq_avail[0][0] = 600;
> + st->filter_freq_avail[1][0] = 800;
> + st->filter_freq_avail[2][0] = 2300;
> + st->filter_freq_avail[3][0] = 2720;
> +
> + st->filter_freq_avail[0][1] = 1000;
> + st->filter_freq_avail[1][1] = 1000;
> + st->filter_freq_avail[2][1] = 1000;
> + st->filter_freq_avail[3][1] = 1000;
> +
> return 0;
> }
>
> @@ -586,48 +597,24 @@ static int ad7192_get_f_adc(struct ad7192_state *st)
> f_order * FIELD_GET(AD7192_MODE_RATE_MASK, st->mode));
> }
>
> -static void ad7192_get_available_filter_freq(struct ad7192_state *st,
> - int *freq)
> +static void ad7192_update_filter_freq_avail(struct ad7192_state *st)
> {
> unsigned int fadc;
>
> /* Formulas for filter at page 25 of the datasheet */
> fadc = ad7192_compute_f_adc(st, false, true);
> - freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> + st->filter_freq_avail[0][0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
>
> fadc = ad7192_compute_f_adc(st, true, true);
> - freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> + st->filter_freq_avail[1][0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
>
> fadc = ad7192_compute_f_adc(st, false, false);
> - freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
> + st->filter_freq_avail[2][0] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
>
> fadc = ad7192_compute_f_adc(st, true, false);
> - freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
> + st->filter_freq_avail[3][0] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
> }
>
> -static ssize_t ad7192_show_filter_avail(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ad7192_state *st = iio_priv(indio_dev);
> - unsigned int freq_avail[4], i;
> - size_t len = 0;
> -
> - ad7192_get_available_filter_freq(st, freq_avail);
> -
> - for (i = 0; i < ARRAY_SIZE(freq_avail); i++)
> - len += sysfs_emit_at(buf, len, "%d.%03d ", freq_avail[i] / 1000,
> - freq_avail[i] % 1000);
> -
> - buf[len - 1] = '\n';
> -
> - return len;
> -}
> -
> -static IIO_DEVICE_ATTR(filter_low_pass_3db_frequency_available,
> - 0444, ad7192_show_filter_avail, NULL, 0);
> -
> static IIO_DEVICE_ATTR(bridge_switch_en, 0644,
> ad7192_show_bridge_switch, ad7192_set,
> AD7192_REG_GPOCON);
> @@ -637,7 +624,6 @@ static IIO_DEVICE_ATTR(ac_excitation_en, 0644,
> AD7192_REG_CONF);
>
> static struct attribute *ad7192_attributes[] = {
> - &iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
> &iio_dev_attr_bridge_switch_en.dev_attr.attr,
> NULL
> };
> @@ -647,7 +633,6 @@ static const struct attribute_group ad7192_attribute_group = {
> };
>
> static struct attribute *ad7195_attributes[] = {
> - &iio_dev_attr_filter_low_pass_3db_frequency_available.dev_attr.attr,
> &iio_dev_attr_bridge_switch_en.dev_attr.attr,
> &iio_dev_attr_ac_excitation_en.dev_attr.attr,
> NULL
> @@ -665,17 +650,15 @@ static unsigned int ad7192_get_temp_scale(bool unipolar)
> static int ad7192_set_3db_filter_freq(struct ad7192_state *st,
> int val, int val2)
> {
> - int freq_avail[4], i, ret, freq;
> + int i, ret, freq;
> unsigned int diff_new, diff_old;
> int idx = 0;
>
> diff_old = U32_MAX;
> freq = val * 1000 + val2;
>
> - ad7192_get_available_filter_freq(st, freq_avail);
> -
> - for (i = 0; i < ARRAY_SIZE(freq_avail); i++) {
> - diff_new = abs(freq - freq_avail[i]);
> + for (i = 0; i < ARRAY_SIZE(st->filter_freq_avail); i++) {
> + diff_new = abs(freq - st->filter_freq_avail[i][0]);
> if (diff_new < diff_old) {
> diff_old = diff_new;
> idx = i;
> @@ -826,6 +809,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
> st->mode &= ~AD7192_MODE_RATE_MASK;
> st->mode |= FIELD_PREP(AD7192_MODE_RATE_MASK, div);
> ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
> + ad7192_update_filter_freq_avail(st);
> break;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> ret = ad7192_set_3db_filter_freq(st, val, val2 / 1000);
> @@ -846,6 +830,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
> break;
> }
> mutex_unlock(&st->lock);
> + ad7192_update_filter_freq_avail(st);
Does this need to go inside of the mutex guard to avoid potential race
conditions?
> break;
> default:
> ret = -EINVAL;
> @@ -888,6 +873,12 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
> /* Values are stored in a 2D matrix */
> *length = ARRAY_SIZE(st->scale_avail) * 2;
>
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + *vals = (int *)st->filter_freq_avail;
> + *type = IIO_VAL_FRACTIONAL;
> + *length = ARRAY_SIZE(st->filter_freq_avail) * 2;
> +
> return IIO_AVAIL_LIST;
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> *vals = (int *)st->oversampling_ratio_avail;
> @@ -956,7 +947,9 @@ static const struct iio_info ad7195_info = {
> BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
> (_mask_all), \
> .info_mask_shared_by_type_available = (_mask_type_av), \
> - .info_mask_shared_by_all_available = (_mask_all_av), \
> + .info_mask_shared_by_all_available = \
> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | \
> + (_mask_all_av), \
> .ext_info = (_ext_info), \
> .scan_index = (_si), \
> .scan_type = { \
> --
> 2.34.1
>
Powered by blists - more mailing lists