| Message ID | 1446094723-6212-5-git-send-email-alim.akhtar@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@free-electrons.com> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > drivers/rtc/rtc-s5m.c | 20 ++++++++++++++++++-- > include/linux/mfd/samsung/rtc.h | 4 ++++ > 2 files changed, 22 insertions(+), 2 deletions(-) Oh, this is totally different than previous patch and changelog does not mention these changes. Was the previous RTC patch tested? Here the AUDR and WUDR seem to be reversed... oh, that is strange. :) Best regards, Krzysztof > > 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; > + > 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; > @@ -275,7 +282,6 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info) > if (info->device_type == S2MPS13X) > regmap_update_bits(info->regmap, info->regs->rtc_udr_update, > S2MPS13_RTC_AUDR_MASK, 0); > - > return ret; > } > > @@ -317,7 +323,8 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) > u8 data[info->regs->regs_count]; > int ret; > > - if (info->device_type == S2MPS14X || info->device_type == S2MPS13X) { > + if (info->device_type == S2MPS15X || info->device_type == S2MPS14X || > + info->device_type == S2MPS13X) { > ret = regmap_update_bits(info->regmap, > info->regs->rtc_udr_update, > S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK); > @@ -339,6 +346,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode); > @@ -366,6 +374,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm) > s5m8763_tm_to_data(tm, data); > break; > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > ret = s5m8767_tm_to_data(tm, data); > @@ -414,6 +423,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode); > @@ -463,6 +473,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > for (i = 0; i < info->regs->regs_count; i++) > @@ -508,6 +519,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > data[RTC_SEC] |= ALARM_ENABLE_MASK; > @@ -548,6 +560,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_tm_to_data(&alrm->time, data); > @@ -631,6 +644,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) > ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2); > break; > > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); > @@ -679,6 +693,7 @@ static int s5m_rtc_probe(struct platform_device *pdev) > return -ENOMEM; > > switch (platform_get_device_id(pdev)->driver_data) { > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > regmap_cfg = &s2mps14_rtc_regmap_config; > @@ -805,6 +820,7 @@ static const struct platform_device_id s5m_rtc_id[] = { > { "s5m-rtc", S5M8767X }, > { "s2mps13-rtc", S2MPS13X }, > { "s2mps14-rtc", S2MPS14X }, > + { "s2mps15-rtc", S2MPS15X }, > { }, > }; > MODULE_DEVICE_TABLE(platform, s5m_rtc_id); > diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h > index 29c30ac36020..0be45fccc52b 100644 > --- a/include/linux/mfd/samsung/rtc.h > +++ b/include/linux/mfd/samsung/rtc.h > @@ -107,6 +107,10 @@ enum s2mps_rtc_reg { > #define S2MPS_RTC_WUDR_MASK (1 << S2MPS_RTC_WUDR_SHIFT) > #define S2MPS13_RTC_AUDR_SHIFT 1 > #define S2MPS13_RTC_AUDR_MASK (1 << S2MPS13_RTC_AUDR_SHIFT) > +#define S2MPS15_RTC_AUDR_SHIFT 4 > +#define S2MPS15_RTC_AUDR_MASK (1 << S2MPS15_RTC_AUDR_SHIFT) > +#define S2MPS15_RTC_WUDR_SHIFT 1 > +#define S2MPS15_RTC_WUDR_MASK (1 << S2MPS15_RTC_WUDR_SHIFT) > #define S2MPS_RTC_RUDR_SHIFT 0 > #define S2MPS_RTC_RUDR_MASK (1 << S2MPS_RTC_RUDR_SHIFT) > #define RTC_TCON_SHIFT 1 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/29/2015 10:37 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@free-electrons.com> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> drivers/rtc/rtc-s5m.c | 20 ++++++++++++++++++-- >> include/linux/mfd/samsung/rtc.h | 4 ++++ >> 2 files changed, 22 insertions(+), 2 deletions(-) > > > Oh, this is totally different than previous patch and changelog does not > mention these changes. > I am still learning to make thing more correct in first attempt, these input will help :-), thanks. > Was the previous RTC patch tested? Yes, it was tested, but the coverage was limited to wakealarm. Now I have tested wakealarm and setting hw clock date/time using hwclock utility, setting system time/date to rtc and visa-versa, both works as expected. > Here the AUDR and WUDR seem to be reversed... oh, that is strange. :) > Yah, indeed this was a surprised for me also, but this how it is implemented in s2mps15. > Best regards, > Krzysztof > >> >> 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; >> + >> 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; >> @@ -275,7 +282,6 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info) >> if (info->device_type == S2MPS13X) >> regmap_update_bits(info->regmap, info->regs->rtc_udr_update, >> S2MPS13_RTC_AUDR_MASK, 0); >> - >> return ret; >> } >> >> @@ -317,7 +323,8 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) >> u8 data[info->regs->regs_count]; >> int ret; >> >> - if (info->device_type == S2MPS14X || info->device_type == S2MPS13X) { >> + if (info->device_type == S2MPS15X || info->device_type == S2MPS14X || >> + info->device_type == S2MPS13X) { >> ret = regmap_update_bits(info->regmap, >> info->regs->rtc_udr_update, >> S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK); >> @@ -339,6 +346,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode); >> @@ -366,6 +374,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm) >> s5m8763_tm_to_data(tm, data); >> break; >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> ret = s5m8767_tm_to_data(tm, data); >> @@ -414,6 +423,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode); >> @@ -463,6 +473,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> for (i = 0; i < info->regs->regs_count; i++) >> @@ -508,6 +519,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> data[RTC_SEC] |= ALARM_ENABLE_MASK; >> @@ -548,6 +560,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_tm_to_data(&alrm->time, data); >> @@ -631,6 +644,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) >> ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2); >> break; >> >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); >> @@ -679,6 +693,7 @@ static int s5m_rtc_probe(struct platform_device *pdev) >> return -ENOMEM; >> >> switch (platform_get_device_id(pdev)->driver_data) { >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> regmap_cfg = &s2mps14_rtc_regmap_config; >> @@ -805,6 +820,7 @@ static const struct platform_device_id s5m_rtc_id[] = { >> { "s5m-rtc", S5M8767X }, >> { "s2mps13-rtc", S2MPS13X }, >> { "s2mps14-rtc", S2MPS14X }, >> + { "s2mps15-rtc", S2MPS15X }, >> { }, >> }; >> MODULE_DEVICE_TABLE(platform, s5m_rtc_id); >> diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h >> index 29c30ac36020..0be45fccc52b 100644 >> --- a/include/linux/mfd/samsung/rtc.h >> +++ b/include/linux/mfd/samsung/rtc.h >> @@ -107,6 +107,10 @@ enum s2mps_rtc_reg { >> #define S2MPS_RTC_WUDR_MASK (1 << S2MPS_RTC_WUDR_SHIFT) >> #define S2MPS13_RTC_AUDR_SHIFT 1 >> #define S2MPS13_RTC_AUDR_MASK (1 << S2MPS13_RTC_AUDR_SHIFT) >> +#define S2MPS15_RTC_AUDR_SHIFT 4 >> +#define S2MPS15_RTC_AUDR_MASK (1 << S2MPS15_RTC_AUDR_SHIFT) >> +#define S2MPS15_RTC_WUDR_SHIFT 1 >> +#define S2MPS15_RTC_WUDR_MASK (1 << S2MPS15_RTC_WUDR_SHIFT) >> #define S2MPS_RTC_RUDR_SHIFT 0 >> #define S2MPS_RTC_RUDR_MASK (1 << S2MPS_RTC_RUDR_SHIFT) >> #define RTC_TCON_SHIFT 1 >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@free-electrons.com> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> > --- > 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)? > 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)? What exactly is necessary to update alarm and time registers on S2MPS15? > @@ -275,7 +282,6 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info) > if (info->device_type == S2MPS13X) > regmap_update_bits(info->regmap, info->regs->rtc_udr_update, > S2MPS13_RTC_AUDR_MASK, 0); > - This is not related to the patch. Best regards, Krzysztof > return ret; > } > > @@ -317,7 +323,8 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) > u8 data[info->regs->regs_count]; > int ret; > > - if (info->device_type == S2MPS14X || info->device_type == S2MPS13X) { > + if (info->device_type == S2MPS15X || info->device_type == S2MPS14X || > + info->device_type == S2MPS13X) { > ret = regmap_update_bits(info->regmap, > info->regs->rtc_udr_update, > S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK); > @@ -339,6 +346,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode); > @@ -366,6 +374,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm) > s5m8763_tm_to_data(tm, data); > break; > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > ret = s5m8767_tm_to_data(tm, data); > @@ -414,6 +423,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode); > @@ -463,6 +473,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > for (i = 0; i < info->regs->regs_count; i++) > @@ -508,6 +519,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > data[RTC_SEC] |= ALARM_ENABLE_MASK; > @@ -548,6 +560,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > break; > > case S5M8767X: > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > s5m8767_tm_to_data(&alrm->time, data); > @@ -631,6 +644,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) > ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2); > break; > > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); > @@ -679,6 +693,7 @@ static int s5m_rtc_probe(struct platform_device *pdev) > return -ENOMEM; > > switch (platform_get_device_id(pdev)->driver_data) { > + case S2MPS15X: > case S2MPS14X: > case S2MPS13X: > regmap_cfg = &s2mps14_rtc_regmap_config; > @@ -805,6 +820,7 @@ static const struct platform_device_id s5m_rtc_id[] = { > { "s5m-rtc", S5M8767X }, > { "s2mps13-rtc", S2MPS13X }, > { "s2mps14-rtc", S2MPS14X }, > + { "s2mps15-rtc", S2MPS15X }, > { }, > }; > MODULE_DEVICE_TABLE(platform, s5m_rtc_id); > diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h > index 29c30ac36020..0be45fccc52b 100644 > --- a/include/linux/mfd/samsung/rtc.h > +++ b/include/linux/mfd/samsung/rtc.h > @@ -107,6 +107,10 @@ enum s2mps_rtc_reg { > #define S2MPS_RTC_WUDR_MASK (1 << S2MPS_RTC_WUDR_SHIFT) > #define S2MPS13_RTC_AUDR_SHIFT 1 > #define S2MPS13_RTC_AUDR_MASK (1 << S2MPS13_RTC_AUDR_SHIFT) > +#define S2MPS15_RTC_AUDR_SHIFT 4 > +#define S2MPS15_RTC_AUDR_MASK (1 << S2MPS15_RTC_AUDR_SHIFT) > +#define S2MPS15_RTC_WUDR_SHIFT 1 > +#define S2MPS15_RTC_WUDR_MASK (1 << S2MPS15_RTC_WUDR_SHIFT) > #define S2MPS_RTC_RUDR_SHIFT 0 > #define S2MPS_RTC_RUDR_MASK (1 << S2MPS_RTC_RUDR_SHIFT) > #define RTC_TCON_SHIFT 1 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@free-electrons.com> >> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> 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. 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. >> 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. >> @@ -275,7 +282,6 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info) >> if (info->device_type == S2MPS13X) >> regmap_update_bits(info->regmap, info->regs->rtc_udr_update, >> S2MPS13_RTC_AUDR_MASK, 0); >> - > > This is not related to the patch. > uhh...will change. > Best regards, > Krzysztof > Thanks again. > >> return ret; >> } >> >> @@ -317,7 +323,8 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) >> u8 data[info->regs->regs_count]; >> int ret; >> >> - if (info->device_type == S2MPS14X || info->device_type == S2MPS13X) { >> + if (info->device_type == S2MPS15X || info->device_type == S2MPS14X || >> + info->device_type == S2MPS13X) { >> ret = regmap_update_bits(info->regmap, >> info->regs->rtc_udr_update, >> S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK); >> @@ -339,6 +346,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode); >> @@ -366,6 +374,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm) >> s5m8763_tm_to_data(tm, data); >> break; >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> ret = s5m8767_tm_to_data(tm, data); >> @@ -414,6 +423,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode); >> @@ -463,6 +473,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> for (i = 0; i < info->regs->regs_count; i++) >> @@ -508,6 +519,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> data[RTC_SEC] |= ALARM_ENABLE_MASK; >> @@ -548,6 +560,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) >> break; >> >> case S5M8767X: >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> s5m8767_tm_to_data(&alrm->time, data); >> @@ -631,6 +644,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) >> ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2); >> break; >> >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); >> @@ -679,6 +693,7 @@ static int s5m_rtc_probe(struct platform_device *pdev) >> return -ENOMEM; >> >> switch (platform_get_device_id(pdev)->driver_data) { >> + case S2MPS15X: >> case S2MPS14X: >> case S2MPS13X: >> regmap_cfg = &s2mps14_rtc_regmap_config; >> @@ -805,6 +820,7 @@ static const struct platform_device_id s5m_rtc_id[] = { >> { "s5m-rtc", S5M8767X }, >> { "s2mps13-rtc", S2MPS13X }, >> { "s2mps14-rtc", S2MPS14X }, >> + { "s2mps15-rtc", S2MPS15X }, >> { }, >> }; >> MODULE_DEVICE_TABLE(platform, s5m_rtc_id); >> diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h >> index 29c30ac36020..0be45fccc52b 100644 >> --- a/include/linux/mfd/samsung/rtc.h >> +++ b/include/linux/mfd/samsung/rtc.h >> @@ -107,6 +107,10 @@ enum s2mps_rtc_reg { >> #define S2MPS_RTC_WUDR_MASK (1 << S2MPS_RTC_WUDR_SHIFT) >> #define S2MPS13_RTC_AUDR_SHIFT 1 >> #define S2MPS13_RTC_AUDR_MASK (1 << S2MPS13_RTC_AUDR_SHIFT) >> +#define S2MPS15_RTC_AUDR_SHIFT 4 >> +#define S2MPS15_RTC_AUDR_MASK (1 << S2MPS15_RTC_AUDR_SHIFT) >> +#define S2MPS15_RTC_WUDR_SHIFT 1 >> +#define S2MPS15_RTC_WUDR_MASK (1 << S2MPS15_RTC_WUDR_SHIFT) >> #define S2MPS_RTC_RUDR_SHIFT 0 >> #define S2MPS_RTC_RUDR_MASK (1 << S2MPS_RTC_RUDR_SHIFT) >> #define RTC_TCON_SHIFT 1 >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-10-29 16:47 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>: > 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@free-electrons.com> >>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/29/2015 04:33 PM, Krzysztof Kozlowski wrote: > 2015-10-29 16:47 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>: >> 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@free-electrons.com> >>>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> >>>> --- >>>> 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" <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 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-10-29 20:20 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>: >>> 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 > > bit:4 is AUDR (for alarm). not WUDR and bit: 1 is WDUR(for timer) Damn, the mismatch in names is confusing. The vendor sets WUDR which on S2MPS15 is bit 1. You are setting bit 1 (okay) and bit 4. This is different which makes me asking questions. >> >> 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. Okay, I will take a look when I get to the office tomorrow. > >>> >>>>> 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. > Anyway setting alarm above is strange. You are setting bit 4 twice. One as: data |= info->regs->rtc_udr_mask; and then: case S2MPS15X: data |= S2MPS15_RTC_AUDR_MASK; Yeah, I know this is misleading. Could you document expected behavior exactly somewhere in the code or in the commit message? "bit fields are changed for WUDR and AUDR." seems not enough. Instead I would be happy to see a statement: "to set time registers, AUDR and WUDR must be set, to set alarm .... etc. where AUDR and WUDR on S2MPS15 are reversed" Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof, On Thu, Oct 29, 2015 at 5:05 PM, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > 2015-10-29 20:20 GMT+09:00 Alim Akhtar <alim.akhtar@samsung.com>: >>>> 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 >> >> bit:4 is AUDR (for alarm). not WUDR and bit: 1 is WDUR(for timer) > > Damn, the mismatch in names is confusing. The vendor sets WUDR which > on S2MPS15 is bit 1. > > You are setting bit 1 (okay) and bit 4. This is different which makes > me asking questions. > hmm..ok let me have a relook. Perhaps I will add a switch case here to handle each device_type, keeping the current behavior as same. >>> >>> 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. > > Okay, I will take a look when I get to the office tomorrow. > >> >>>> >>>>>> 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. >> > > Anyway setting alarm above is strange. You are setting bit 4 twice. One as: > data |= info->regs->rtc_udr_mask; > and then: > case S2MPS15X: > data |= S2MPS15_RTC_AUDR_MASK; > ok, I can see what you are saying...good catch. > Yeah, I know this is misleading. Could you document expected behavior > exactly somewhere in the code or in the commit message? "bit fields > are changed for WUDR and AUDR." seems not enough. Instead I would be > happy to see a statement: "to set time registers, AUDR and WUDR must > be set, to set alarm .... etc. where AUDR and WUDR on S2MPS15 are > reversed" > This is a nice suggestions, will add. > Best regards, > Krzysztof > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + 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; @@ -275,7 +282,6 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info) if (info->device_type == S2MPS13X) regmap_update_bits(info->regmap, info->regs->rtc_udr_update, S2MPS13_RTC_AUDR_MASK, 0); - return ret; } @@ -317,7 +323,8 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) u8 data[info->regs->regs_count]; int ret; - if (info->device_type == S2MPS14X || info->device_type == S2MPS13X) { + if (info->device_type == S2MPS15X || info->device_type == S2MPS14X || + info->device_type == S2MPS13X) { ret = regmap_update_bits(info->regmap, info->regs->rtc_udr_update, S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK); @@ -339,6 +346,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm) break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode); @@ -366,6 +374,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm) s5m8763_tm_to_data(tm, data); break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: ret = s5m8767_tm_to_data(tm, data); @@ -414,6 +423,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode); @@ -463,6 +473,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info) break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: for (i = 0; i < info->regs->regs_count; i++) @@ -508,6 +519,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info) break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: data[RTC_SEC] |= ALARM_ENABLE_MASK; @@ -548,6 +560,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) break; case S5M8767X: + case S2MPS15X: case S2MPS14X: case S2MPS13X: s5m8767_tm_to_data(&alrm->time, data); @@ -631,6 +644,7 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) ret = regmap_raw_write(info->regmap, S5M_ALARM0_CONF, data, 2); break; + case S2MPS15X: case S2MPS14X: case S2MPS13X: data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); @@ -679,6 +693,7 @@ static int s5m_rtc_probe(struct platform_device *pdev) return -ENOMEM; switch (platform_get_device_id(pdev)->driver_data) { + case S2MPS15X: case S2MPS14X: case S2MPS13X: regmap_cfg = &s2mps14_rtc_regmap_config; @@ -805,6 +820,7 @@ static const struct platform_device_id s5m_rtc_id[] = { { "s5m-rtc", S5M8767X }, { "s2mps13-rtc", S2MPS13X }, { "s2mps14-rtc", S2MPS14X }, + { "s2mps15-rtc", S2MPS15X }, { }, }; MODULE_DEVICE_TABLE(platform, s5m_rtc_id); diff --git a/include/linux/mfd/samsung/rtc.h b/include/linux/mfd/samsung/rtc.h index 29c30ac36020..0be45fccc52b 100644 --- a/include/linux/mfd/samsung/rtc.h +++ b/include/linux/mfd/samsung/rtc.h @@ -107,6 +107,10 @@ enum s2mps_rtc_reg { #define S2MPS_RTC_WUDR_MASK (1 << S2MPS_RTC_WUDR_SHIFT) #define S2MPS13_RTC_AUDR_SHIFT 1 #define S2MPS13_RTC_AUDR_MASK (1 << S2MPS13_RTC_AUDR_SHIFT) +#define S2MPS15_RTC_AUDR_SHIFT 4 +#define S2MPS15_RTC_AUDR_MASK (1 << S2MPS15_RTC_AUDR_SHIFT) +#define S2MPS15_RTC_WUDR_SHIFT 1 +#define S2MPS15_RTC_WUDR_MASK (1 << S2MPS15_RTC_WUDR_SHIFT) #define S2MPS_RTC_RUDR_SHIFT 0 #define S2MPS_RTC_RUDR_MASK (1 << S2MPS_RTC_RUDR_SHIFT) #define RTC_TCON_SHIFT 1
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@free-electrons.com> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com> --- drivers/rtc/rtc-s5m.c | 20 ++++++++++++++++++-- include/linux/mfd/samsung/rtc.h | 4 ++++ 2 files changed, 22 insertions(+), 2 deletions(-)