From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C353C3A59F for ; Mon, 26 Aug 2019 10:06:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4522F21852 for ; Mon, 26 Aug 2019 10:06:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730036AbfHZKGx (ORCPT ); Mon, 26 Aug 2019 06:06:53 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:47401 "EHLO relay4-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729563AbfHZKGx (ORCPT ); Mon, 26 Aug 2019 06:06:53 -0400 X-Originating-IP: 86.207.98.53 Received: from localhost (aclermont-ferrand-651-1-259-53.w86-207.abo.wanadoo.fr [86.207.98.53]) (Authenticated sender: alexandre.belloni@bootlin.com) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 0A522E0002; Mon, 26 Aug 2019 10:06:50 +0000 (UTC) Date: Mon, 26 Aug 2019 12:06:50 +0200 From: Alexandre Belloni To: Biwen Li Cc: Nandor Han , Leo Li , Mark Brown , "a.zummo@towertech.it" , "linux-rtc@vger.kernel.org" , lkml Subject: Re: [EXT] Re: [v2] rtc: pcf85363/pcf85263: fix error that failed to run hwclock -w Message-ID: <20190826100650.GB21713@piout.net> References: <20190816024636.34738-1-biwen.li@nxp.com> <20190816080417.GB3545@piout.net> <20190816162825.GE3545@piout.net> <21f417e3-db50-5930-ddc9-eed54f5d5893@vaisala.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.1 (2019-06-15) Sender: linux-rtc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rtc@vger.kernel.org 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 > > >>> wrote: > > >>>> > > >>>> On 16/08/2019 10:50:49-0500, Li Yang wrote: > > >>>>> On Fri, Aug 16, 2019 at 3:05 AM Alexandre Belloni > > >>>>> 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. 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