lkml.org 
[lkml]   [2019]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [EXT] Re: [v2] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w
    Hi,

    On 26/08/2019 09:49:49+0000, Biwen Li wrote:
    > >
    > > On 8/26/19 7:29 AM, Biwen Li wrote:
    > > >>
    > > >> On 8/16/19 10:40 PM, Li Yang wrote:
    > > >>> On Fri, Aug 16, 2019 at 11:30 AM Alexandre Belloni
    > > >>> <alexandre.belloni@bootlin.com> wrote:
    > > >>>>
    > > >>>> On 16/08/2019 10:50:49-0500, Li Yang wrote:
    > > >>>>> On Fri, Aug 16, 2019 at 3:05 AM Alexandre Belloni
    > > >>>>> <alexandre.belloni@bootlin.com> wrote:
    > > >>>>>>
    > > >>>>>> On 16/08/2019 10:46:36+0800, Biwen Li wrote:
    > > >>>>>>> Issue:
    > > >>>>>>> - # hwclock -w
    > > >>>>>>> hwclock: RTC_SET_TIME: Invalid argument
    > > >>>>>>>
    > > >>>>>>> Why:
    > > >>>>>>> - Relative patch:
    > > >> https://lkml.org
    > > >> %2Flkml%2F2019%2F4%2F3%2F55&amp;data=02%7C01%7Cbiwen.li%40n
    > > xp.
    > > >>
    > > com%7Cff8cebc3f1034ae3fa9608d725ff9e5e%7C686ea1d3bc2b4c6fa92cd99
    > > >>
    > > c5c301635%7C0%7C0%7C637019652111923736&amp;sdata=spY6e22YOkOF
    > > >> 3%2BF7crSM0M6xPmOhgULDqMZLQw%2BAmdI%3D&amp;reserved=0 ,
    > > this patch
    > > >>>>>>> will always check for unwritable registers, it will compare reg
    > > >>>>>>> with max_register in regmap_writeable.
    > > >>>>>>>
    > > >>>>>>> - In drivers/rtc/rtc-pcf85363.c, CTRL_STOP_EN is 0x2e, but
    > > >> DT_100THS
    > > >>>>>>> is 0, max_regiter is 0x2f, then reg will be equal to 0x30,
    > > >>>>>>> '0x30 < 0x2f' is false,so regmap_writeable will return false.
    > > >>>>>>>
    > > >>>>>>> - Root cause: the buf[] was written to a wrong place in the file
    > > >>>>>>> drivers/rtc/rtc-pcf85363.c
    > > >>>>>>>
    > > >>>>>>
    > > >>>>>> This is not true, the RTC wraps the register accesses properly
    > > >>>>>> and this
    > > >>>>>
    > > >>>>> This performance hack probably deserve some explanation in the
    > > >>>>> code comment. :)
    > > >>>>>
    > > >>>>>> is probably something that should be handled by regmap_writable.
    > > >>>>>
    > > >>>>> The address wrapping is specific to this RTC chip. Is it also
    > > >>>>> commonly used by other I2C devices? I'm not sure if
    > > >>>>> regmap_writable should handle the wrapping case if it is too special.
    > > >>>>>
    > > >>>>
    > > >>>> Most of the i2c RTCs do address wrapping which is sometimes the
    > > >>>> only way to properly set the time.
    > > >>>
    > > >>> Adding Mark and Nandor to the loop.
    > > >>>
    > > >>> Regards,
    > > >>> Leo
    > > >>>
    > > >>
    > > >> Hi,
    > > >> `regmap` provides couple of ways to validate the registers:
    > > >> max_register, callback function and write table. All of these are
    > > >> optional, so it gives you the freedom to customize it as needed.
    > > >>
    > > >> In this situation probably you could:
    > > >> 1. Avoid using the wrapping feature of pcf85363 (you can just
    > > >> provide separate calls for stop, reset and time confguration). In
    > > >> this way the `max_register` validation method will work fine.
    > > > Yes, I use this way. Path as follows:
    > > > Stop and reset - > set time > stop
    > > >
    > >
    > > Some of the concerns regarding this method was that it might not be precise
    > > enough. That because you need 2 I2C operations (one for stop and one for time
    > > configuration). Not sure about your case if this is a problem or not.
    > Ok, got it, thanks.

    To be clear, for this RTC it is fine to separate both writes. Want I
    want is a corrected commit message with a proper reference to
    8b9f9d4dc511309918c4f6793bae7387c0c638af instead of a link to lkml.org
    and a proper explanation.

    --
    Alexandre Belloni, Bootlin
    Embedded Linux and Kernel engineering
    https://bootlin.com

    \
     
     \ /
      Last update: 2019-08-26 12:07    [W:4.048 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site