Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Hello Doug, On Wed, Sep 17, 2014 at 12:15 AM, Doug Anderson diand...@chromium.org wrote: I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this clock will be left at whatever the bootloader set it to, right? Then there will be no auto-disabling by the CCF and the RTC will work. Yes, that's how Daniel was working around the issue. That's one argument for making the clock optional. Indeed. NOTE: I don't think that the builtin RTC is terribly important for any exynos-based Chromebooks that I'm aware of. We rely on the RTC that's part of the Maxim PMIC itself and pretty much ignore the one built-in to the exynos. I think there are some cases it was used (as a fallback wakeup source in certain test scripts), but nothing very important. Ok, I'll post the patch I shared before that makes the rtc to claim the max77802 32kHz clock as rtc_src anyways since that is the right thing to do even if the rtc_src ends being optional due DT backward compat. -Doug Best regards, Javier -- 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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Daniel, On Wed, Sep 17, 2014 at 9:49 AM, Daniel Drake dr...@endlessm.com wrote: On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson diand...@chromium.org wrote: NOTE: I don't think that the builtin RTC is terribly important for any exynos-based Chromebooks that I'm aware of. We rely on the RTC that's part of the Maxim PMIC itself and pretty much ignore the one built-in to the exynos. I think there are some cases it was used (as a fallback wakeup source in certain test scripts), but nothing very important. That's not true for all hardware though, at least the board I'm working on now has the SoC RTC as battery-backed and the PMIC one with no battery. So in this case at least, the interesting RTC is the SoC one. Yup, I can totally believe that. My statement was meant only to apply to the boards I knew about firsthand... -Doug -- 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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Hello Chanwoo,
On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi cw00.c...@samsung.com wrote:
This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
list of common clk framework, Exynos RTC drvier have to control this clock.
Clock list for s3c-rtc device:
- rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
- rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
Is this RTC source clock needed for all Exynos SoCs?
@@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
info-rtc_clk = devm_clk_get(pdev-dev, rtc);
if (IS_ERR(info-rtc_clk)) {
- dev_err(pdev-dev, failed to find rtc clock source\n);
+ dev_err(pdev-dev, failed to find rtc clock\n);
return PTR_ERR(info-rtc_clk);
}
clk_prepare_enable(info-rtc_clk);
+ info-rtc_src_clk = devm_clk_get(pdev-dev, rtc_src);
+ if (IS_ERR(info-rtc_src_clk)) {
+ dev_err(pdev-dev, failed to find rtc source clock\n);
+ return PTR_ERR(info-rtc_src_clk);
On an Exynos5420 Peach Pit machine I'm having the following error and
the driver fails to probe:
[2.095700] s3c-rtc: probe of 101e.rtc failed with error -2
Reverting this patch fixes the issue and the s3c RTC works again on
this machine.
Best regards,
Javier
--
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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Clock list for s3c-rtc device: - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC. - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC. Is this RTC source clock needed for all Exynos SoCs? It is at least needed on Exynos4412, which has the XrtcXTI thing exactly as you describe. However the very standard setup there is to hook it up to the CP clock output of the MAX76686 PMIC. This CP clock is on by default, so you can potentially live without that detail being present in the DT. However... one small issue with this setup is that when you enable CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's common clock framework, and Linux then turns it off because it believes it is unused. Then the RTC stops ticking. So the rtc_src idea would also be good for Exynos4412. Maybe it would make sense to drop the needs_src_clk flag, and simply require/enable the src clock whenever it is present in the DT. Also, are you sure about the way you are treating this clock, all those enable/disable calls? You only seem to enable it when doing some particular driver operations e.g. reading the time, leaving it disabled at all other times. However I believe on Exynos4412 that if you disable this clock then the RTC will not tick. Daniel -- 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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
[adding Doug Anderson as cc]
On Tue, Sep 16, 2014 at 3:48 PM, Javier Martinez Canillas
jav...@dowhile0.org wrote:
On Thu, Aug 28, 2014 at 11:02 AM, Chanwoo Choi cw00.c...@samsung.com wrote:
This patch add support for RTC of Exynos3250 SoC. The Exynos3250 needs source
clock(32.768KHz) for RTC block. If source clock of RTC is registerd on clock
list of common clk framework, Exynos RTC drvier have to control this clock.
Clock list for s3c-rtc device:
- rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC.
- rtc_src : XrtcXTI is 32.768.kHz source clock for RTC.
Is this RTC source clock needed for all Exynos SoCs?
By looking at the Exynos5420 user manual I see that there is indeed a
XrtcXTI clock pad and the doc says:
XRTCXTI: Specifies a clock from 32.768 kHz crystal pad with XRTCXTI
and XRTCXTO pins. RTC uses this
clock as the source of a real-time clock.
Also by looking at the board schematic I see that the 32KHZAP clock
from the Maxim77802 PMIC is used as the external 32.768.kHz source
clock for the RTC but that information is not in the downstream
ChromeOS DTS since AFAIU the Maxim driver in that tree predates the
common clock framework so the PMIC clocks were modeled as regulators.
@@ -480,11 +530,19 @@ static int s3c_rtc_probe(struct platform_device *pdev)
info-rtc_clk = devm_clk_get(pdev-dev, rtc);
if (IS_ERR(info-rtc_clk)) {
- dev_err(pdev-dev, failed to find rtc clock source\n);
+ dev_err(pdev-dev, failed to find rtc clock\n);
return PTR_ERR(info-rtc_clk);
}
clk_prepare_enable(info-rtc_clk);
+ info-rtc_src_clk = devm_clk_get(pdev-dev, rtc_src);
+ if (IS_ERR(info-rtc_src_clk)) {
+ dev_err(pdev-dev, failed to find rtc source clock\n);
+ return PTR_ERR(info-rtc_src_clk);
On an Exynos5420 Peach Pit machine I'm having the following error and
the driver fails to probe:
[2.095700] s3c-rtc: probe of 101e.rtc failed with error -2
Reverting this patch fixes the issue and the s3c RTC works again on
this machine.
The following change [0] in the Peach Pit DTS makes the RTC to claim
the external clock and is working again but I'm not sure if $subject
is correct in making the rtc_src property mandatory since it breaks
backward DT compatibility.
Best regards,
Javier
[0]
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a23382..47d7a5b 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -12,6 +12,7 @@
#include dt-bindings/input/input.h
#include dt-bindings/gpio/gpio.h
#include dt-bindings/interrupt-controller/irq.h
+#include dt-bindings/clock/maxim,max77802.h
#include exynos5420.dtsi
/ {
@@ -151,7 +152,7 @@
status = okay;
clock-frequency = 40;
- max77802-pmic@9 {
+ max77802: max77802-pmic@9 {
compatible = maxim,max77802;
interrupt-parent = gpx3;
interrupts = 1 IRQ_TYPE_NONE;
@@ -726,6 +727,8 @@
};
rtc {
+ clocks = clock CLK_RTC, max77802 MAX77802_CLK_32K_AP;
+ clock-names = rtc, rtc_src;
status = okay;
};
--
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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Hello Daniel, On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake dr...@endlessm.com wrote: On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Clock list for s3c-rtc device: - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC. - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC. Is this RTC source clock needed for all Exynos SoCs? It is at least needed on Exynos4412, which has the XrtcXTI thing exactly as you describe. However the very standard setup there is to hook it up to the CP clock output of the MAX76686 PMIC. This CP clock is on by default, so you can potentially live without that detail being present in the DT. Thanks for confirming for Exynos4412, I just answered my own email saying that I found to be needed on Exynos5420 as well and as you said, it was just working because the Maxim clocks were left on by default. However... one small issue with this setup is that when you enable CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's common clock framework, and Linux then turns it off because it believes it is unused. Then the RTC stops ticking. Indeed, this is an issue about relying on default state. We had a quite long discussion a couple of weeks ago about simplefb relying on clocks and regulators left enabled by the bootloader but once these were know to the kernel, the frameworks disable them because were unused making simplefb to fail. So the rtc_src idea would also be good for Exynos4412. Maybe it would make sense to drop the needs_src_clk flag, and simply require/enable the src clock whenever it is present in the DT. That sounds more sensible to me as well. I wonder what should happen in this case with DT backward compatibility though. But as you said, the external clock is required and the kernel will disable this clock once is know to the CCF since is not used so maybe will be hard to maintain DT backward compatibility in this case. Also, are you sure about the way you are treating this clock, all those enable/disable calls? You only seem to enable it when doing some particular driver operations e.g. reading the time, leaving it disabled at all other times. However I believe on Exynos4412 that if you disable this clock then the RTC will not tick. Daniel Best regards, Javier -- 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
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Hi, On Tue, Sep 16, 2014 at 8:20 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Hello Daniel, On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake dr...@endlessm.com wrote: On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Clock list for s3c-rtc device: - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC. - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC. Is this RTC source clock needed for all Exynos SoCs? It is at least needed on Exynos4412, which has the XrtcXTI thing exactly as you describe. However the very standard setup there is to hook it up to the CP clock output of the MAX76686 PMIC. This CP clock is on by default, so you can potentially live without that detail being present in the DT. Thanks for confirming for Exynos4412, I just answered my own email saying that I found to be needed on Exynos5420 as well and as you said, it was just working because the Maxim clocks were left on by default. However... one small issue with this setup is that when you enable CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's common clock framework, and Linux then turns it off because it believes it is unused. Then the RTC stops ticking. Indeed, this is an issue about relying on default state. We had a quite long discussion a couple of weeks ago about simplefb relying on clocks and regulators left enabled by the bootloader but once these were know to the kernel, the frameworks disable them because were unused making simplefb to fail. So the rtc_src idea would also be good for Exynos4412. Maybe it would make sense to drop the needs_src_clk flag, and simply require/enable the src clock whenever it is present in the DT. That sounds more sensible to me as well. I wonder what should happen in this case with DT backward compatibility though. But as you said, the external clock is required and the kernel will disable this clock once is know to the CCF since is not used so maybe will be hard to maintain DT backward compatibility in this case. I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this clock will be left at whatever the bootloader set it to, right? Then there will be no auto-disabling by the CCF and the RTC will work. That's one argument for making the clock optional. NOTE: I don't think that the builtin RTC is terribly important for any exynos-based Chromebooks that I'm aware of. We rely on the RTC that's part of the Maxim PMIC itself and pretty much ignore the one built-in to the exynos. I think there are some cases it was used (as a fallback wakeup source in certain test scripts), but nothing very important. -Doug -- 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
