On 08/21/2015 10:21 AM, Krzysztof Kozlowski wrote: > On 21.08.2015 10:00, Joonyoung Shim wrote: >> On 08/21/2015 09:44 AM, Krzysztof Kozlowski wrote: >>> On 21.08.2015 08:15, Alexandre Belloni wrote: >>>> Hi, >>>> >>>> On 13/08/2015 at 17:49:24 +0900, Joonyoung Shim wrote : >>>>> According to datasheet, the S2MPS13X and S2MPS14X should update write >>>>> buffer via setting WUDR bit to high after ctrl register is updated. >>>>> >>>>> If not, ALARM interrupt of rtc-s5m doesn't happen first time when i use >>>>> tools/testing/selftests/timers/rtctest.c test program and hour format is >>>>> used to 12 hour mode in Odroid-XU3 board. >>>>> >>>> >>>> >From what I understood, I should expect a v2 of tihat patch also setting >>>> RUDR, is that right? OR would you prefer that I apply that one and then >>>> fix RUDR in a following patch? >>> >>> Right, I would expect that as well... or a comment if this is not needed. >>> >> >> Hmm, the driver only writes control register now, so i don't feel the >> need of patch setting RUDR for control register. > > Yes, you're right. There is only regmap_write() (not > remap_update_bits()) so your patch is completely fine. Thanks for > explanation. > > Reviewed-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> > Thanks for review. I found one more issue, the RTC doesn't keep time on Odroid-XU3 board when i turn on board after power off even if RTC battery is connected. A difference with RTC driver of hardkernel kernel is that it sets not only WUDR bit but also RUDR bit to high at the same time after RTC_CTRL register is written. It's same with condition of only writing ALARM registers like below description. from S2MPS14 datasheet: "3. For write only Alarm 0&1 Registers (0x0B~0x18), set WUDR & RUDR bits to high." So i tried it and it works well with keeping time. I'm not sure RTC of S2MPS13 type also has a similar issue because it differs a little bit. from S2MPS13 datasheet: "3. For write only Alarm 0&1 Registers (0x0B~0x18), set WUDR & A_UDR bits to high." If S2MPS13 also has same issue, we can fix the problem via just below patch. diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c index 8c70d78..bb8e888 100644 --- a/drivers/rtc/rtc-s5m.c +++ b/drivers/rtc/rtc-s5m.c @@ -635,6 +635,10 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) case S2MPS13X: data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); ret = regmap_write(info->regmap, info->regs->ctrl, data[0]); + if (ret < 0) + break; + + ret = s5m8767_rtc_set_alarm_reg(info); break; default: But i can't find any reasonable reason about this fix from datasheet, Thanks. > Best regards, > Krzysztof > >> >>> Best regards, >>> Krzysztof >>> >>> >>>> >>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> >>>>> Cc: <stable@xxxxxxxxxxxxxxx> >>>> >>>> can you update the stable tag with the kernel version introducing the >>>> issue? >> >> Sure, i think it should be v3.16. >> >>>> >>>>> --- >>>>> drivers/rtc/rtc-s5m.c | 12 ++++++++++++ >>>>> 1 file changed, 12 insertions(+) >>>> >>>>> diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c >>>>> index 8c70d78..03828bb 100644 >>>>> --- a/drivers/rtc/rtc-s5m.c >>>>> +++ b/drivers/rtc/rtc-s5m.c >>>>> @@ -635,6 +635,18 @@ static int s5m8767_rtc_init_reg(struct s5m_rtc_info *info) >>>>> case S2MPS13X: >>>>> data[0] = (0 << BCD_EN_SHIFT) | (1 << MODEL24_SHIFT); >>>>> ret = regmap_write(info->regmap, info->regs->ctrl, data[0]); >>>>> + if (ret < 0) >>>>> + break; >>>>> + >>>>> + ret = regmap_update_bits(info->regmap, >>>>> + info->regs->rtc_udr_update, >>>>> + info->regs->rtc_udr_mask, >>>>> + info->regs->rtc_udr_mask); >>>> >>>> Very small indentation issue here, it should be aligned with the open >>>> parenthesis. >> >> OK, i will. >> >> Thanks. >> > > -- 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
 |