Re: [PATCH v2 2/2] iio: adc: ad9467: support write/read offset

[Tomas Melin]

On 02/12/2025 17:28, David Lechner wrote:
> On 12/2/25 9:05 AM, Nuno Sá wrote:
>> On Tue, 2025-12-02 at 16:52 +0200, Tomas Melin wrote:

>>>>> +
>>>>> +static int ad9467_set_offset(struct ad9467_state *st, int val)
>>>>> +{
>>>>> +	int ret;
>>>>> +
>>>>> +	if (val < st->info->offset_range[0] || val > st->info->offset_range[2])
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	ret = ad9467_spi_write(st, AN877_ADC_REG_OFFSET, val);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +	/* Sync registers */
>>>>
>>>> I think this is not what David meant by adding a comment. IMHO, the comment as-is does not
>>>> bring any added value.
>>> The sync operation is needed in several places and is not commented in
>>> other locations either. Do you prefer no comment or even more elaborate
>>> comment for this particular sync operation?
>>>
>>
>> I know. I'm just stating the comment, as is, does not bring much value. But I was not the one asking
>> for it so I guess you should ask David :)
>>
>> - Nuno Sá
> 
> I did not look at the rest of the driver before. I guess the
> fact that it does the sync after every register write makes it
> clear enough that this is just a thing you have to do. So I'm
> OK with leaving out the comment.
Thanks for the clarification. I will remove the commment for v3.

> 
> What I was asking for though is _why_ do we need to do that?

Addresses in range 0x8-0x2A require a sync operation to transfer the
data into use. So it's a kind of latching.

Thanks,
Tomas