From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756975AbbJ2LTY (ORCPT ); Thu, 29 Oct 2015 07:19:24 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:40658 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756958AbbJ2LTV (ORCPT ); Thu, 29 Oct 2015 07:19:21 -0400 X-AuditID: cbfee690-f794e6d0000014de-4e-563200b7a893 Message-id: <563200F2.4010202@samsung.com> Date: Thu, 29 Oct 2015 16:50:18 +0530 From: Alim Akhtar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-version: 1.0 To: Krzysztof Kozlowski Cc: lee.jones@linaro.org, broonie@kernel.org, linux-samsung-soc@vger.kernel.org, rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org, Alexandre Belloni Subject: Re: [PATCH v4 4/4] drivers/rtc/rtc-s5m.c: add support for S2MPS15 RTC References: <1446094723-6212-1-git-send-email-alim.akhtar@samsung.com> <1446094723-6212-5-git-send-email-alim.akhtar@samsung.com> <5631B487.3050501@samsung.com> <5631CF0A.90109@samsung.com> In-reply-to: Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrKIsWRmVeSWpSXmKPExsWyRsSkSnc7g1GYwYl+OYuOa4uZLKY+fMJm 8fqFocX9r0cZLS7vmsNmMeP8PiaL/Z0djA7sHk82XWT02DPxJJvHplWdbB53ru1h8+jbsorR 4/MmuQC2KC6blNSczLLUIn27BK6M3W/OsBccUas4vPIuWwPjTNkuRg4OCQETiSXPlLoYOYFM MYkL99azdTFycQgJrGCU+P1gJjNEwkTi97H9LBCJWYwSb7+dZQJJCAk8YJQ4P9cWxOYV0JJY 0vSZHcRmEVCVmP51KhuIzSagLXF3+hYmkGWiAhESjy8IQZQLSvyYfI8FxBYRMJQ4uHs7E8h8 ZoGTjBJL9iwFSwgL+EkcmtbBDrGrg0li1dJyEJtTIFhi612IG5gFzCQetaxjhrDlJTaveQt1 9C12iT1/TSDuEZD4NvkQC8TDshKbDkCVSEocXHGDZQKj2CwkJ81CMnUWkqkLGJlXMYqmFiQX FCelF5noFSfmFpfmpesl5+duYgTG3ul/zybsYLx3wPoQowAHoxIP7wIjwzAh1sSy4srcQ4ym QFdMZJYSTc4HRnheSbyhsZmRhamJqbGRuaWZkjjva6mfwUIC6YklqdmpqQWpRfFFpTmpxYcY mTg4pRoYZ4S//db80i/M5umbilll0udrlU5zvnfa+3/Hgc74Xq/I42rshVZODNP5hXZbNGrb lQocjN884/Tz1YwfPr2VMe8Iz3A6EfXT54jGlW2JVbt1nmSrzc9hX1w6OfRjY65A0pXEPae9 lLa07tLd/rN89/yAxZdElumcWXTr/q23/fsnzSxj4JtxQYmlOCPRUIu5qDgRAEfgQpu4AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrCIsWRmVeSWpSXmKPExsVy+t9jQd3tDEZhBr8miFh0XFvMZDH14RM2 i9cvDC3ufz3KaHF51xw2ixnn9zFZ7O/sYHRg93iy6SKjx56JJ9k8Nq3qZPO4c20Pm0ffllWM Hp83yQWwRTUw2mSkJqakFimk5iXnp2TmpdsqeQfHO8ebmhkY6hpaWpgrKeQl5qbaKrn4BOi6 ZeYA3aKkUJaYUwoUCkgsLlbSt8M0ITTETdcCpjFC1zckCK7HyAANJKxhzNj95gx7wRG1isMr 77I1MM6U7WLk5JAQMJH4fWw/C4QtJnHh3nq2LkYuDiGBWYwSb7+dZQJJCAk8YJQ4P9cWxOYV 0JJY0vSZHcRmEVCVmP51KhuIzSagLXF3+hageg4OUYEIiccXhCDKBSV+TL4HNl9EwFDi4O7t TCDzmQVOMkos2bMULCEs4CdxaFoHO8SuDiaJVUvLQWxOgWCJrXchbmAWMJN41LKOGcKWl9i8 5i3zBEagKxF2zEJSNgtJ2QJG5lWMEqkFyQXFSem5hnmp5XrFibnFpXnpesn5uZsYwTH+TGoH 48Fd7ocYBTgYlXh4FxgZhgmxJpYVV+YeYpTgYFYS4RW6ARTiTUmsrEotyo8vKs1JLT7EaAoM hInMUqLJ+cD0k1cSb2hsYm5qbGppYmFiZqkkznshQyNMSCA9sSQ1OzW1ILUIpo+Jg1OqgVFm 4Z73bV9rrwn6tDfv+JtxlWff0/jNKccMa9l5jBtfpHwz1TQ+v0lPW09RcLeo8nvF+CSttZK/ fpzgSts/07l/p+LRDAfuC7ubp/KJWhi/1FJ/8kG15cHBjj1Gag4/s+Y6SH+IcpSq5vnGuPHM xX1Lnq1iTlhr///htYLt2pu3HjhbVn1+0zElluKMREMt5qLiRACV18IWBwMAAA== 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 On 10/29/2015 04:33 PM, Krzysztof Kozlowski wrote: > 2015-10-29 16:47 GMT+09:00 Alim Akhtar : >> Hello Krzysztof, >> >> >> On 10/29/2015 11:24 AM, Krzysztof Kozlowski wrote: >>> >>> On 29.10.2015 13:58, Alim Akhtar wrote: >>>> >>>> RTC found in s2mps15 is almost same as one found on s2mps14/13 >>>> with few differences in RTC_UPDATE register fields, like >>>> bit fields are changed for WUDR and AUDR. >>>> This patch add required changes to enable s2mps15 rtc timer. >>>> >>>> Cc: Alexandre Belloni >>>> Signed-off-by: Alim Akhtar >>>> --- >>>> drivers/rtc/rtc-s5m.c | 20 ++++++++++++++++++-- >>>> include/linux/mfd/samsung/rtc.h | 4 ++++ >>>> 2 files changed, 22 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c >>>> index f2504b4eef34..0d106a99958f 100644 >>>> --- a/drivers/rtc/rtc-s5m.c >>>> +++ b/drivers/rtc/rtc-s5m.c >>>> @@ -188,6 +188,7 @@ static inline int >>>> s5m_check_peding_alarm_interrupt(struct s5m_rtc_info *info, >>>> ret = regmap_read(info->regmap, S5M_RTC_STATUS, &val); >>>> val &= S5M_ALARM0_STATUS; >>>> break; >>>> + case S2MPS15X: >>>> case S2MPS14X: >>>> case S2MPS13X: >>>> ret = regmap_read(info->s5m87xx->regmap_pmic, >>>> S2MPS14_REG_ST2, >>>> @@ -223,6 +224,9 @@ static inline int s5m8767_rtc_set_time_reg(struct >>>> s5m_rtc_info *info) >>>> if (info->device_type == S5M8763X || info->device_type == >>>> S5M8767X) >>>> data |= S5M_RTC_TIME_EN_MASK; >>>> >>>> + if (info->device_type == S2MPS15X) >>>> + data |= S2MPS15_RTC_WUDR_MASK; >>>> + >>> >>> >>> >>> You are setting here bit 1 and bit 4. However vendor code sets only bit >>> 4 (called WUDR there). That's confusing. Why the difference? Or maybe I >>> am looking at wrong vendor tree (SM-G920F)? >>> >> Sorry, I don't have access to vendor code that you are looking into, so >> won't be able to comment on that. > > Everyone have the access. It is on opensource.samsung.com, as required by GPL. > Ok, I didn't check there. >> >> I am testing this patch before sending them, what I have found is if you >> don't update WUDR the time does not changes in rtc. >> e.g. >> if you don't do above changes then you will see below: >> ----- >> # date --set="Oct 29 14:10:40 2015" >> Thu Oct 29 14:10:40 UTC 2015 >> # >> # hwclock --systohc >> # hwclock >> Thu Oct 29 12:52:32 2015 .922889 seconds >> ---- >> which is not excepted. >> >> And with the above change I see: >> >> ---- >> # date --set="Oct 29 14:30:10 2015" >> Thu Oct 29 14:30:10 UTC 2015 >> # date >> Thu Oct 29 14:30:12 UTC 2015 >> # hwclock --systohc >> # hwclock >> Thu Oct 29 14:30:21 2015 .333006 seconds >> ---- >> >> Which is as expected. > > Okay, but I said that vendor is setting only WUDR which is bit 4. You bit:4 is AUDR (for alarm). not WUDR and bit: 1 is WDUR(for timer) > are setting bit 1 and bit 4. Your reply - about the need of setting > WUDR (bit 4 I guess) - does not explain my concerns. Do you have to > set bit 1? > Yes, I think we need to set both for timer and alarm. I have send you a snapshot of the rtc_update register in UM via internal email. PTAL. >> >>>> ret = regmap_write(info->regmap, info->regs->rtc_udr_update, >>>> data); >>>> if (ret < 0) { >>>> dev_err(info->dev, "failed to write update reg(%d)\n", >>>> ret); >>>> @@ -252,6 +256,9 @@ static inline int s5m8767_rtc_set_alarm_reg(struct >>>> s5m_rtc_info *info) >>>> case S5M8767X: >>>> data &= ~S5M_RTC_TIME_EN_MASK; >>>> break; >>>> + case S2MPS15X: >>>> + data |= S2MPS15_RTC_AUDR_MASK; >>>> + break; >>>> case S2MPS14X: >>>> data |= S2MPS_RTC_RUDR_MASK; >>>> break; >>> >>> >>> Another difference: you are setting here exactly the same values as >>> S2MPS13 - bit 1 and bit 4. However vendor code sets only bit 4 (called >>> WUDR there)? >>> >> No they are not, AUDR on s2mps13 is bit:1 where as it is bit:4 on s2mps15. >>> >>> What exactly is necessary to update alarm and time registers on S2MPS15? >>> >> As explained above in example, it is required to update the 'write alarm >> buffer(AUDR)' and 'write time buffer(WUDR)' bits in-order to get rtc >> working. > > For updating time and alarm registers? Or only for updating alarm registers? > example was only for the timer register. > Best regards, > Krzysztof >