> > 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@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2 > > > >> > Flkml.org&data=02%7C01%7Cbiwen.li%40nxp.com%7C03141ff7858343 > 3 > > > >> > 20be408d72a0d1e10%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C63 > > > >> > 7024108138794294&sdata=QrALkFN6heF%2B7S73FQ9c%2FyKNRHyBuL > %2B6 > > > >> %2B4PDM9hYRyM%3D&reserved=0 > > > >> %2Flkml%2F2019%2F4%2F3%2F55&data=02%7C01%7Cbiwen.li% > 40n > > > xp. > > > >> > > > > com%7Cff8cebc3f1034ae3fa9608d725ff9e5e%7C686ea1d3bc2b4c6fa92cd99 > > > >> > > > > c5c301635%7C0%7C0%7C637019652111923736&sdata=spY6e22YOkOF > > > >> > 3%2BF7crSM0M6xPmOhgULDqMZLQw%2BAmdI%3D&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. Ok, got it, thanks.I will replace link to lkml.org with 8b9f9d4dc511309918c4f6793bae7387c0c638af and add a proper explanation to the commit message in v4. > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin. > com&data=02%7C01%7Cbiwen.li%40nxp.com%7C03141ff7858343320b > e408d72a0d1e10%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6 > 37024108138794294&sdata=XnAxJmOkh1VVA9ed%2FLr%2BbvWbVpLD > bwLjJrdaFidRtDk%3D&reserved=0