Re: [PATCH 4/6] rtc: rv8803: Always apply the IÂC workaround

[BenoÃt ThÃbaudeau]

On 21/07/2016 at 14:34, BenoÃt ThÃbaudeau wrote:
> On 21/07/2016 at 13:10, Alexandre Belloni wrote:
>> On 21/07/2016 at 12:41:30 +0200, BenoÃt ThÃbaudeau wrote :
>>> The IÂC NACK issue of the RV-8803 may occur after any IÂC START
>>> condition, depending on the timings. Consequently, the workaround must
>>> be applied for all the IÂC transfers.
>>>
>>> This commit abstracts the IÂC transfer code into register access
>>> functions. This avoids duplicating the IÂC workaround everywhere. This
>>> also avoids the duplication of the code handling the return value of
>>> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
>>> definitive register access failures (if the workaround fails). This
>>> change also makes the IÂC transfer return value checks consistent.
>>>
>>
>> Well, my initial idea was that the workaround is actually needed only
>> for operations that are not restartable from userspace.
>>
>> Did you it that bug? On which RTC?
>
> No, I've not seen that bug. However, the errata sheet says that this issue may
> occur if two consecutive IÂC START conditions are slightly more than 950 ms
> apart. It does not even say that this is restricted to IÂC transfers
> addressing
> this RTC. Moreover, if the time is read or set from userspace, this might
> occur
> at any time relatively to an RTC interrupt or to a previous time read/set
> operation, so possibly 950 ms afterwards.
>
> All in all, it's safer and easier to always apply this workaround, all the
> more
> it is applied at almost no cost when there is no issue.

To fully answer your questions, it's the job of the driver to execute the
userspace requests properly without returning spurious errors. Userspace might
consider that such errors are definitive and that there is no reason to try and
restart the operation, which could cause all sorts of issues. In other words,
the driver should not rely on userspace operation restarts as a kind of
workaround if it can easily handle this itself.

Also, the kernel itself (not only userspace) may read the RTC time (with
CONFIG_RTC_HCTOSYS), without userspace caring.

I'm rather working with the RX8900.

Best regards,
BenoÃt