Messages in this thread | | | Date | Wed, 27 Jan 2016 11:35:54 +0900 | From | Andi Shyti <> | Subject | Re: [PATCH v3 07/10] rtc: max77686: Use dev_warn() instead of pr_warn() |
| |
> > 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.
Nevertheless, the function fails, and those who call max77686_rtc_tm_to_data() fail as well, so, we are printing warning, but behaving as error.
Either we print error and return error, or we print warning, we set:
tm->tm_year = 100; /* don't know if I got the logic right */
and return 0
Right?
Thanks, Andi
| |