Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752217AbbHUG6o (ORCPT ); Fri, 21 Aug 2015 02:58:44 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:54682 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751302AbbHUG6m (ORCPT ); Fri, 21 Aug 2015 02:58:42 -0400 X-AuditID: cbfee68f-f793b6d000005f66-3d-55d6cc1a95f9 Message-id: <55D6CC1C.9040402@samsung.com> Date: Fri, 21 Aug 2015 15:58:36 +0900 From: Joonyoung Shim User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-version: 1.0 To: Krzysztof Kozlowski , Alexandre Belloni Cc: rtc-linux@googlegroups.com, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, a.zummo@towertech.it, sbkim73@samsung.com Subject: Re: [PATCH] rtc: s5m: fix to update ctrl register References: <1439455764-23526-1-git-send-email-jy0922.shim@samsung.com> <20150820231551.GE3769@piout.net> <55D67487.3040503@samsung.com> <55D67838.6080902@samsung.com> <55D67D14.9060809@samsung.com> In-reply-to: <55D67D14.9060809@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrMIsWRmVeSWpSXmKPExsWyRsSkSFfqzLVQg8tnpCyWXLzKbtFxbTGT xesXhhaXd81hs5hxfh+Txf7ODkaLiyu+MFks2PiI0YHD48mmi4weeyaeZPPo27KK0WP6vJ9M Hp83yQWwRnHZpKTmZJalFunbJXBlfPoyg6XgrVzFvFNTGRsYX0l0MXJySAiYSDRv+8UIYYtJ XLi3nq2LkYtDSGAFo0TH6qPMMEXPdy9hgkgsZZRYvuQ0E0hCSOABo8SVU+kgNq+AlkTLpUdg cRYBVYnGlw/YQGw2AT2JO9uOg8VFBcIkzszoYIGoF5T4MfkekM3BISKQK/F/cSjIfGaBZYwS Kw+2soPEhQWsJD6czoBYdYpR4s46aRCbU0Bb4uzc10wgJcxA4+9f1AIJMwvIS2xe8xbq5Fvs Ep1dWRDXCEh8m3wIbJOEgKzEpgNQJZISB1fcYJnAKDYLyT2zEIbOQjJ0ASPzKkbR1ILkguKk 9CJjveLE3OLSvHS95PzcTYzAuDv971n/Dsa7B6wPMQpwMCrx8M6IvBYqxJpYVlyZe4jRFOiI icxSosn5wOjOK4k3NDYzsjA1MTU2Mrc0UxLnXSj1M1hIID2xJDU7NbUgtSi+qDQntfgQIxMH p1QDY/YXSeOLWSvLlrR62ypUOdyxnNKc9fH+xKzcVUXF385+7d/lH/ja2yqa8+P7ORJb/b6b cn6U6VtyeelRvZ0f7RqeF7z8vOl++3ehOmGLNYkXp73tl/5xJdEy7Q9T7l9Pt2MPf2zeuinP 1vB6+m67zIzqu3MrFI4yKmXL56/+zbQk8nRuX7v3FCWW4oxEQy3mouJEAPh2bQy2AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrIIsWRmVeSWpSXmKPExsVy+t9jAV2pM9dCDfY+EbFYcvEqu0XHtcVM Fq9fGFpc3jWHzWLG+X1MFvs7OxgtLq74wmSxYOMjRgcOjyebLjJ67Jl4ks2jb8sqRo/p834y eXzeJBfAGtXAaJORmpiSWqSQmpecn5KZl26r5B0c7xxvamZgqGtoaWGupJCXmJtqq+TiE6Dr lpkDdIuSQlliTilQKCCxuFhJ3w7ThNAQN10LmMYIXd+QILgeIwM0kLCGMePTlxksBW/lKuad msrYwPhKoouRk0NCwETi+e4lTBC2mMSFe+vZuhi5OIQEljJKLF9yGiwhJPCAUeLKqXQQm1dA S6Ll0iOwOIuAqkTjywdsIDabgJ7EnW3HweKiAmESZ2Z0sEDUC0r8mHwPyObgEBHIlfi/OBRk PrPAMkaJlQdb2UHiwgJWEh9OZ0CsOsUocWedNIjNKaAtcXbuayaQEmag8fcvaoGEmQXkJTav ecs8gVFgFpIFsxCqZiGpWsDIvIpRIrUguaA4KT3XMC+1XK84Mbe4NC9dLzk/dxMjOLqfSe1g PLjL/RCjAAejEg/vjMhroUKsiWXFlbmHGCU4mJVEePevAQrxpiRWVqUW5ccXleakFh9iNAWG wERmKdHkfGDiySuJNzQ2MTOyNDI3tDAyNlcS55XdsDlUSCA9sSQ1OzW1ILUIpo+Jg1OqgXHH sfiLmtNcpV60ODIn8sjJvDC6PqPhfOjv3XNzXzwuiFqzLFpvR88k1/0XYm7bry1l5OhT2K+S e+u63A4B8037pgfutN4joH4sXfaX111pFp2wxfJJwnPmdnCbaiVtFeaZb3QtJvikR+6lTZ0+ evEqIv90t7KcXvGIWe/0DSm+FdZL66e90lNiKc5INNRiLipOBAAYqAqhBAMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4070 Lines: 129 On 08/21/2015 10:21 AM, Krzysztof Kozlowski wrote: > On 21.08.2015 10:00, Joonyoung Shim wrote: >> On 08/21/2015 09:44 AM, Krzysztof Kozlowski wrote: >>> On 21.08.2015 08:15, Alexandre Belloni wrote: >>>> Hi, >>>> >>>> On 13/08/2015 at 17:49:24 +0900, Joonyoung Shim wrote : >>>>> According to datasheet, the S2MPS13X and S2MPS14X should update write >>>>> buffer via setting WUDR bit to high after ctrl register is updated. >>>>> >>>>> If not, ALARM interrupt of rtc-s5m doesn't happen first time when i use >>>>> tools/testing/selftests/timers/rtctest.c test program and hour format is >>>>> used to 12 hour mode in Odroid-XU3 board. >>>>> >>>> >>>> >From what I understood, I should expect a v2 of tihat patch also setting >>>> RUDR, is that right? OR would you prefer that I apply that one and then >>>> fix RUDR in a following patch? >>> >>> Right, I would expect that as well... or a comment if this is not needed. >>> >> >> Hmm, the driver only writes control register now, so i don't feel the >> need of patch setting RUDR for control register. > > Yes, you're right. There is only regmap_write() (not > remap_update_bits()) so your patch is completely fine. Thanks for > explanation. > > Reviewed-by: Krzysztof Kozlowski > Thanks for review. I found one more issue, the RTC doesn't keep time on Odroid-XU3 board when i turn on board after power off even if RTC battery is connected. A difference with RTC driver of hardkernel kernel is that it sets not only WUDR bit but also RUDR bit to high at the same time after RTC_CTRL register is written. It's same with condition of only writing ALARM registers like below description. from S2MPS14 datasheet: "3. For write only Alarm 0&1 Registers (0x0B~0x18), set WUDR & RUDR bits to high." So i tried it and it works well with keeping time. I'm not sure RTC of S2MPS13 type also has a similar issue because it differs a little bit. from S2MPS13 datasheet: "3. For write only Alarm 0&1 Registers (0x0B~0x18), set WUDR & A_UDR bits to high." If S2MPS13 also has same issue, we can fix the problem via just below patch. diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c index 8c70d78..bb8e888 100644 --- a/drivers/rtc/rtc-s5m.c +++ b/drivers/rtc/rtc-s5m.c @@ -635,6 +635,10 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) case S2MPS13X: data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); ret = regmap_write(info->regmap, info->regs->ctrl, data[0]); + if (ret < 0) + break; + + ret = s5m8767_rtc_set_alarm_reg(info); break; default: But i can't find any reasonable reason about this fix from datasheet, Thanks. > Best regards, > Krzysztof > >> >>> Best regards, >>> Krzysztof >>> >>> >>>> >>>>> Signed-off-by: Joonyoung Shim >>>>> Cc: >>>> >>>> can you update the stable tag with the kernel version introducing the >>>> issue? >> >> Sure, i think it should be v3.16. >> >>>> >>>>> --- >>>>> drivers/rtc/rtc-s5m.c | 12 ++++++++++++ >>>>> 1 file changed, 12 insertions(+) >>>> >>>>> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c >>>>> index 8c70d78..03828bb 100644 >>>>> --- a/drivers/rtc/rtc-s5m.c >>>>> +++ b/drivers/rtc/rtc-s5m.c >>>>> @@ -635,6 +635,18 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) >>>>> case S2MPS13X: >>>>> data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); >>>>> ret = regmap_write(info->regmap, info->regs->ctrl, data[0]); >>>>> + if (ret < 0) >>>>> + break; >>>>> + >>>>> + ret = regmap_update_bits(info->regmap, >>>>> + info->regs->rtc_udr_update, >>>>> + info->regs->rtc_udr_mask, >>>>> + info->regs->rtc_udr_mask); >>>> >>>> Very small indentation issue here, it should be aligned with the open >>>> parenthesis. >> >> OK, i will. >> >> Thanks. >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/