Re: [PATCH v3 07/10] rtc: max77686: Use dev_warn() instead of pr_warn()

From: Krzysztof Kozlowski
Date: Tue Jan 26 2016 - 21:06:23 EST


On 27.01.2016 10:53, Javier Martinez Canillas wrote:
> Hello Andi,
>
> Thanks a lot for your feedback and review.
>
> On 01/26/2016 10:22 PM, Andi Shyti wrote:
>> Hi Javier,
>>
>>> if (tm->tm_year < 100) {
>>> - pr_warn("RTC can't handle year %d. Assume it's 2000.\n",
>>> - 1900 + tm->tm_year);
>>> + dev_warn(info->dev,
>>> + "RTC can't handle year %d. Assume it's 2000\n",
>>> + 1900 + tm->tm_year);
>>> return -EINVAL;
>>
>> Because we are returning an error value, why not use dev_err()?
>>
>
> You are absolutely right. Since the driver was using pr_warn(), I used
> dev_warn() but dev_err() would had been correct.

Wait. The message says that "2000 will be assumed" which is not an
error. The message indicates that driver will proceed, thus the warning.

However the driver won't proceed because the max77686_rtc_set_time()
will abort. This came from max8997 which has the same issue.

This means that either message should be changed (dev_err() without the
"assume" verb) or the function should not abort and set the year to
2000+something (then dev_warn()... look at rtc-ds3234.c and rtc-mcp795.c).

The easiest would be to choose #1 - no changes in the logic.

BR,
Krzysztof