| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 18 Nov 2014 11:01:53 +0100
From: Krzysztof Kozlowski <k.kozlowski@...sung.com>
To: Chanwoo Choi <cw00.choi@...sung.com>
Cc: lee.jones@...aro.org, broonie@...nel.org, mturquette@...aro.org,
a.zummo@...ertech.it, lgirdwood@...il.com, sbkim73@...sung.com,
sameo@...ux.intel.com, geunsik.lim@...sung.com,
inki.dae@...sung.com, kyungmin.park@...sung.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 4/5] rtc: s5m: Add the support for S2MPS13 RTC
On wto, 2014-11-18 at 10:14 +0900, Chanwoo Choi wrote:
> Hi Krzysztof,
>
> On 11/17/2014 07:19 PM, Krzysztof Kozłowski wrote:
> > On 17.11.2014 01:42, Chanwoo Choi wrote:
> >> This patch adds only the compatible string for S2MPS13 clock which is identical
> >> with S2MPS14 clock driver.
> >>
> >> Cc: Alessandro Zummo <a.zummo@...ertech.it>
> >> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> >> ---
> >> drivers/rtc/rtc-s5m.c | 15 ++++++++++++++-
> >> 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > The RTC block looks exactly the same as S2MPS14. I wonder can S2MPS13 be
> > supported without adding all "+ case S2MPS13X:" here? Maybe pass the
> > same ID in s5m_rtc_id? This would reduce number of changes to only one line.
>
> I tried to use S2MPS14x type without adding S2MPS13X.
> But, there are two method to probe RTC driver probe.
> First, as you comment, use s5m_rtc_id (in drivers/rtc/rtc-s5m.c) to probe S2MPS13 RTC.
> Second, register mfd devcies for s2mps13 RTC(in drivers/mfd/sec-core.c) to probe S2MPS13 RTC.
>
> We could support S2MPS13 RTC to modify s5m_rtc_id table in first case as you comment,
> But, We could not support S2MPS13 RTC in second case because S2MPS13 is different from S2MPS14
> and S2MPS13 must have other type form existing S2MPS14x type.
OK, I'm fine with that. The patch looks good so:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
Best regards,
Krzysztof
>
> Best Regards,
> Chanwoo Choi
>
> >
> > Best regards,
> > Krzysztof
> >
> >
> >>
> >> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
> >> index 8754c33..e5fdfd3 100644
> >> --- a/drivers/rtc/rtc-s5m.c
> >> +++ b/drivers/rtc/rtc-s5m.c
> >> @@ -191,6 +191,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 S2MPS13X:
> >> case S2MPS14X:
> >> ret = regmap_read(info->s5m87xx->regmap_pmic, S2MPS14_REG_ST2,
> >> &val);
> >> @@ -254,6 +255,7 @@ static inline int s5m8767_rtc_set_alarm_reg(struct s5m_rtc_info *info)
> >> case S5M8767X:
> >> data &= ~S5M_RTC_TIME_EN_MASK;
> >> break;
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> data |= S2MPS_RTC_RUDR_MASK;
> >> break;
> >> @@ -311,7 +313,9 @@ 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) {
> >> + switch (info->device_type) {
> >> + case S2MPS13X:
> >> + case S2MPS14X:
> >> ret = regmap_update_bits(info->regmap,
> >> info->regs->rtc_udr_update,
> >> S2MPS_RTC_RUDR_MASK, S2MPS_RTC_RUDR_MASK);
> >> @@ -333,6 +337,7 @@ static int s5m_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >> break;
> >>
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> s5m8767_data_to_tm(data, tm, info->rtc_24hr_mode);
> >> break;
> >> @@ -359,6 +364,7 @@ static int s5m_rtc_set_time(struct device *dev, struct rtc_time *tm)
> >> s5m8763_tm_to_data(tm, data);
> >> break;
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> ret = s5m8767_tm_to_data(tm, data);
> >> break;
> >> @@ -406,6 +412,7 @@ static int s5m_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> >> break;
> >>
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> s5m8767_data_to_tm(data, &alrm->time, info->rtc_24hr_mode);
> >> alrm->enabled = 0;
> >> @@ -454,6 +461,7 @@ static int s5m_rtc_stop_alarm(struct s5m_rtc_info *info)
> >> break;
> >>
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> for (i = 0; i < info->regs->regs_count; i++)
> >> data[i] &= ~ALARM_ENABLE_MASK;
> >> @@ -498,6 +506,7 @@ static int s5m_rtc_start_alarm(struct s5m_rtc_info *info)
> >> break;
> >>
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> data[RTC_SEC] |= ALARM_ENABLE_MASK;
> >> data[RTC_MIN] |= ALARM_ENABLE_MASK;
> >> @@ -537,6 +546,7 @@ static int s5m_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> >> break;
> >>
> >> case S5M8767X:
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> s5m8767_tm_to_data(&alrm->time, data);
> >> break;
> >> @@ -641,6 +651,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 S2MPS13X:
> >> case S2MPS14X:
> >> data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT);
> >> ret = regmap_write(info->regmap, info->regs->ctrl, data[0]);
> >> @@ -678,6 +689,7 @@ static int s5m_rtc_probe(struct platform_device *pdev)
> >> return -ENOMEM;
> >>
> >> switch (pdata->device_type) {
> >> + case S2MPS13X:
> >> case S2MPS14X:
> >> regmap_cfg = &s2mps14_rtc_regmap_config;
> >> info->regs = &s2mps_rtc_regs;
> >> @@ -831,6 +843,7 @@ static SIMPLE_DEV_PM_OPS(s5m_rtc_pm_ops, s5m_rtc_suspend, s5m_rtc_resume);
> >>
> >> static const struct platform_device_id s5m_rtc_id[] = {
> >> { "s5m-rtc", S5M8767X },
> >> + { "s2mps13-rtc", S2MPS13X },
> >> { "s2mps14-rtc", S2MPS14X },
> >> };
> >>
> >>
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists