linux-kernel - Re: [PATCH 4/6] iio: adc: ad7606: rework scale-available to be static

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0a7c5305-b4f7-4166-8a8a-05cf6e3273cc@baylibre.com>
Date: Mon, 21 Oct 2024 13:17:01 -0500
From: David Lechner <dlechner@...libre.com>
To: Alexandru Ardelean <aardelean@...libre.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Cc: jic23@...nel.org, krzk+dt@...nel.org, robh@...nel.org, lars@...afoo.de,
 michael.hennerich@...log.com, gstols@...libre.com
Subject: Re: [PATCH 4/6] iio: adc: ad7606: rework scale-available to be static

On 10/21/24 8:02 AM, Alexandru Ardelean wrote:
> The main driver for this change is the AD7607 part, which has a scale of
> "1.220703" for the ±10V range. The AD7607 has a resolution of 14-bits.
> 
> So, just adding the scale-available list for that part would require some
> quirks to handle just that scale value.
> But to do it more neatly, the best approach is to rework the scale
> available lists to have the same format as it is returned to userspace.
> That way, we can also get rid of the allocation for the 'scale_avail_show'
> array.
> 
> Signed-off-by: Alexandru Ardelean <aardelean@...libre.com>
> ---

...


>  static ssize_t in_voltage_scale_available_show(struct device *dev,
>  					       struct device_attribute *attr,
>  					       char *buf)
> @@ -703,8 +690,16 @@ static ssize_t in_voltage_scale_available_show(struct device *dev,
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	struct ad7606_chan_scale *cs = &st->chan_scales[0];
> +	const unsigned int (*vals)[2] = cs->scale_avail;
> +	unsigned int i;
> +	size_t len = 0;
>  
> -	return ad7606_show_avail(buf, cs->scale_avail, cs->num_scales, true);
> +	for (i = 0; i < cs->num_scales; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "%u.%06u ",
> +				 vals[i][0], vals[i][1]);
> +	buf[len - 1] = '\n';
> +
> +	return len;
>  }
>  
>  static IIO_DEVICE_ATTR_RO(in_voltage_scale_available, 0);

Probably a reason for this that I forgot, but why is this handled in a
custom attribute instead of ad7606_read_avail()?

> @@ -742,6 +737,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  			    long mask)
>  {
>  	struct ad7606_state *st = iio_priv(indio_dev);
> +	unsigned int scale_avail[AD760X_MAX_SCALES];

Calling this scale_avail_uv would make the code easier for me to understand.

>  	struct ad7606_chan_scale *cs;
>  	int i, ret, ch = 0;
>  
> @@ -752,7 +748,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
>  		if (st->sw_mode_en)
>  			ch = chan->address;
>  		cs = &st->chan_scales[ch];
> -		i = find_closest(val2, cs->scale_avail, cs->num_scales);
> +		for (i = 0; i < cs->num_scales; i++) {
> +			scale_avail[i] = cs->scale_avail[i][0] * 1000000 +

								 MICRO

> +					 cs->scale_avail[i][1];
> +		}
> +		val = val * 1000000 + val2;
> +		i = find_closest(val, scale_avail, cs->num_scales);
>  		ret = st->write_scale(indio_dev, ch, i + cs->reg_offset);
>  		if (ret < 0)
>  			return ret;
> @@ -788,9 +789,15 @@ static ssize_t ad7606_oversampling_ratio_avail(struct device *dev,
>  {
>  	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>  	struct ad7606_state *st = iio_priv(indio_dev);
> +	const unsigned int *vals = st->oversampling_avail;
> +	unsigned int i;
> +	size_t len = 0;
>  
> -	return ad7606_show_avail(buf, st->oversampling_avail,
> -				 st->num_os_ratios, false);
> +	for (i = 0; i < st->num_os_ratios; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "%u ", vals[i]);
> +	buf[len - 1] = '\n';
> +
> +	return len;
>  }
>  
>  static IIO_DEVICE_ATTR(oversampling_ratio_available, 0444,

Same question about ad7606_read_avail() instead for this one.