lkml.org 
[lkml]   [2020]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/5] iio: adc: ad7793: use read_avail iio hook for scale available
From
Date
On 3/19/20 8:58 AM, Ardelean, Alexandru wrote:
> On Wed, 2020-03-18 at 16:10 +0100, Lars-Peter Clausen wrote:
>> On 3/18/20 2:40 PM, Alexandru Ardelean wrote:
>>> This change uses the read_avail and '.info_mask_shared_by_type_available'
>>> modifier to set the available scale.
>>> Essentially, nothing changes to the driver's ABI.
>>>
>>> The main idea for this patch is to remove the AD7793 driver from
>>> checkpatch's radar. There have been about ~3 attempts to fix/break the
>>> 'in_voltage-voltage_scale_available' attribute, because checkpatch assumed
>>> it to be an arithmetic operation and people were trying to change that.
>>
>> Yeah, probably a good idea!
>>
>>> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>>> ---
>>> drivers/iio/adc/ad7793.c | 53 +++++++++++++++++++++++++++-------------
>>> 1 file changed, 36 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c
>>> index 5592ae573e6b..fad98f1801db 100644
>>> --- a/drivers/iio/adc/ad7793.c
>>> +++ b/drivers/iio/adc/ad7793.c
>>> @@ -354,29 +354,28 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
>>> static IIO_CONST_ATTR_NAMED(sampling_frequency_available_ad7797,
>>> sampling_frequency_available, "123 62 50 33 17 16 12 10 8 6 4");
>>>
>>> -static ssize_t ad7793_show_scale_available(struct device *dev,
>>> - struct device_attribute *attr, char *buf)
>>> +static int ad7793_read_avail(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + const int **vals, int *type, int *length,
>>> + long mask)
>>> {
>>> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> struct ad7793_state *st = iio_priv(indio_dev);
>>> - int i, len = 0;
>>>
>>> - for (i = 0; i < ARRAY_SIZE(st->scale_avail); i++)
>>> - len += sprintf(buf + len, "%d.%09u ", st->scale_avail[i][0],
>>> - st->scale_avail[i][1]);
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_SCALE:
>>> + *vals = (int *)st->scale_avail;
>>
>> Can you change the type of scale_avail to int so we don't need the cast?
>>
>
> So, I don't want to come-up as looking lazy.
> [I mean, I am lazy, but I don't want to look lazy.]
>
> I took a look at what it means to change this to a simple array.
> The rework feels to me like a bit more noise than is probably worth it.
> I mean, if the purpose of the rework is to just get rid of this cast, then it
> feels noisy [to me].
>
> That being said, if you insist, I can take a look and do a patch [before this
> one] to convert it to a simple array.

Hm, ok, looks like is more complicated to get rid of the cast than I
though. So keep it.

\
 
 \ /
  Last update: 2020-03-19 09:39    [W:0.064 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site