linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] rtc: sun6i: Make external oscillator optional
@ 2020-03-08 13:58 Jernej Skrabec
  2020-03-08 13:58 ` [PATCH v2 1/2] rtc: sun6i: Make external 32k " Jernej Skrabec
  2020-03-08 13:58 ` [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs Jernej Skrabec
  0 siblings, 2 replies; 6+ messages in thread
From: Jernej Skrabec @ 2020-03-08 13:58 UTC (permalink / raw)
  To: mripard, wens
  Cc: robh+dt, a.zummo, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc

This is implementation of idea discussed here:
https://lore.kernel.org/linux-arm-kernel/20200117183901.lkieha3hu6nz2hoj@gilmour.lan/T/

Part of first patch commit message:

Some boards, like OrangePi PC2 (H5), OrangePi Plus 2E (H3) and Tanix TX6
(H6) don't have external 32kHz oscillator. Till H6, it didn't really
matter if external oscillator was enabled because HW detected error and
fall back to internal one. H6 has same functionality but it's the first
SoC which have "auto switch bypass" bit documented and always enabled in
driver. This prevents RTC to work correctly if external crystal is not
present on board. There are other side effects - all peripherals which
depends on this clock also don't work (HDMI CEC for example).

In this series I fixed only H6 based boards since improper settings have
real impact due to explicitly forbidden fallback to internal oscillator.
Since most boards actually contain external oscillator, I wonder if it's
better to leave external oscillator in common H6 dtsi and just delete
clocks property in rtc node and ext. oscillator node in board dts file?

What do you think?

Best regards,
Jernej

Changes from v1:
- added comments to driver to make more clear how clock registration
  works

Jernej Skrabec (2):
  rtc: sun6i: Make external 32k oscillator optional
  arm64: dts: allwinner: h6: Move ext. oscillator to board DTs

 .../boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 11 +++++++++++
 .../boot/dts/allwinner/sun50i-h6-orangepi-3.dts  | 11 +++++++++++
 .../boot/dts/allwinner/sun50i-h6-orangepi.dtsi   | 11 +++++++++++
 .../boot/dts/allwinner/sun50i-h6-pine-h64.dts    | 11 +++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi     |  8 --------
 drivers/rtc/rtc-sun6i.c                          | 16 ++++++++--------
 6 files changed, 52 insertions(+), 16 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] rtc: sun6i: Make external 32k oscillator optional
  2020-03-08 13:58 [PATCH v2 0/2] rtc: sun6i: Make external oscillator optional Jernej Skrabec
