[{"id":1752080,"web_url":"http://patchwork.ozlabs.org/comment/1752080/","msgid":"<20170820083219.siez3rl2hbjgjgk5@piout.net>","list_archive_url":null,"date":"2017-08-20T08:32:19","subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","submitter":{"id":26276,"url":"http://patchwork.ozlabs.org/api/people/26276/","name":"Alexandre Belloni","email":"alexandre.belloni@free-electrons.com"},"content":"Hi,\n\nOn 20/08/2017 at 03:36:30 +0200, Andreas Färber wrote:\n> +static int rtd119x_rtc_read_time(struct device *dev, struct rtc_time *tm)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\ttime64_t t;\n> +\tu32 day;\n> +\n> +\tday = readl_relaxed(data->base + RTD_RTCDATE_LOW);\n> +\tday |= readl_relaxed(data->base + RTD_RTCDATE_HIGH) << 8;\n\nIs RTD_RTCDATE_HIGH latched when RTD_RTCDATE_LOW is read? If this is not\nthe case, you probably want to handle a possible update between both\nreadl_relaxed.\n\n> +\tt = mktime64(data->base_year, 1, 1, 0, 0, 0);\n> +\tt += day * 24 * 60 * 60;\n> +\trtc_time64_to_tm(t, tm);\n> +\ttm->tm_sec = readl_relaxed(data->base + RTD_RTCSEC) >> 1;\n> +\ttm->tm_min = readl_relaxed(data->base + RTD_RTCMIN);\n> +\ttm->tm_hour = readl_relaxed(data->base + RTD_RTCHR);\n> +\n> +\treturn rtc_valid_tm(tm);\n> +}\n> +\n> +static int rtd119x_rtc_set_time(struct device *dev, struct rtc_time *tm)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\ttime64_t time_base, new_time, time_delta;\n> +\tunsigned long day;\n> +\n> +\tif (tm->tm_year < data->base_year)\n> +\t\treturn -EINVAL;\n> +\n> +\ttime_base = mktime64(data->base_year, 1, 1, 0, 0, 0);\n> +\tnew_time = rtc_tm_to_time64(tm);\n> +\ttime_delta = new_time - time_base;\n> +\tday = time_delta / (24 * 60 * 60);\n> +\tif (day > 0x7fff)\n> +\t\treturn -EINVAL;\n> +\n> +\trtd119x_rtc_set_enabled(dev, false);\n> +\n> +\twritel_relaxed(tm->tm_sec, data->base + RTD_RTCSEC);\n> +\twritel_relaxed(tm->tm_min, data->base + RTD_RTCMIN);\n> +\twritel_relaxed(tm->tm_hour, data->base + RTD_RTCHR);\n> +\twritel_relaxed(day & 0xff, data->base + RTD_RTCDATE_LOW);\n> +\twritel_relaxed((day >> 8) & 0x7f, data->base + RTD_RTCDATE_HIGH);\n> +\n> +\trtd119x_rtc_set_enabled(dev, true);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static int rtd119x_rtc_open(struct device *dev)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\tu32 val;\n> +\tint ret;\n> +\n> +\tret = clk_prepare_enable(data->clk);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tval = readl_relaxed(data->base + RTD_RTCACR);\n> +\tdev_info(dev, \"rtcacr = 0x%08x\\n\", val);\n> +\tif (!(val & BIT(7))) {\n> +\t}\n\nI don't see the point of reading that register, and not doing anything\nwith it.\n\n> +\n> +\trtd119x_rtc_set_enabled(dev, true);\n> +\n\nThis is certainly not what you want. The RTC device is usually not\nopened so enabling the RTC when open and disabling it when closed will\nnot work on most systems. This is probably true for the clock too. i.e\nwhat you do here should be done in probe.\n\n> +\treturn 0;\n> +}\n> +\n> +static void rtd119x_rtc_release(struct device *dev)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\n> +\trtd119x_rtc_set_enabled(dev, false);\n> +\n> +\tclk_disable_unprepare(data->clk);\n> +}\n> +\n> +static const struct rtc_class_ops rtd119x_rtc_ops = {\n> +\t.open\t\t= rtd119x_rtc_open,\n> +\t.release\t= rtd119x_rtc_release,\n> +\t.read_time\t= rtd119x_rtc_read_time,\n> +\t.set_time\t= rtd119x_rtc_set_time,\n> +};\n> +\n> +static const struct of_device_id rtd119x_rtc_dt_ids[] = {\n> +\t { .compatible = \"realtek,rtd1295-rtc\" },\n> +\t { }\n> +};\n> +\n> +static int rtd119x_rtc_probe(struct platform_device *pdev)\n> +{\n> +\tstruct rtd119x_rtc *data;\n> +\tstruct resource *res;\n> +\n> +\tdata = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);\n> +\tif (!data)\n> +\t\treturn -ENOMEM;\n> +\n> +\tplatform_set_drvdata(pdev, data);\n> +\tdata->base_year = 2014;\n> +\n> +\tres = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n> +\tdata->base = devm_ioremap_resource(&pdev->dev, res);\n> +\tif (IS_ERR(data->base))\n> +\t\treturn PTR_ERR(data->base);\n> +\n> +\tdata->clk = of_clk_get(pdev->dev.of_node, 0);\n> +\tif (IS_ERR(data->clk))\n> +\t\treturn PTR_ERR(data->clk);\n> +\n> +\tdata->rtcdev = devm_rtc_device_register(&pdev->dev, \"rtc\",\n> +\t\t\t\t&rtd119x_rtc_ops, THIS_MODULE);\n> +\tif (IS_ERR(data->rtcdev)) {\n> +\t\tdev_err(&pdev->dev, \"failed to register rtc device\");\n> +\t\tclk_put(data->clk);\n> +\t\treturn PTR_ERR(data->rtcdev);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +static struct platform_driver rtd119x_rtc_driver = {\n> +\t.probe = rtd119x_rtc_probe,\n> +\t.driver = {\n> +\t\t.name = \"rtd1295-rtc\",\n> +\t\t.of_match_table\t= rtd119x_rtc_dt_ids,\n> +\t},\n> +};\n> +builtin_platform_driver(rtd119x_rtc_driver);\n> -- \n> 2.12.3\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xZqnN5F6jz9sN5\n\tfor ;\n\tSun, 20 Aug 2017 18:32:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751549AbdHTIcX (ORCPT );\n\tSun, 20 Aug 2017 04:32:23 -0400","from mail.free-electrons.com ([62.4.15.54]:49351 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751264AbdHTIcW (ORCPT\n\t); Sun, 20 Aug 2017 04:32:22 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid EC8D623009; Sun, 20 Aug 2017 10:32:19 +0200 (CEST)","from localhost (102.201.6.84.rev.sfr.net [84.6.201.102])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id A328A23007;\n\tSun, 20 Aug 2017 10:32:19 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT,\n\tURIBL_BLOCKED shortcircuit=ham autolearn=disabled version=3.4.0","Date":"Sun, 20 Aug 2017 10:32:19 +0200","From":"Alexandre Belloni ","To":"Andreas =?iso-8859-1?Q?F=E4rber?= ","Cc":"Alessandro Zummo , linux-rtc@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tRoc He ,\n\t=?utf-8?B?6JKL5Li955C0?= ","Subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","Message-ID":"<20170820083219.siez3rl2hbjgjgk5@piout.net>","References":"<20170820013632.18375-1-afaerber@suse.de>\n\t<20170820013632.18375-3-afaerber@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20170820013632.18375-3-afaerber@suse.de>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1752187,"web_url":"http://patchwork.ozlabs.org/comment/1752187/","msgid":"<20170820154017.GD24150@lunn.ch>","list_archive_url":null,"date":"2017-08-20T15:40:17","subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","submitter":{"id":13608,"url":"http://patchwork.ozlabs.org/api/people/13608/","name":"Andrew Lunn","email":"andrew@lunn.ch"},"content":"> +static void rtd119x_rtc_set_enabled(struct device *dev, bool enable)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\tu32 val;\n> +\n> +\tval = readl_relaxed(data->base + RTD_RTCEN);\n> +\tdev_info(dev, \"%s: rtcen = 0x%08x\\n\", __func__, val);\n\ndev_dbg()?\n\n> +static int rtd119x_rtc_open(struct device *dev)\n> +{\n> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n> +\tu32 val;\n> +\tint ret;\n> +\n> +\tret = clk_prepare_enable(data->clk);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tval = readl_relaxed(data->base + RTD_RTCACR);\n> +\tdev_info(dev, \"rtcacr = 0x%08x\\n\", val);\n\ndev_dbg()?\n\n\tAndrew","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xb1HX5nMRz9sRg\n\tfor ;\n\tMon, 21 Aug 2017 01:40:40 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753064AbdHTPkj (ORCPT );\n\tSun, 20 Aug 2017 11:40:39 -0400","from vps0.lunn.ch ([178.209.37.122]:36254 \"EHLO vps0.lunn.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753039AbdHTPki (ORCPT );\n\tSun, 20 Aug 2017 11:40:38 -0400","from andrew by vps0.lunn.ch with local (Exim 4.84_2)\n\t(envelope-from )\n\tid 1djSKn-0002s5-F5; Sun, 20 Aug 2017 17:40:17 +0200"],"Date":"Sun, 20 Aug 2017 17:40:17 +0200","From":"Andrew Lunn ","To":"Andreas =?iso-8859-1?Q?F=E4rber?= ","Cc":"Alessandro Zummo ,\n\tAlexandre Belloni ,\n\tlinux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\t????????? ,\n\tlinux-kernel@vger.kernel.org, Roc He ","Subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","Message-ID":"<20170820154017.GD24150@lunn.ch>","References":"<20170820013632.18375-1-afaerber@suse.de>\n\t<20170820013632.18375-3-afaerber@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170820013632.18375-3-afaerber@suse.de>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1752265,"web_url":"http://patchwork.ozlabs.org/comment/1752265/","msgid":"<3307d3ed-c8e0-a7b5-f0ee-d401f4b12a90@suse.de>","list_archive_url":null,"date":"2017-08-20T21:10:16","subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","submitter":{"id":9542,"url":"http://patchwork.ozlabs.org/api/people/9542/","name":"Andreas Färber","email":"afaerber@suse.de"},"content":"Hi Alexandre,\n\nAm 20.08.2017 um 10:32 schrieb Alexandre Belloni:\n> On 20/08/2017 at 03:36:30 +0200, Andreas Färber wrote:\n>> +static int rtd119x_rtc_read_time(struct device *dev, struct rtc_time *tm)\n>> +{\n>> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n>> +\ttime64_t t;\n>> +\tu32 day;\n>> +\n>> +\tday = readl_relaxed(data->base + RTD_RTCDATE_LOW);\n>> +\tday |= readl_relaxed(data->base + RTD_RTCDATE_HIGH) << 8;\n> \n> Is RTD_RTCDATE_HIGH latched when RTD_RTCDATE_LOW is read?\n\nI do not have an answer to that.\n\n> If this is not\n> the case, you probably want to handle a possible update between both\n> readl_relaxed.\n\nAre you proposing to disable the RTC while reading the registers, or\nsomething related to my choice of _relaxed? (it follows an explanation\nby Marc Zyngier on the irq mux series) Inconsistencies might not be\nlimited to the date.\n\n>> +\tt = mktime64(data->base_year, 1, 1, 0, 0, 0);\n>> +\tt += day * 24 * 60 * 60;\n>> +\trtc_time64_to_tm(t, tm);\n\nBTW is there any more efficient way to get from year+days to\nday/month/year without going via seconds?\n\n>> +\ttm->tm_sec = readl_relaxed(data->base + RTD_RTCSEC) >> 1;\n>> +\ttm->tm_min = readl_relaxed(data->base + RTD_RTCMIN);\n>> +\ttm->tm_hour = readl_relaxed(data->base + RTD_RTCHR);\n>> +\n>> +\treturn rtc_valid_tm(tm);\n>> +}\n>> +\n>> +static int rtd119x_rtc_set_time(struct device *dev, struct rtc_time *tm)\n>> +{\n>> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n>> +\ttime64_t time_base, new_time, time_delta;\n>> +\tunsigned long day;\n>> +\n>> +\tif (tm->tm_year < data->base_year)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\ttime_base = mktime64(data->base_year, 1, 1, 0, 0, 0);\n>> +\tnew_time = rtc_tm_to_time64(tm);\n>> +\ttime_delta = new_time - time_base;\n>> +\tday = time_delta / (24 * 60 * 60);\n>> +\tif (day > 0x7fff)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\trtd119x_rtc_set_enabled(dev, false);\n>> +\n>> +\twritel_relaxed(tm->tm_sec, data->base + RTD_RTCSEC);\n>> +\twritel_relaxed(tm->tm_min, data->base + RTD_RTCMIN);\n>> +\twritel_relaxed(tm->tm_hour, data->base + RTD_RTCHR);\n>> +\twritel_relaxed(day & 0xff, data->base + RTD_RTCDATE_LOW);\n>> +\twritel_relaxed((day >> 8) & 0x7f, data->base + RTD_RTCDATE_HIGH);\n>> +\n>> +\trtd119x_rtc_set_enabled(dev, true);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +static int rtd119x_rtc_open(struct device *dev)\n>> +{\n>> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n>> +\tu32 val;\n>> +\tint ret;\n>> +\n>> +\tret = clk_prepare_enable(data->clk);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tval = readl_relaxed(data->base + RTD_RTCACR);\n>> +\tdev_info(dev, \"rtcacr = 0x%08x\\n\", val);\n>> +\tif (!(val & BIT(7))) {\n>> +\t}\n> \n> I don't see the point of reading that register, and not doing anything\n> with it.\n\nThanks for spotting this. The story is that the old downstream has code\nfor this case, but in my testing I didn't run into that path and forgot.\nExplanations are largely missing in the vendor code. That code should\nprobably be added here in v2 and the dev_info() dropped, rather than\ndropping the current no-op expression.\n\n>> +\n>> +\trtd119x_rtc_set_enabled(dev, true);\n>> +\n> \n> This is certainly not what you want. The RTC device is usually not\n> opened so enabling the RTC when open and disabling it when closed will\n> not work on most systems. This is probably true for the clock too. i.e\n> what you do here should be done in probe.\n\nI did test the probe path to work, but I can change it again. The\ndownstream code had it in probe, but looking at rtc_class_ops I saw\nthose hooks and thought they'd serve a purpose - what are they for then?\n(Any chance you can improve the documentation comments to avoid such\nmisunderstandings? :))\n\n>> +\treturn 0;\n>> +}\n[snip]\n\nRegards,\nAndreas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xb8bz4lPLz9t2V\n\tfor ;\n\tMon, 21 Aug 2017 07:10:23 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753284AbdHTVKV (ORCPT );\n\tSun, 20 Aug 2017 17:10:21 -0400","from mx2.suse.de ([195.135.220.15]:45246 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1753234AbdHTVKU (ORCPT );\n\tSun, 20 Aug 2017 17:10:20 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id E6A4FAABA;\n\tSun, 20 Aug 2017 21:10:18 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","To":"Alexandre Belloni ","Cc":"Alessandro Zummo , linux-rtc@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tRoc He ,\n\t=?UTF-8?B?6JKL5Li955C0?= ","References":"<20170820013632.18375-1-afaerber@suse.de>\n\t<20170820013632.18375-3-afaerber@suse.de>\n\t<20170820083219.siez3rl2hbjgjgk5@piout.net>","From":"=?UTF-8?Q?Andreas_F=c3=a4rber?= ","Organization":"SUSE Linux GmbH","Message-ID":"<3307d3ed-c8e0-a7b5-f0ee-d401f4b12a90@suse.de>","Date":"Sun, 20 Aug 2017 23:10:16 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170820083219.siez3rl2hbjgjgk5@piout.net>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1752267,"web_url":"http://patchwork.ozlabs.org/comment/1752267/","msgid":"<5b607c23-92f9-a466-fd1c-92189177f174@suse.de>","list_archive_url":null,"date":"2017-08-20T21:12:05","subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","submitter":{"id":9542,"url":"http://patchwork.ozlabs.org/api/people/9542/","name":"Andreas Färber","email":"afaerber@suse.de"},"content":"Am 20.08.2017 um 17:40 schrieb Andrew Lunn:\n>> +static void rtd119x_rtc_set_enabled(struct device *dev, bool enable)\n>> +{\n>> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n>> +\tu32 val;\n>> +\n>> +\tval = readl_relaxed(data->base + RTD_RTCEN);\n>> +\tdev_info(dev, \"%s: rtcen = 0x%08x\\n\", __func__, val);\n> \n> dev_dbg()?\n> \n>> +static int rtd119x_rtc_open(struct device *dev)\n>> +{\n>> +\tstruct rtd119x_rtc *data = dev_get_drvdata(dev);\n>> +\tu32 val;\n>> +\tint ret;\n>> +\n>> +\tret = clk_prepare_enable(data->clk);\n>> +\tif (ret)\n>> +\t\treturn ret;\n>> +\n>> +\tval = readl_relaxed(data->base + RTD_RTCACR);\n>> +\tdev_info(dev, \"rtcacr = 0x%08x\\n\", val);\n> \n> dev_dbg()?\n\nYeah, either that or dropping them altogether.\n\nThanks,\nAndreas","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xb8f20JQrz9t2V\n\tfor ;\n\tMon, 21 Aug 2017 07:12:09 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753351AbdHTVMI (ORCPT );\n\tSun, 20 Aug 2017 17:12:08 -0400","from mx2.suse.de ([195.135.220.15]:45325 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1753284AbdHTVMI (ORCPT );\n\tSun, 20 Aug 2017 17:12:08 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 09702AAD0;\n\tSun, 20 Aug 2017 21:12:07 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","To":"Andrew Lunn ","Cc":"Alessandro Zummo ,\n\tAlexandre Belloni ,\n\tlinux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\t????????? ,\n\tlinux-kernel@vger.kernel.org, Roc He ","References":"<20170820013632.18375-1-afaerber@suse.de>\n\t<20170820013632.18375-3-afaerber@suse.de>\n\t<20170820154017.GD24150@lunn.ch>","From":"=?UTF-8?Q?Andreas_F=c3=a4rber?= ","Organization":"SUSE Linux GmbH","Message-ID":"<5b607c23-92f9-a466-fd1c-92189177f174@suse.de>","Date":"Sun, 20 Aug 2017 23:12:05 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170820154017.GD24150@lunn.ch>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"}},{"id":1754618,"web_url":"http://patchwork.ozlabs.org/comment/1754618/","msgid":"<20170823011822.adda7emuz3eig3yt@piout.net>","list_archive_url":null,"date":"2017-08-23T01:18:22","subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","submitter":{"id":26276,"url":"http://patchwork.ozlabs.org/api/people/26276/","name":"Alexandre Belloni","email":"alexandre.belloni@free-electrons.com"},"content":"On 20/08/2017 at 23:10:16 +0200, Andreas Färber wrote:\n> > Is RTD_RTCDATE_HIGH latched when RTD_RTCDATE_LOW is read?\n> \n> I do not have an answer to that.\n> \n> > If this is not\n> > the case, you probably want to handle a possible update between both\n> > readl_relaxed.\n> \n> Are you proposing to disable the RTC while reading the registers, or\n> something related to my choice of _relaxed? (it follows an explanation\n> by Marc Zyngier on the irq mux series) Inconsistencies might not be\n> limited to the date.\n> \n\nA simple way to be sure would be to read RTD_RTCSEC first, then the\nother registers and check RTD_RTCSEC didn't change. This will ensure the\nother registers have not be updated in the meantime (unless it takes\none minute but I doubt this is the case). if RTD_RTCSEC changed, then\nyou can start over.\n\n> >> +\tt = mktime64(data->base_year, 1, 1, 0, 0, 0);\n> >> +\tt += day * 24 * 60 * 60;\n> >> +\trtc_time64_to_tm(t, tm);\n> \n> BTW is there any more efficient way to get from year+days to\n> day/month/year without going via seconds?\n> \n\nCompletely untested:\n\nfor (y = data->base_year; (day - (365 + is_leap_year(y))) > 0; y++)\n\tday -= (365 + is_leap_year(y));\n\nfor (m = 0; (day - rtc_month_days(m, y)) > 0; m++)\n\tday -= rtc_month_days(m, y);\n\n\n> >> +\n> >> +\trtd119x_rtc_set_enabled(dev, true);\n> >> +\n> > \n> > This is certainly not what you want. The RTC device is usually not\n> > opened so enabling the RTC when open and disabling it when closed will\n> > not work on most systems. This is probably true for the clock too. i.e\n> > what you do here should be done in probe.\n> \n> I did test the probe path to work, but I can change it again. The\n> downstream code had it in probe, but looking at rtc_class_ops I saw\n> those hooks and thought they'd serve a purpose - what are they for then?\n> (Any chance you can improve the documentation comments to avoid such\n> misunderstandings? :))\n> \n\nThey are (were) used only when the rtc character device (/dev/rtcx) is\nopened/released but the cdev interface is one of many interfaces to the\nRTC sod it doesn't make sense to do something only in that case\n(especially requesting IRQs).\n\nTo solve your concern, this is what I'm going to apply after fixing the\ntwo remaining uses of .release:\n\nhttp://patchwork.ozlabs.org/patch/804707/","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-rtc-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xcV1G4Yfrz9s0Z\n\tfor ;\n\tWed, 23 Aug 2017 11:18:26 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752822AbdHWBSZ (ORCPT );\n\tTue, 22 Aug 2017 21:18:25 -0400","from mail.free-electrons.com ([62.4.15.54]:49063 \"EHLO\n\tmail.free-electrons.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752749AbdHWBSY (ORCPT\n\t); Tue, 22 Aug 2017 21:18:24 -0400","by mail.free-electrons.com (Postfix, from userid 110)\n\tid 88FCF20A22; Wed, 23 Aug 2017 03:18:22 +0200 (CEST)","from localhost (unknown [88.191.26.124])\n\tby mail.free-electrons.com (Postfix) with ESMTPSA id 5912B20A1D;\n\tWed, 23 Aug 2017 03:18:22 +0200 (CEST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on\n\tmail.free-electrons.com","X-Spam-Level":"","X-Spam-Status":"No, score=-1.0 required=5.0 tests=ALL_TRUSTED,SHORTCIRCUIT\n\tshortcircuit=ham autolearn=disabled version=3.4.0","Date":"Wed, 23 Aug 2017 03:18:22 +0200","From":"Alexandre Belloni ","To":"Andreas =?iso-8859-1?Q?F=E4rber?= ","Cc":"Alessandro Zummo , linux-rtc@vger.kernel.org,\n\tlinux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,\n\tRoc He ,\n\t=?utf-8?B?6JKL5Li955C0?= ","Subject":"Re: [RFC 2/3] rtc: Add Realtek RTD1295","Message-ID":"<20170823011822.adda7emuz3eig3yt@piout.net>","References":"<20170820013632.18375-1-afaerber@suse.de>\n\t<20170820013632.18375-3-afaerber@suse.de>\n\t<20170820083219.siez3rl2hbjgjgk5@piout.net>\n\t<3307d3ed-c8e0-a7b5-f0ee-d401f4b12a90@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<3307d3ed-c8e0-a7b5-f0ee-d401f4b12a90@suse.de>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"linux-rtc-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-rtc@vger.kernel.org"}}]