On 1/20/25 6:37 AM, Miclaus, Antoniu wrote:
>>> + }
>>> + channels++;
>>> +
>>> + st->bipolar_ch[reg] = fwnode_property_read_bool(child,
>> "bipolar");
>>> +
>>> + if (st->bipolar_ch[reg]) {
>>> + channels->scan_type.sign = 's';
>>> + } else {
>>> + ret = regmap_write(st->regmap,
>> AD4851_REG_CHX_SOFTSPAN(reg),
>>> + AD4851_SOFTSPAN_0V_40V);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + *ad4851_channels = channels;
>>
>> At this point, channels is pointing to memory we didn't allocate (because of
>> channels++). As in the previous review, I suggest we just get rid of the output
>> parameter since indio_dev->channels already has the correct pointer.
>>
>> It's less chance for mistakes like this and avoids needing to provide an unused
>> arg in ad4857_parse_channels().
>
> Hmm, how can I then do the assignments in `ad4858_parse_channels` ?
>
> drivers/iio/adc/ad4851.c:1055:42: error: assignment of member ‘has_ext_scan_type’ in read-only object
> 1055 | indio_dev->channels->has_ext_scan_type = 1;
> | ^
> drivers/iio/adc/ad4851.c:1057:39: error: assignment of member ‘ext_scan_type’ in read-only object
> 1057 | indio_dev->channels->ext_scan_type = ad4851_scan_type_20_b;
> | ^
> drivers/iio/adc/ad4851.c:1058:43: error: assignment of member ‘num_ext_scan_type’ in read-only object
> 1058 | indio_dev->channels->num_ext_scan_type = ARRAY_SIZE(ad4851_scan_type_20_b);
> | ^
> drivers/iio/adc/ad4851.c:1061:39: error: assignment of member ‘ext_scan_type’ in read-only object
> 1061 | indio_dev->channels->ext_scan_type = ad4851_scan_type_20_u;
> | ^
> drivers/iio/adc/ad4851.c:1062:43: error: assignment of member ‘num_ext_scan_type’ in read-only object
> 1062 | indio_dev->channels->num_ext_scan_type = ARRAY_SIZE(ad4851_scan_type_20_u);
> | ^
I would be tempted to just not have a second loop of
device_for_each_child_node_scoped(dev, child)
in ad4858_parse_channels() and instead do everything in ad4851_parse_channels()
and just pass a boolean parameter to conditionally handle the difference
between the two types of chips.
Or you use a cast to remove the const qualifier.
ad4851_channels = (struct iio_chan_spec *)indio_dev->channels;
