2015-10-29 16:47 GMT+09:00 Alim Akhtar <alim.akhtar@xxxxxxxxxxx>: > 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 <alexandre.belloni@xxxxxxxxxxxxxxxxxx> >>> Signed-off-by: Alim Akhtar <alim.akhtar@xxxxxxxxxxx> >>> --- >>> 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. > > 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" <update the a new date> > Thu Oct 29 14:10:40 UTC 2015 > # > # hwclock --systohc <copy system clock to hardware clock/rtc> > # hwclock <read back 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" <update the a new date> > Thu Oct 29 14:30:10 UTC 2015 > # date > Thu Oct 29 14:30:12 UTC 2015 > # hwclock --systohc <copy system clock to hardware clock/rtc> > # hwclock <read back 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 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? > >>> 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? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html