> > 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://lkml.org > >> %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. > > >> 2. Replace `max_register` method validation with `callback > >> function` validation method, were you could make your own validation. > > It is not work, show the code in as follows: > > > > bool regmap_writeable(struct regmap *map, unsigned int reg) { > > if (map->max_register && reg > map->max_register) > > return false; > > Callback function (writeable_reg) will not be called. > > if (map->writeable_reg) > > return map->writeable_reg(map->dev, reg); > > Hi Li, > If you *replace* the `max_register` method with `callback function` it > should work. The code above will use every method *if provided*. In other > words if `map->max_register` is 0 will go to the next step and check > `map->writeable_reg`. Right? Yes, you are right. Thanks. > > > > Regards, > Nandor