@ 2020-03-08 13:58 ` Jernej Skrabec
  2020-03-09 15:17   ` Maxime Ripard
  2020-03-12 23:20   ` Alexandre Belloni
  2020-03-08 13:58 ` [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs Jernej Skrabec
  1 sibling, 2 replies; 6+ messages in thread
From: Jernej Skrabec @ 2020-03-08 13:58 UTC (permalink / raw)
  To: mripard, wens
  Cc: robh+dt, a.zummo, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc

Some boards, like OrangePi PC2 (H5), OrangePi Plus 2E (H3) and Tanix TX6
(H6) don't have external 32kHz oscillator. Till H6, it didn't really
matter if external oscillator was enabled because HW detected error and
fall back to internal one. H6 has same functionality but it's the first
SoC which have "auto switch bypass" bit documented and always enabled in
driver. This prevents RTC to work correctly if external crystal is not
present on board. There are other side effects - all peripherals which
depends on this clock also don't work (HDMI CEC for example).

Make clocks property optional. If it is present, select external
oscillator. If not, stay on internal.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 drivers/rtc/rtc-sun6i.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 852f5f3b3592..415a20a936e4 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -250,19 +250,17 @@ static void __init sun6i_rtc_clk_init(struct device_node *node,
 		writel(reg, rtc->base + SUN6I_LOSC_CTRL);
 	}
 
-	/* Switch to the external, more precise, oscillator */
-	reg |= SUN6I_LOSC_CTRL_EXT_OSC;
-	if (rtc->data->has_losc_en)
-		reg |= SUN6I_LOSC_CTRL_EXT_LOSC_EN;
+	/* Switch to the external, more precise, oscillator, if present */
+	if (of_get_property(node, "clocks", NULL)) {
+		reg |= SUN6I_LOSC_CTRL_EXT_OSC;
+		if (rtc->data->has_losc_en)
+			reg |= SUN6I_LOSC_CTRL_EXT_LOSC_EN;
+	}
 	writel(reg, rtc->base + SUN6I_LOSC_CTRL);
 
 	/* Yes, I know, this is ugly. */
 	sun6i_rtc = rtc;
 
-	/* Deal with old DTs */
-	if (!of_get_property(node, "clocks", NULL))
-		goto err;
-
 	/* Only read IOSC name from device tree if it is exported */
 	if (rtc->data->export_iosc)
 		of_property_read_string_index(node, "clock-output-names", 2,
@@ -279,11 +277,13 @@ static void __init sun6i_rtc_clk_init(struct device_node *node,
 	}
 
 	parents[0] = clk_hw_get_name(rtc->int_osc);
+	/* If there is no external oscillator, this will be NULL and ... */
 	parents[1] = of_clk_get_parent_name(node, 0);
 
 	rtc->hw.init = &init;
 
 	init.parent_names = parents;
+	/* ... number of clock parents will be 1. */
 	init.num_parents = of_clk_get_parent_count(node) + 1;
 	of_property_read_string_index(node, "clock-output-names", 0,
 				      &init.name);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs
  2020-03-08 13:58 [PATCH v2 0/2] rtc: sun6i: Make external oscillator optional Jernej Skrabec
  2020-03-08 13:58 ` [PATCH v2 1/2] rtc: sun6i: Make external 32k " Jernej Skrabec
@ 2020-03-08 13:58 ` Jernej Skrabec
  2020-03-09 15:18   ` Maxime Ripard
  1 sibling, 1 reply; 6+ messages in thread
From: Jernej Skrabec @ 2020-03-08 13:58 UTC (permalink / raw)
  To: mripard, wens
  Cc: robh+dt, a.zummo, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc

It turns out that not all H6 boards have external 32kHz oscillator.
Currently the only one known such H6 board is Tanix TX6.

Move external oscillator node from common H6 dtsi to board specific dts
files where present.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 .../boot/dts/allwinner/sun50i-h6-beelink-gs1.dts      | 11 +++++++++++
 .../arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts | 11 +++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi | 11 +++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts  | 11 +++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi          |  8 --------
 5 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
index df6d872c34e2..8f09d209359b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -32,6 +32,13 @@ hdmi_con_in: endpoint {
 		};
 	};
 
+	ext_osc32k: ext_osc32k_clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <32768>;
+		clock-output-names = "ext_osc32k";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -275,6 +282,10 @@ &r_pio {
 	vcc-pm-supply = <&reg_aldo1>;
 };
 
+&rtc {
+	clocks = <&ext_osc32k>;
+};
+
 &spdif {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 1e0abd9d047f..47f579610dcc 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -32,6 +32,13 @@ hdmi_con_in: endpoint {
 		};
 	};
 
+	ext_osc32k: ext_osc32k_clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <32768>;
+		clock-output-names = "ext_osc32k";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -285,6 +292,10 @@ &r_ir {
 	status = "okay";
 };
 
+&rtc {
+	clocks = <&ext_osc32k>;
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_ph_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
index 37f4c57597d4..37fc3f3697f7 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
@@ -20,6 +20,13 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	ext_osc32k: ext_osc32k_clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <32768>;
+		clock-output-names = "ext_osc32k";
+	};
+
 	leds {
 		compatible = "gpio-leds";
 
@@ -197,6 +204,10 @@ &r_ir {
 	status = "okay";
 };
 
+&rtc {
+	clocks = <&ext_osc32k>;
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_ph_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
index 3c9dd0d69754..b0642d841933 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
@@ -21,6 +21,13 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	ext_osc32k: ext_osc32k_clk {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <32768>;
+		clock-output-names = "ext_osc32k";
+	};
+
 	hdmi_connector: connector {
 		compatible = "hdmi-connector";
 		type = "a";
@@ -279,6 +286,10 @@ &r_pio {
 	vcc-pm-supply = <&reg_aldo1>;
 };
 
+&rtc {
+	clocks = <&ext_osc32k>;
+};
+
 /*
  * The CS pin is shared with the MMC2 CMD pin, so we cannot have the SPI
  * flash and eMMC at the same time, as one of them would fail probing.
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 967249e58811..b9ab7d8fa8af 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -62,13 +62,6 @@ osc24M: osc24M_clk {
 		clock-output-names = "osc24M";
 	};
 
-	ext_osc32k: ext_osc32k_clk {
-		#clock-cells = <0>;
-		compatible = "fixed-clock";
-		clock-frequency = <32768>;
-		clock-output-names = "ext_osc32k";
-	};
-
 	pmu {
 		compatible = "arm,cortex-a53-pmu";
 		interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>,
@@ -854,7 +847,6 @@ rtc: rtc@7000000 {
 			interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
 			clock-output-names = "osc32k", "osc32k-out", "iosc";
-			clocks = <&ext_osc32k>;
 			#clock-cells = <1>;
 		};
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] rtc: sun6i: Make external 32k oscillator optional
  2020-03-08 13:58 ` [PATCH v2 1/2] rtc: sun6i: Make external 32k " Jernej Skrabec
@ 2020-03-09 15:17   ` Maxime Ripard
  2020-03-12 23:20   ` Alexandre Belloni
  1 sibling, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2020-03-09 15:17 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: wens, robh+dt, a.zummo, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On Sun, Mar 08, 2020 at 02:58:48PM +0100, Jernej Skrabec wrote:
> Some boards, like OrangePi PC2 (H5), OrangePi Plus 2E (H3) and Tanix TX6
> (H6) don't have external 32kHz oscillator. Till H6, it didn't really
> matter if external oscillator was enabled because HW detected error and
> fall back to internal one. H6 has same functionality but it's the first
> SoC which have "auto switch bypass" bit documented and always enabled in
> driver. This prevents RTC to work correctly if external crystal is not
> present on board. There are other side effects - all peripherals which
> depends on this clock also don't work (HDMI CEC for example).
>
> Make clocks property optional. If it is present, select external
> oscillator. If not, stay on internal.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Acked-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs
  2020-03-08 13:58 ` [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs Jernej Skrabec
@ 2020-03-09 15:18   ` Maxime Ripard
  0 siblings, 0 replies; 6+ messages in thread
From: Maxime Ripard @ 2020-03-09 15:18 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: wens, robh+dt, a.zummo, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-rtc

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

On Sun, Mar 08, 2020 at 02:58:49PM +0100, Jernej Skrabec wrote:
> It turns out that not all H6 boards have external 32kHz oscillator.
> Currently the only one known such H6 board is Tanix TX6.
>
> Move external oscillator node from common H6 dtsi to board specific dts
> files where present.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Applied, thanks

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] rtc: sun6i: Make external 32k oscillator optional
  2020-03-08 13:58 ` [PATCH v2 1/2] rtc: sun6i: Make external 32k " Jernej Skrabec
  2020-03-09 15:17   ` Maxime Ripard
@ 2020-03-12 23:20   ` Alexandre Belloni
  1 sibling, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2020-03-12 23:20 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: mripard, wens, robh+dt, a.zummo, linux-arm-kernel, devicetree,
	linux-kernel, linux-rtc

On 08/03/2020 14:58:48+0100, Jernej Skrabec wrote:
> Some boards, like OrangePi PC2 (H5), OrangePi Plus 2E (H3) and Tanix TX6
> (H6) don't have external 32kHz oscillator. Till H6, it didn't really
> matter if external oscillator was enabled because HW detected error and
> fall back to internal one. H6 has same functionality but it's the first
> SoC which have "auto switch bypass" bit documented and always enabled in
> driver. This prevents RTC to work correctly if external crystal is not
> present on board. There are other side effects - all peripherals which
> depends on this clock also don't work (HDMI CEC for example).
> 
> Make clocks property optional. If it is present, select external
> oscillator. If not, stay on internal.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/rtc/rtc-sun6i.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-12 23:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-08 13:58 [PATCH v2 0/2] rtc: sun6i: Make external oscillator optional Jernej Skrabec
2020-03-08 13:58 ` [PATCH v2 1/2] rtc: sun6i: Make external 32k " Jernej Skrabec
2020-03-09 15:17   ` Maxime Ripard
2020-03-12 23:20   ` Alexandre Belloni
2020-03-08 13:58 ` [PATCH v2 2/2] arm64: dts: allwinner: h6: Move ext. oscillator to board DTs Jernej Skrabec
2020-03-09 15:18   ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).