Re: [PATCH] rtc: s5m: fix to update ctrl register
On 21.08.2015 17:49, Joonyoung Shim wrote: > On 08/21/2015 04:25 PM, Krzysztof Kozlowski wrote: >> On 21.08.2015 15:58, Joonyoung Shim wrote: >>> 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 >>> >>> 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. >> >> It seems that setting RUDR to high is needed... but it shouldn't. For >> example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL >> register. Mainline driver does not perform read. >> >> Maybe RUDR is not set high properly before some next time read and your >> code is a coincidence, a work-around? > > As you know, the driver sets RUDR bit to high always before reads time. > I'm not sure whether any delay time needs to guarantee that RUDR bit is > set properly, but it fails keeping time. > > I tried setting RUDR bit to high after or before setting WUDR bit to > high for writing of RTC_CTRL register but it also fails keeping time. > If achieve keeping time, i should set WUDR & RUDR bits to high at the > same time. > > I'm referring also rtc-sec driver source of SM-N910U_SEA(Samsung Note4) > from SM-N910U_SEA_KK_Opensource.zip file[1]. It also does the same > operation setting WUDR & RUDR bits to high for writing of RTC_CTRL > register. > > [1] > http://opensource.samsung.com/reception/receptionSub.do?method=sub=F=N910U > Indeed, vendor code sets both WUDR and RUDR for S2MPS11 and S2MPS13 even though datasheet says something different. Probably datasheet we have is not correct or up to date. Can you send updated patch with a comment next to s5m8767_rtc_set_alarm_reg() call why it is needed? I'll give it later a try on Rinato board (S2MPS14) and my Odroid at home. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
On 08/21/2015 04:25 PM, Krzysztof Kozlowski wrote: > On 21.08.2015 15:58, Joonyoung Shim wrote: >> 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 >>> >> >> 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. > > It seems that setting RUDR to high is needed... but it shouldn't. For > example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL > register. Mainline driver does not perform read. > > Maybe RUDR is not set high properly before some next time read and your > code is a coincidence, a work-around? As you know, the driver sets RUDR bit to high always before reads time. I'm not sure whether any delay time needs to guarantee that RUDR bit is set properly, but it fails keeping time. I tried setting RUDR bit to high after or before setting WUDR bit to high for writing of RTC_CTRL register but it also fails keeping time. If achieve keeping time, i should set WUDR & RUDR bits to high at the same time. I'm referring also rtc-sec driver source of SM-N910U_SEA(Samsung Note4) from SM-N910U_SEA_KK_Opensource.zip file[1]. It also does the same operation setting WUDR & RUDR bits to high for writing of RTC_CTRL register. [1] http://opensource.samsung.com/reception/receptionSub.do?method=sub=F=N910U > > Best regards, > Krzysztof > >> >> 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 >>> Cc: >> >> 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;
Re: [PATCH] rtc: s5m: fix to update ctrl register
On 21.08.2015 15:58, Joonyoung Shim wrote: > 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 >> > > 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. It seems that setting RUDR to high is needed... but it shouldn't. For example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL register. Mainline driver does not perform read. Maybe RUDR is not set high properly before some next time read and your code is a coincidence, a work-around? Best regards, Krzysztof > > 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 >> Cc: > > 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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 > 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 > Cc: 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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.kozlow...@samsung.com 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 01 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 01 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.s...@samsung.com Cc: sta...@vger.kernel.org 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
On 21.08.2015 17:49, Joonyoung Shim wrote: On 08/21/2015 04:25 PM, Krzysztof Kozlowski wrote: On 21.08.2015 15:58, Joonyoung Shim wrote: 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.kozlow...@samsung.com 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. It seems that setting RUDR to high is needed... but it shouldn't. For example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL register. Mainline driver does not perform read. Maybe RUDR is not set high properly before some next time read and your code is a coincidence, a work-around? As you know, the driver sets RUDR bit to high always before reads time. I'm not sure whether any delay time needs to guarantee that RUDR bit is set properly, but it fails keeping time. I tried setting RUDR bit to high after or before setting WUDR bit to high for writing of RTC_CTRL register but it also fails keeping time. If achieve keeping time, i should set WUDR RUDR bits to high at the same time. I'm referring also rtc-sec driver source of SM-N910U_SEA(Samsung Note4) from SM-N910U_SEA_KK_Opensource.zip file[1]. It also does the same operation setting WUDR RUDR bits to high for writing of RTC_CTRL register. [1] http://opensource.samsung.com/reception/receptionSub.do?method=subsub=FsearchValue=N910U Indeed, vendor code sets both WUDR and RUDR for S2MPS11 and S2MPS13 even though datasheet says something different. Probably datasheet we have is not correct or up to date. Can you send updated patch with a comment next to s5m8767_rtc_set_alarm_reg() call why it is needed? I'll give it later a try on Rinato board (S2MPS14) and my Odroid at home. Best regards, Krzysztof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
On 21.08.2015 15:58, Joonyoung Shim wrote: 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.kozlow...@samsung.com 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. It seems that setting RUDR to high is needed... but it shouldn't. For example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL register. Mainline driver does not perform read. Maybe RUDR is not set high properly before some next time read and your code is a coincidence, a work-around? Best regards, Krzysztof from S2MPS14 datasheet: 3. For write only Alarm 01 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 01 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.s...@samsung.com Cc: sta...@vger.kernel.org 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
On 08/21/2015 04:25 PM, Krzysztof Kozlowski wrote: On 21.08.2015 15:58, Joonyoung Shim wrote: 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.kozlow...@samsung.com 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. It seems that setting RUDR to high is needed... but it shouldn't. For example in SM-G900H with S2MPS11 it is set before reading RTC_CTRL register. Mainline driver does not perform read. Maybe RUDR is not set high properly before some next time read and your code is a coincidence, a work-around? As you know, the driver sets RUDR bit to high always before reads time. I'm not sure whether any delay time needs to guarantee that RUDR bit is set properly, but it fails keeping time. I tried setting RUDR bit to high after or before setting WUDR bit to high for writing of RTC_CTRL register but it also fails keeping time. If achieve keeping time, i should set WUDR RUDR bits to high at the same time. I'm referring also rtc-sec driver source of SM-N910U_SEA(Samsung Note4) from SM-N910U_SEA_KK_Opensource.zip file[1]. It also does the same operation setting WUDR RUDR bits to high for writing of RTC_CTRL register. [1] http://opensource.samsung.com/reception/receptionSub.do?method=subsub=FsearchValue=N910U Best regards, Krzysztof from S2MPS14 datasheet: 3. For write only Alarm 01 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 01 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.s...@samsung.com Cc: sta...@vger.kernel.org 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
Re: [PATCH] rtc: s5m: fix to update ctrl register
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 Best regards, Krzysztof > >> Best regards, >> Krzysztof >> >> >>> Signed-off-by: Joonyoung Shim Cc: >>> >>> 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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. > Best regards, > Krzysztof > > >> >>> Signed-off-by: Joonyoung Shim >>> Cc: >> >> 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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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. Best regards, Krzysztof > >> Signed-off-by: Joonyoung Shim >> Cc: > > can you update the stable tag with the kernel version introducing the > issue? > >> --- >> 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. > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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? > Signed-off-by: Joonyoung Shim > Cc: can you update the stable tag with the kernel version introducing the issue? > --- > 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. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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. Best regards, Krzysztof Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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. Best regards, Krzysztof Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org can you update the stable tag with the kernel version introducing the issue? --- 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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.kozlow...@samsung.com Best regards, Krzysztof Best regards, Krzysztof Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] rtc: s5m: fix to update ctrl register
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? Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org can you update the stable tag with the kernel version introducing the issue? --- 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. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 17.08.2015 11:28, Joonyoung Shim wrote:
> On 08/17/2015 11:00 AM, Krzysztof Kozlowski wrote:
>> On 17.08.2015 10:47, Joonyoung Shim wrote:
>>> Hi,
>>>
>>> On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
> According to datasheet, the S2MPS13X and S2MPS14X should update write
> buffer via setting WUDR bit to high after ctrl register is updated.
Hi,
I cannot find this information in S2MPS14 datasheet. On which page is it?
>>>
>>> I got below information from S2MPS14_Data Sheet_REV0.1 document.
>>>
>>> 5.2.2.3 RTC_UPDATE
>>> ...
>>>
>>> NOTE:
>>> 1. For write Time Registers (0x00, 0x04~0x0A) & Alarm 0&1 Registers
>>> (0x0B~0x18), set WUDR bit to high.
>>
>> Right, I have too but it does not say anything about control register.
>> It mentions only time, alarm 0 and alarm 1 registers, not control.
>>
>
> Address 0x00 is RTC_CTRL, so i don't know what you say.
Ah, I see now!
You're right, the datasheet mentions control register as time register.
In this case before reading the RUDR should be set high (for all
S2MPS1[134]).
BR,
Krzysztof
>
>>>
>
> 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.
Two questions here:
1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
S2MP14)? There are some minor differences between them so I would not be
surprised if only some of them required this action.
>>>
>>> I'm not sure about that because i don't have S2MPS11 datasheet. I just
>>> got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
>>> driver,
>>
>> Yeah, I added it. But these RTC modules are slightly different,
>> especially S2MPS14, so you cannot assume that they are the same after
>> looking at mainline code.
>>
>
> OK, could you check this issue from different things?
>
> Thanks.
>
>>>
>>> static const struct mfd_cell s2mps11_devs[] = {
>>>
>>> {
>>>
>>> .name = "s2mps11-pmic",
>>>
>>> }, {
>>>
>>> .name = "s2mps14-rtc",
>>>
>>> }, {
>>>
>>> .name = "s2mps11-clk",
>>>
>>> .of_compatible = "samsung,s2mps11-clk",
>>>
>>> }
>>>
>>> };
>>>
2. The driver operates in 24-hour omode. Actually it sets the 24-hour
mode just before your new regmap_update_bits() call. What do you mean by
12-hour mode?
>>>
>>> RTC_CTRL register value is 0 until write buffer is updated, so it is
>>> used to 12-hour mode.
>>
>> I mean what do you have in mind by saying:
>> "... and hour format is used to 12 hour mode in Odroid-XU3 board."
>> The hour format is set by driver to 24h mode.
>>
>>
>>>
> Signed-off-by: Joonyoung Shim
> Cc:
Thanks for putting a cc-stable tag. How far this should be ported? If
this is needed only for S2MPS11 then v4.1. If all of them then probably
for earlier version?
>>>
>>> If find exact version, i think it will be a version after below commit
>>> was applied.
>>>
>>> The commit 0c5deb1ea92f("rtc: s5m: add support for S2MPS14 RTC")
>>>
>>> $ git name-rev 0c5deb1ea92f
>>> 0c5deb1ea92f tags/v3.16-rc1~53^2~1
>>
>> Hmmm... I am not against the patch (especially that it matches source
>> code of SM-G900H with S2MPS11) but I have doubts about affected
>> chipsets. My Odroid (probably because of bootloader), S2MPS13 and
>> S2MPS14 do not need it. This confuses me...
>>
>> Best regards,
>> Krzysztof
>>
>>>
>>> Thanks.
>>>
Best regards,
Krzysztof
> ---
> 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 <<
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 08/17/2015 11:00 AM, Krzysztof Kozlowski wrote:
> On 17.08.2015 10:47, Joonyoung Shim wrote:
>> Hi,
>>
>> On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
>>> W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
According to datasheet, the S2MPS13X and S2MPS14X should update write
buffer via setting WUDR bit to high after ctrl register is updated.
>>>
>>> Hi,
>>>
>>> I cannot find this information in S2MPS14 datasheet. On which page is it?
>>>
>>
>> I got below information from S2MPS14_Data Sheet_REV0.1 document.
>>
>> 5.2.2.3 RTC_UPDATE
>> ...
>>
>> NOTE:
>> 1. For write Time Registers (0x00, 0x04~0x0A) & Alarm 0&1 Registers
>> (0x0B~0x18), set WUDR bit to high.
>
> Right, I have too but it does not say anything about control register.
> It mentions only time, alarm 0 and alarm 1 registers, not control.
>
Address 0x00 is RTC_CTRL, so i don't know what you say.
>>
>>>
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.
>>>
>>> Two questions here:
>>> 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
>>> Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
>>> S2MP14)? There are some minor differences between them so I would not be
>>> surprised if only some of them required this action.
>>>
>>
>> I'm not sure about that because i don't have S2MPS11 datasheet. I just
>> got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
>> driver,
>
> Yeah, I added it. But these RTC modules are slightly different,
> especially S2MPS14, so you cannot assume that they are the same after
> looking at mainline code.
>
OK, could you check this issue from different things?
Thanks.
>>
>> static const struct mfd_cell s2mps11_devs[] = {
>>
>> {
>>
>> .name = "s2mps11-pmic",
>>
>> }, {
>>
>> .name = "s2mps14-rtc",
>>
>> }, {
>>
>> .name = "s2mps11-clk",
>>
>> .of_compatible = "samsung,s2mps11-clk",
>>
>> }
>>
>> };
>>
>>> 2. The driver operates in 24-hour omode. Actually it sets the 24-hour
>>> mode just before your new regmap_update_bits() call. What do you mean by
>>> 12-hour mode?
>>>
>>
>> RTC_CTRL register value is 0 until write buffer is updated, so it is
>> used to 12-hour mode.
>
> I mean what do you have in mind by saying:
> "... and hour format is used to 12 hour mode in Odroid-XU3 board."
> The hour format is set by driver to 24h mode.
>
>
>>
Signed-off-by: Joonyoung Shim
Cc:
>>>
>>> Thanks for putting a cc-stable tag. How far this should be ported? If
>>> this is needed only for S2MPS11 then v4.1. If all of them then probably
>>> for earlier version?
>>>
>>
>> If find exact version, i think it will be a version after below commit
>> was applied.
>>
>> The commit 0c5deb1ea92f("rtc: s5m: add support for S2MPS14 RTC")
>>
>> $ git name-rev 0c5deb1ea92f
>> 0c5deb1ea92f tags/v3.16-rc1~53^2~1
>
> Hmmm... I am not against the patch (especially that it matches source
> code of SM-G900H with S2MPS11) but I have doubts about affected
> chipsets. My Odroid (probably because of bootloader), S2MPS13 and
> S2MPS14 do not need it. This confuses me...
>
> Best regards,
> Krzysztof
>
>>
>> Thanks.
>>
>>> Best regards,
>>> Krzysztof
>>>
---
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,
+
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 17.08.2015 10:47, Joonyoung Shim wrote:
> Hi,
>
> On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
>> W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
>>> According to datasheet, the S2MPS13X and S2MPS14X should update write
>>> buffer via setting WUDR bit to high after ctrl register is updated.
>>
>> Hi,
>>
>> I cannot find this information in S2MPS14 datasheet. On which page is it?
>>
>
> I got below information from S2MPS14_Data Sheet_REV0.1 document.
>
> 5.2.2.3 RTC_UPDATE
> ...
>
> NOTE:
> 1. For write Time Registers (0x00, 0x04~0x0A) & Alarm 0&1 Registers
> (0x0B~0x18), set WUDR bit to high.
Right, I have too but it does not say anything about control register.
It mentions only time, alarm 0 and alarm 1 registers, not control.
>
>>
>>>
>>> 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.
>>
>> Two questions here:
>> 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
>> Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
>> S2MP14)? There are some minor differences between them so I would not be
>> surprised if only some of them required this action.
>>
>
> I'm not sure about that because i don't have S2MPS11 datasheet. I just
> got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
> driver,
Yeah, I added it. But these RTC modules are slightly different,
especially S2MPS14, so you cannot assume that they are the same after
looking at mainline code.
>
> static const struct mfd_cell s2mps11_devs[] = {
>
> {
>
> .name = "s2mps11-pmic",
>
> }, {
>
> .name = "s2mps14-rtc",
>
> }, {
>
> .name = "s2mps11-clk",
>
> .of_compatible = "samsung,s2mps11-clk",
>
> }
>
> };
>
>> 2. The driver operates in 24-hour omode. Actually it sets the 24-hour
>> mode just before your new regmap_update_bits() call. What do you mean by
>> 12-hour mode?
>>
>
> RTC_CTRL register value is 0 until write buffer is updated, so it is
> used to 12-hour mode.
I mean what do you have in mind by saying:
"... and hour format is used to 12 hour mode in Odroid-XU3 board."
The hour format is set by driver to 24h mode.
>
>>> Signed-off-by: Joonyoung Shim
>>> Cc:
>>
>> Thanks for putting a cc-stable tag. How far this should be ported? If
>> this is needed only for S2MPS11 then v4.1. If all of them then probably
>> for earlier version?
>>
>
> If find exact version, i think it will be a version after below commit
> was applied.
>
> The commit 0c5deb1ea92f("rtc: s5m: add support for S2MPS14 RTC")
>
> $ git name-rev 0c5deb1ea92f
> 0c5deb1ea92f tags/v3.16-rc1~53^2~1
Hmmm... I am not against the patch (especially that it matches source
code of SM-G900H with S2MPS11) but I have doubts about affected
chipsets. My Odroid (probably because of bootloader), S2MPS13 and
S2MPS14 do not need it. This confuses me...
Best regards,
Krzysztof
>
> Thanks.
>
>> Best regards,
>> Krzysztof
>>
>>> ---
>>> 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);
>>> + if (ret < 0)
>>> + break;
>>> +
>>> + ret = s5m8767_wait_for_udr_update(info);
>>> +
>>> break;
>>>
>>> default:
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
Hi,
On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
> W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
>> According to datasheet, the S2MPS13X and S2MPS14X should update write
>> buffer via setting WUDR bit to high after ctrl register is updated.
>
> Hi,
>
> I cannot find this information in S2MPS14 datasheet. On which page is it?
>
I got below information from S2MPS14_Data Sheet_REV0.1 document.
5.2.2.3 RTC_UPDATE
...
NOTE:
1. For write Time Registers (0x00, 0x04~0x0A) & Alarm 0&1 Registers
(0x0B~0x18), set WUDR bit to high.
>
>>
>> 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.
>
> Two questions here:
> 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
> Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
> S2MP14)? There are some minor differences between them so I would not be
> surprised if only some of them required this action.
>
I'm not sure about that because i don't have S2MPS11 datasheet. I just
got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
driver,
static const struct mfd_cell s2mps11_devs[] = {
{
.name = "s2mps11-pmic",
}, {
.name = "s2mps14-rtc",
}, {
.name = "s2mps11-clk",
.of_compatible = "samsung,s2mps11-clk",
}
};
> 2. The driver operates in 24-hour omode. Actually it sets the 24-hour
> mode just before your new regmap_update_bits() call. What do you mean by
> 12-hour mode?
>
RTC_CTRL register value is 0 until write buffer is updated, so it is
used to 12-hour mode.
>> Signed-off-by: Joonyoung Shim
>> Cc:
>
> Thanks for putting a cc-stable tag. How far this should be ported? If
> this is needed only for S2MPS11 then v4.1. If all of them then probably
> for earlier version?
>
If find exact version, i think it will be a version after below commit
was applied.
The commit 0c5deb1ea92f("rtc: s5m: add support for S2MPS14 RTC")
$ git name-rev 0c5deb1ea92f
0c5deb1ea92f tags/v3.16-rc1~53^2~1
Thanks.
> Best regards,
> Krzysztof
>
>> ---
>> 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);
>> +if (ret < 0)
>> +break;
>> +
>> +ret = s5m8767_wait_for_udr_update(info);
>> +
>> break;
>>
>> default:
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 08/13/2015 07:42 PM, Krzysztof Kozlowski wrote: > W dniu 13.08.2015 o 19:02, Krzysztof Kozlowski pisze: >> W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: >>> According to datasheet, the S2MPS13X and S2MPS14X should update write >>> buffer via setting WUDR bit to high after ctrl register is updated. >> >> Hi, >> >> I cannot find this information in S2MPS14 datasheet. On which page is it? >> >> >>> >>> 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. >> >> Two questions here: >> 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. >> Are you sure that this applies to all of them (S2MPS11, S2MPS13 and >> S2MP14)? There are some minor differences between them so I would not be >> surprised if only some of them required this action. >> >> 2. The driver operates in 24-hour omode. Actually it sets the 24-hour >> mode just before your new regmap_update_bits() call. What do you mean by >> 12-hour mode? >> >>> Signed-off-by: Joonyoung Shim >>> Cc: >> >> Thanks for putting a cc-stable tag. How far this should be ported? If >> this is needed only for S2MPS11 then v4.1. If all of them then probably >> for earlier version? >> >> Best regards, >> Krzysztof > > One more doubt. On my Odroid XU3 first and consecutive executions > (including first) of rtctest run fine. They pass. Can you describe > exactly observable issue and how to reproduce it? This is also important > as a reason for stable backport. I just tested it on next-20150810 with Odroid-XU3 board. First rtctest execution is blocked, # ./rtctest RTC Driver Test Example. Counting 5 update (1/sec) interrupts from reading /dev/rtc0: Any no progress. > > I tested it on next-20150729 on board with Hardkernel bootloader. Maybe > the bootloader sets 12-hour mode which cannot be switched to 24-hour? > My bootloader doesn't touch any registers of s5m-rtc. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
Hi,
On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
According to datasheet, the S2MPS13X and S2MPS14X should update write
buffer via setting WUDR bit to high after ctrl register is updated.
Hi,
I cannot find this information in S2MPS14 datasheet. On which page is it?
I got below information from S2MPS14_Data Sheet_REV0.1 document.
5.2.2.3 RTC_UPDATE
...
NOTE:
1. For write Time Registers (0x00, 0x04~0x0A) Alarm 01 Registers
(0x0B~0x18), set WUDR bit to high.
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.
Two questions here:
1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
S2MP14)? There are some minor differences between them so I would not be
surprised if only some of them required this action.
I'm not sure about that because i don't have S2MPS11 datasheet. I just
got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
driver,
static const struct mfd_cell s2mps11_devs[] = {
{
.name = s2mps11-pmic,
}, {
.name = s2mps14-rtc,
}, {
.name = s2mps11-clk,
.of_compatible = samsung,s2mps11-clk,
}
};
2. The driver operates in 24-hour omode. Actually it sets the 24-hour
mode just before your new regmap_update_bits() call. What do you mean by
12-hour mode?
RTC_CTRL register value is 0 until write buffer is updated, so it is
used to 12-hour mode.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Cc: sta...@vger.kernel.org
Thanks for putting a cc-stable tag. How far this should be ported? If
this is needed only for S2MPS11 then v4.1. If all of them then probably
for earlier version?
If find exact version, i think it will be a version after below commit
was applied.
The commit 0c5deb1ea92f(rtc: s5m: add support for S2MPS14 RTC)
$ git name-rev 0c5deb1ea92f
0c5deb1ea92f tags/v3.16-rc1~53^2~1
Thanks.
Best regards,
Krzysztof
---
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);
+if (ret 0)
+break;
+
+ret = s5m8767_wait_for_udr_update(info);
+
break;
default:
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc
in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 08/13/2015 07:42 PM, Krzysztof Kozlowski wrote: W dniu 13.08.2015 o 19:02, Krzysztof Kozlowski pisze: W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: According to datasheet, the S2MPS13X and S2MPS14X should update write buffer via setting WUDR bit to high after ctrl register is updated. Hi, I cannot find this information in S2MPS14 datasheet. On which page is it? 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. Two questions here: 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. Are you sure that this applies to all of them (S2MPS11, S2MPS13 and S2MP14)? There are some minor differences between them so I would not be surprised if only some of them required this action. 2. The driver operates in 24-hour omode. Actually it sets the 24-hour mode just before your new regmap_update_bits() call. What do you mean by 12-hour mode? Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org Thanks for putting a cc-stable tag. How far this should be ported? If this is needed only for S2MPS11 then v4.1. If all of them then probably for earlier version? Best regards, Krzysztof One more doubt. On my Odroid XU3 first and consecutive executions (including first) of rtctest run fine. They pass. Can you describe exactly observable issue and how to reproduce it? This is also important as a reason for stable backport. I just tested it on next-20150810 with Odroid-XU3 board. First rtctest execution is blocked, # ./rtctest RTC Driver Test Example. Counting 5 update (1/sec) interrupts from reading /dev/rtc0: Any no progress. I tested it on next-20150729 on board with Hardkernel bootloader. Maybe the bootloader sets 12-hour mode which cannot be switched to 24-hour? My bootloader doesn't touch any registers of s5m-rtc. Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 17.08.2015 10:47, Joonyoung Shim wrote:
Hi,
On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
According to datasheet, the S2MPS13X and S2MPS14X should update write
buffer via setting WUDR bit to high after ctrl register is updated.
Hi,
I cannot find this information in S2MPS14 datasheet. On which page is it?
I got below information from S2MPS14_Data Sheet_REV0.1 document.
5.2.2.3 RTC_UPDATE
...
NOTE:
1. For write Time Registers (0x00, 0x04~0x0A) Alarm 01 Registers
(0x0B~0x18), set WUDR bit to high.
Right, I have too but it does not say anything about control register.
It mentions only time, alarm 0 and alarm 1 registers, not control.
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.
Two questions here:
1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
S2MP14)? There are some minor differences between them so I would not be
surprised if only some of them required this action.
I'm not sure about that because i don't have S2MPS11 datasheet. I just
got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
driver,
Yeah, I added it. But these RTC modules are slightly different,
especially S2MPS14, so you cannot assume that they are the same after
looking at mainline code.
static const struct mfd_cell s2mps11_devs[] = {
{
.name = s2mps11-pmic,
}, {
.name = s2mps14-rtc,
}, {
.name = s2mps11-clk,
.of_compatible = samsung,s2mps11-clk,
}
};
2. The driver operates in 24-hour omode. Actually it sets the 24-hour
mode just before your new regmap_update_bits() call. What do you mean by
12-hour mode?
RTC_CTRL register value is 0 until write buffer is updated, so it is
used to 12-hour mode.
I mean what do you have in mind by saying:
... and hour format is used to 12 hour mode in Odroid-XU3 board.
The hour format is set by driver to 24h mode.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Cc: sta...@vger.kernel.org
Thanks for putting a cc-stable tag. How far this should be ported? If
this is needed only for S2MPS11 then v4.1. If all of them then probably
for earlier version?
If find exact version, i think it will be a version after below commit
was applied.
The commit 0c5deb1ea92f(rtc: s5m: add support for S2MPS14 RTC)
$ git name-rev 0c5deb1ea92f
0c5deb1ea92f tags/v3.16-rc1~53^2~1
Hmmm... I am not against the patch (especially that it matches source
code of SM-G900H with S2MPS11) but I have doubts about affected
chipsets. My Odroid (probably because of bootloader), S2MPS13 and
S2MPS14 do not need it. This confuses me...
Best regards,
Krzysztof
Thanks.
Best regards,
Krzysztof
---
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);
+ if (ret 0)
+ break;
+
+ ret = s5m8767_wait_for_udr_update(info);
+
break;
default:
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc
in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 08/17/2015 11:00 AM, Krzysztof Kozlowski wrote:
On 17.08.2015 10:47, Joonyoung Shim wrote:
Hi,
On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
According to datasheet, the S2MPS13X and S2MPS14X should update write
buffer via setting WUDR bit to high after ctrl register is updated.
Hi,
I cannot find this information in S2MPS14 datasheet. On which page is it?
I got below information from S2MPS14_Data Sheet_REV0.1 document.
5.2.2.3 RTC_UPDATE
...
NOTE:
1. For write Time Registers (0x00, 0x04~0x0A) Alarm 01 Registers
(0x0B~0x18), set WUDR bit to high.
Right, I have too but it does not say anything about control register.
It mentions only time, alarm 0 and alarm 1 registers, not control.
Address 0x00 is RTC_CTRL, so i don't know what you say.
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.
Two questions here:
1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
S2MP14)? There are some minor differences between them so I would not be
surprised if only some of them required this action.
I'm not sure about that because i don't have S2MPS11 datasheet. I just
got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
driver,
Yeah, I added it. But these RTC modules are slightly different,
especially S2MPS14, so you cannot assume that they are the same after
looking at mainline code.
OK, could you check this issue from different things?
Thanks.
static const struct mfd_cell s2mps11_devs[] = {
{
.name = s2mps11-pmic,
}, {
.name = s2mps14-rtc,
}, {
.name = s2mps11-clk,
.of_compatible = samsung,s2mps11-clk,
}
};
2. The driver operates in 24-hour omode. Actually it sets the 24-hour
mode just before your new regmap_update_bits() call. What do you mean by
12-hour mode?
RTC_CTRL register value is 0 until write buffer is updated, so it is
used to 12-hour mode.
I mean what do you have in mind by saying:
... and hour format is used to 12 hour mode in Odroid-XU3 board.
The hour format is set by driver to 24h mode.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Cc: sta...@vger.kernel.org
Thanks for putting a cc-stable tag. How far this should be ported? If
this is needed only for S2MPS11 then v4.1. If all of them then probably
for earlier version?
If find exact version, i think it will be a version after below commit
was applied.
The commit 0c5deb1ea92f(rtc: s5m: add support for S2MPS14 RTC)
$ git name-rev 0c5deb1ea92f
0c5deb1ea92f tags/v3.16-rc1~53^2~1
Hmmm... I am not against the patch (especially that it matches source
code of SM-G900H with S2MPS11) but I have doubts about affected
chipsets. My Odroid (probably because of bootloader), S2MPS13 and
S2MPS14 do not need it. This confuses me...
Best regards,
Krzysztof
Thanks.
Best regards,
Krzysztof
---
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);
+ if (ret 0)
+ break;
+
+ ret = s5m8767_wait_for_udr_update(info);
+
break;
default:
--
To unsubscribe from this list: send the line unsubscribe
linux-samsung-soc in
the body of a message to
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
On 17.08.2015 11:28, Joonyoung Shim wrote:
On 08/17/2015 11:00 AM, Krzysztof Kozlowski wrote:
On 17.08.2015 10:47, Joonyoung Shim wrote:
Hi,
On 08/13/2015 07:02 PM, Krzysztof Kozlowski wrote:
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze:
According to datasheet, the S2MPS13X and S2MPS14X should update write
buffer via setting WUDR bit to high after ctrl register is updated.
Hi,
I cannot find this information in S2MPS14 datasheet. On which page is it?
I got below information from S2MPS14_Data Sheet_REV0.1 document.
5.2.2.3 RTC_UPDATE
...
NOTE:
1. For write Time Registers (0x00, 0x04~0x0A) Alarm 01 Registers
(0x0B~0x18), set WUDR bit to high.
Right, I have too but it does not say anything about control register.
It mentions only time, alarm 0 and alarm 1 registers, not control.
Address 0x00 is RTC_CTRL, so i don't know what you say.
Ah, I see now!
You're right, the datasheet mentions control register as time register.
In this case before reading the RUDR should be set high (for all
S2MPS1[134]).
BR,
Krzysztof
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.
Two questions here:
1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11.
Are you sure that this applies to all of them (S2MPS11, S2MPS13 and
S2MP14)? There are some minor differences between them so I would not be
surprised if only some of them required this action.
I'm not sure about that because i don't have S2MPS11 datasheet. I just
got the information that S2MPS11 also use S2MPS14 rtc from sec-core mfd
driver,
Yeah, I added it. But these RTC modules are slightly different,
especially S2MPS14, so you cannot assume that they are the same after
looking at mainline code.
OK, could you check this issue from different things?
Thanks.
static const struct mfd_cell s2mps11_devs[] = {
{
.name = s2mps11-pmic,
}, {
.name = s2mps14-rtc,
}, {
.name = s2mps11-clk,
.of_compatible = samsung,s2mps11-clk,
}
};
2. The driver operates in 24-hour omode. Actually it sets the 24-hour
mode just before your new regmap_update_bits() call. What do you mean by
12-hour mode?
RTC_CTRL register value is 0 until write buffer is updated, so it is
used to 12-hour mode.
I mean what do you have in mind by saying:
... and hour format is used to 12 hour mode in Odroid-XU3 board.
The hour format is set by driver to 24h mode.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Cc: sta...@vger.kernel.org
Thanks for putting a cc-stable tag. How far this should be ported? If
this is needed only for S2MPS11 then v4.1. If all of them then probably
for earlier version?
If find exact version, i think it will be a version after below commit
was applied.
The commit 0c5deb1ea92f(rtc: s5m: add support for S2MPS14 RTC)
$ git name-rev 0c5deb1ea92f
0c5deb1ea92f tags/v3.16-rc1~53^2~1
Hmmm... I am not against the patch (especially that it matches source
code of SM-G900H with S2MPS11) but I have doubts about affected
chipsets. My Odroid (probably because of bootloader), S2MPS13 and
S2MPS14 do not need it. This confuses me...
Best regards,
Krzysztof
Thanks.
Best regards,
Krzysztof
---
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);
+ if (ret 0)
+
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
W dniu 13.08.2015 o 19:02, Krzysztof Kozlowski pisze: > W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: >> According to datasheet, the S2MPS13X and S2MPS14X should update write >> buffer via setting WUDR bit to high after ctrl register is updated. > > Hi, > > I cannot find this information in S2MPS14 datasheet. On which page is it? > > >> >> 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. > > Two questions here: > 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. > Are you sure that this applies to all of them (S2MPS11, S2MPS13 and > S2MP14)? There are some minor differences between them so I would not be > surprised if only some of them required this action. > > 2. The driver operates in 24-hour omode. Actually it sets the 24-hour > mode just before your new regmap_update_bits() call. What do you mean by > 12-hour mode? > >> Signed-off-by: Joonyoung Shim >> Cc: > > Thanks for putting a cc-stable tag. How far this should be ported? If > this is needed only for S2MPS11 then v4.1. If all of them then probably > for earlier version? > > Best regards, > Krzysztof One more doubt. On my Odroid XU3 first and consecutive executions (including first) of rtctest run fine. They pass. Can you describe exactly observable issue and how to reproduce it? This is also important as a reason for stable backport. I tested it on next-20150729 on board with Hardkernel bootloader. Maybe the bootloader sets 12-hour mode which cannot be switched to 24-hour? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: > According to datasheet, the S2MPS13X and S2MPS14X should update write > buffer via setting WUDR bit to high after ctrl register is updated. Hi, I cannot find this information in S2MPS14 datasheet. On which page is it? > > 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. Two questions here: 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. Are you sure that this applies to all of them (S2MPS11, S2MPS13 and S2MP14)? There are some minor differences between them so I would not be surprised if only some of them required this action. 2. The driver operates in 24-hour omode. Actually it sets the 24-hour mode just before your new regmap_update_bits() call. What do you mean by 12-hour mode? > Signed-off-by: Joonyoung Shim > Cc: Thanks for putting a cc-stable tag. How far this should be ported? If this is needed only for S2MPS11 then v4.1. If all of them then probably for earlier version? Best regards, Krzysztof > --- > 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); > + if (ret < 0) > + break; > + > + ret = s5m8767_wait_for_udr_update(info); > + > break; > > default: > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: s5m: fix to update ctrl register
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. Signed-off-by: Joonyoung Shim Cc: --- 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); + if (ret < 0) + break; + + ret = s5m8767_wait_for_udr_update(info); + break; default: -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] rtc: s5m: fix to update ctrl register
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. Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org --- 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); + if (ret 0) + break; + + ret = s5m8767_wait_for_udr_update(info); + break; default: -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: According to datasheet, the S2MPS13X and S2MPS14X should update write buffer via setting WUDR bit to high after ctrl register is updated. Hi, I cannot find this information in S2MPS14 datasheet. On which page is it? 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. Two questions here: 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. Are you sure that this applies to all of them (S2MPS11, S2MPS13 and S2MP14)? There are some minor differences between them so I would not be surprised if only some of them required this action. 2. The driver operates in 24-hour omode. Actually it sets the 24-hour mode just before your new regmap_update_bits() call. What do you mean by 12-hour mode? Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org Thanks for putting a cc-stable tag. How far this should be ported? If this is needed only for S2MPS11 then v4.1. If all of them then probably for earlier version? Best regards, Krzysztof --- 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); + if (ret 0) + break; + + ret = s5m8767_wait_for_udr_update(info); + break; default: -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [rtc-linux] [PATCH] rtc: s5m: fix to update ctrl register
W dniu 13.08.2015 o 19:02, Krzysztof Kozlowski pisze: W dniu 13.08.2015 o 17:49, Joonyoung Shim pisze: According to datasheet, the S2MPS13X and S2MPS14X should update write buffer via setting WUDR bit to high after ctrl register is updated. Hi, I cannot find this information in S2MPS14 datasheet. On which page is it? 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. Two questions here: 1. Earlier you mentioned S2MPS1[34], now Odroid XU3 which has S2MPS11. Are you sure that this applies to all of them (S2MPS11, S2MPS13 and S2MP14)? There are some minor differences between them so I would not be surprised if only some of them required this action. 2. The driver operates in 24-hour omode. Actually it sets the 24-hour mode just before your new regmap_update_bits() call. What do you mean by 12-hour mode? Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Cc: sta...@vger.kernel.org Thanks for putting a cc-stable tag. How far this should be ported? If this is needed only for S2MPS11 then v4.1. If all of them then probably for earlier version? Best regards, Krzysztof One more doubt. On my Odroid XU3 first and consecutive executions (including first) of rtctest run fine. They pass. Can you describe exactly observable issue and how to reproduce it? This is also important as a reason for stable backport. I tested it on next-20150729 on board with Hardkernel bootloader. Maybe the bootloader sets 12-hour mode which cannot be switched to 24-hour? Best regards, Krzysztof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
