linux-kernel - Re: [PATCH 4/5] rtc: s5m: Add the support for S2MPS13 RTC

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-id: <546A9D7D.7040207@samsung.com>
Date:	Tue, 18 Nov 2014 10:14:37 +0900
From:	Chanwoo Choi <cw00.choi@...sung.com>
To:	Krzysztof Kozłowski <k.kozlowski@...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

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.

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/