[PATCH 0/8] ARM: AM4372: Enable RTC

[PATCH 0/8] ARM: AM4372: Enable RTC

[Keerthy]

The series brings multiple fixes needed to get rtc ticking
on am437x-gp-evm.

This is tested on latest 4.2-rc5. Boot tested on AM437X-GP-EVM
also tested rtc ticking via sysfs nodes. 

Dave Gerlach (1):
  ARM: dts: am437x-gp-evm: Add regulator-always-on and regulator-boot-on
    for RTC DCDCs

Keerthy (7):
  ARM: dts: AM4372: Append the rtc compatible string
  rtc: omap: Add external clock enabling support
  ARM: dts: AM4372: Add external clock flag
  ARM: AM43xx: Introduce a separate soc_is function for am438x series of
    SoCs
  ARM: dts: am43x-epos-evm: Add the am438 compatible string
  ARM: AM43XX: HWMOD: Add rtc hwmod
  ARM: dts: AM437x-gp-evm: Enable RTC

 Documentation/devicetree/bindings/arm/omap/omap.txt |  3 +++
 Documentation/devicetree/bindings/rtc/rtc-omap.txt  |  2 ++
 arch/arm/boot/dts/am4372.dtsi                       |  2 +-
 arch/arm/boot/dts/am437x-gp-evm.dts                 |  9 +++++++++
 arch/arm/boot/dts/am437x-idk-evm.dts                |  1 +
 arch/arm/boot/dts/am437x-sk-evm.dts                 |  1 +
 arch/arm/boot/dts/am43x-epos-evm.dts                |  2 +-
 arch/arm/mach-omap2/board-generic.c                 |  1 +
 arch/arm/mach-omap2/id.c                            |  2 ++
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c          | 16 ++++++++++++++--
 arch/arm/mach-omap2/soc.h                           |  7 +++++--
 drivers/rtc/rtc-omap.c                              | 10 ++++++++++
 12 files changed, 50 insertions(+), 6 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Keerthy]

Compared to da830-rtc compatibility am3352-rtc is more compatible to
the one in am437x. Hence adding the am3352-rtc compatible to cover the
entire feature set.

The ti,am4372-rtc has no Documentation and not used even in the driver
hence removing it.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am4372.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ade28c79..40c01c4 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -308,7 +308,7 @@
 		};
 
 		rtc: rtc@44e3e000 {
-			compatible = "ti,am4372-rtc","ti,da830-rtc";
+			compatible = "ti,am3352-rtc", "ti,da830-rtc";
 			reg = <0x44e3e000 0x1000>;
 			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH
 				      GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/8] ARM: dts: am437x-gp-evm: Add regulator-always-on and regulator-boot-on for RTC DCDCs

[Keerthy]

From: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>

DCDC5 and DCDC6 supply rtc and need to be on for accessing the module.

On A1 revision of the TPS65218, FSEAL bit would be undefined without
coin-cell present which in many cases led to it being set, causing DCDC5
and DCDC6 to stay active, but also leading to unexplained failures when
it was not. On B1 revision, FSEAL is always 0 when no coin-cell is present
so this patch is required on boards with B1 revision to ever work. This
implementation works on boards with either A1 or B1 revision and makes
sure that DCDC5 and DCDC6 always stay active.

Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am437x-gp-evm.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 84aa30c..816027b 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -455,6 +455,8 @@
 			regulator-name = "v1_0bat";
 			regulator-min-microvolt = <1000000>;
 			regulator-max-microvolt = <1000000>;
+			regulator-boot-on;
+			regulator-always-on;
 		};
 
 		dcdc6: regulator-dcdc6 {
@@ -462,6 +464,8 @@
 			regulator-name = "v1_8bat";
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
+			regulator-boot-on;
+			regulator-always-on;
 		};
 
 		ldo1: regulator-ldo1 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] rtc: omap: Add external clock enabling support

[Keerthy]

Based on the board property switch the source from internal
to external clock. Switching to external source is needed for
rtcwake to work in low power modes.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |  2 ++
 drivers/rtc/rtc-omap.c                             | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index 4ba4dbd..0c44755 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -15,6 +15,7 @@ Required properties:
 Optional properties:
 - system-power-controller: whether the rtc is controlling the system power
   through pmic_power_en
+- ext-clk-src: Whether the rtc can be sourced by external clock
 
 Example:
 
@@ -25,4 +26,5 @@ rtc@1c23000 {
 		      19>;
 	interrupt-parent = <&intc>;
 	system-power-controller;
+	ext-clk-src;
 };
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 8b6355f..cb8936a 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -107,6 +107,7 @@
 
 /* OMAP_RTC_OSC_REG bit fields: */
 #define OMAP_RTC_OSC_32KCLK_EN		BIT(6)
+#define OMAP_RTC_OSC_SEL_32KCLK_SRC	BIT(3)
 
 /* OMAP_RTC_IRQWAKEEN bit fields: */
 #define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN	BIT(1)
@@ -136,6 +137,7 @@ struct omap_rtc {
 	int irq_timer;
 	u8 interrupts_reg;
 	bool is_pmic_controller;
+	bool is_ext_src;
 	const struct omap_rtc_device_type *type;
 };
 
@@ -540,6 +542,8 @@ static int omap_rtc_probe(struct platform_device *pdev)
 		rtc->is_pmic_controller = rtc->type->has_pmic_mode &&
 				of_property_read_bool(pdev->dev.of_node,
 						"system-power-controller");
+		rtc->is_ext_src = of_property_read_bool(pdev->dev.of_node,
+						"ext-clk-src");
 	} else {
 		id_entry = platform_get_device_id(pdev);
 		rtc->type = (void *)id_entry->driver_data;
@@ -627,6 +631,12 @@ static int omap_rtc_probe(struct platform_device *pdev)
 	if (reg != new_ctrl)
 		rtc_write(rtc, OMAP_RTC_CTRL_REG, new_ctrl);
 
+	if (rtc->is_ext_src) {
+		reg = rtc_read(rtc, OMAP_RTC_OSC_REG);
+		rtc_writel(rtc, OMAP_RTC_OSC_REG,
+			   reg | OMAP_RTC_OSC_SEL_32KCLK_SRC);
+	}
+
 	rtc->type->lock(rtc);
 
 	device_init_wakeup(&pdev->dev, true);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] ARM: dts: AM4372: Add external clock flag

[Keerthy]

The rtc IP has a bit to switch source from internal to external
clock. Adding a flag to convey the source.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am437x-idk-evm.dts | 1 +
 arch/arm/boot/dts/am437x-sk-evm.dts  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am437x-idk-evm.dts b/arch/arm/boot/dts/am437x-idk-evm.dts
index 37834427..f7d8656 100644
--- a/arch/arm/boot/dts/am437x-idk-evm.dts
+++ b/arch/arm/boot/dts/am437x-idk-evm.dts
@@ -395,6 +395,7 @@
 
 &rtc {
 	status = "okay";
+	ext-clk-src;
 };
 
 &wdt {
diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index c17097d..2698884 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -671,6 +671,7 @@
 
 &rtc {
 	status = "okay";
+	ext-clk-src;
 };
 
 &wdt {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/8] ARM: AM43xx: Introduce a separate soc_is function for am438x series of SoCs

[Keerthy]

EPOS evms are fitted with a separate family of am43xx SoCs and are named
am438x series. Adding a separate soc_is function to identify that
particular series of SoCs. This can be done to avoid unnecessarily
registering hwmods like rtc when not needed on EPOS evms.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/omap/omap.txt | 3 +++
 arch/arm/mach-omap2/board-generic.c                 | 1 +
 arch/arm/mach-omap2/id.c                            | 2 ++
 arch/arm/mach-omap2/soc.h                           | 7 +++++--
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
index 4f6a82c..97717e5 100644
--- a/Documentation/devicetree/bindings/arm/omap/omap.txt
+++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
@@ -100,6 +100,9 @@ SoCs:
 - AM4372
   compatible = "ti,am4372", "ti,am43"
 
+- AM438x
+  compatible = "ti,am438x", "ti,am43"
+
 Boards:
 
 - OMAP3 BeagleBoard : Low cost community board
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 34ff14b..3d70d6a 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -266,6 +266,7 @@ MACHINE_END
 #ifdef CONFIG_SOC_AM43XX
 static const char *const am43_boards_compat[] __initconst = {
 	"ti,am4372",
+	"ti,am438x",
 	"ti,am43",
 	NULL,
 };
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index e3f713f..1bab9f5 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -228,6 +228,8 @@ static void __init omap3_cpuinfo(void)
 		cpu_name =  "AM335X";
 	} else if (soc_is_am437x()) {
 		cpu_name =  "AM437x";
+	} else if (soc_is_am438x()) {
+		cpu_name =  "AM438X";
 	} else if (cpu_is_ti814x()) {
 		cpu_name = "TI814X";
 	} else if (omap3_has_iva() && omap3_has_sgx()) {
diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index f97654d..feb27fd 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -238,6 +238,7 @@ IS_AM_SUBCLASS(437x, 0x437)
 #define soc_is_am335x()			0
 #define soc_is_am43xx()			0
 #define soc_is_am437x()			0
+#define soc_is_am438x()			0
 #define cpu_is_omap44xx()		0
 #define cpu_is_omap443x()		0
 #define cpu_is_omap446x()		0
@@ -371,8 +372,10 @@ IS_OMAP_TYPE(3430, 0x3430)
 #ifdef	CONFIG_SOC_AM43XX
 # undef soc_is_am43xx
 # undef soc_is_am437x
-# define soc_is_am43xx()		is_am43xx()
-# define soc_is_am437x()		is_am437x()
+# undef soc_is_am438x
+# define soc_is_am43xx()		of_machine_is_compatible("ti,am43")
+# define soc_is_am437x()		of_machine_is_compatible("ti,am4372")
+# define soc_is_am438x()		of_machine_is_compatible("ti,am438x")
 #endif
 
 # if defined(CONFIG_ARCH_OMAP4)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/8] ARM: dts: am43x-epos-evm: Add the am438 compatible string

[Keerthy]

The SoCs on am43x-epos-evm are named am438x.
Hence add the compatiblity string and remove the am4372 string.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index 795d68a..6b9a170 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -17,7 +17,7 @@
 
 / {
 	model = "TI AM43x EPOS EVM";
-	compatible = "ti,am43x-epos-evm","ti,am4372","ti,am43";
+	compatible = "ti,am43x-epos-evm","ti,am438x","ti,am43";
 
 	aliases {
 		display0 = &lcd0;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] ARM: AM43XX: HWMOD: Add rtc hwmod

[Keerthy]

The patch adds rtc hwmod. This is present on gp and sk evm and not on
epos evm. Hence adding it selectively using a seprate list.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/omap_hwmod_43xx_data.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_43xx_data.c b/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
index 215d5ef..a0850b9 100644
--- a/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_43xx_data.c
@@ -21,7 +21,7 @@
 #include "prcm43xx.h"
 #include "omap_hwmod_common_data.h"
 #include "hdq1w.h"
-
+#include "soc.h"
 
 /* IP blocks */
 static struct omap_hwmod am43xx_emif_hwmod = {
@@ -1020,9 +1020,21 @@ static struct omap_hwmod_ocp_if *am43xx_hwmod_ocp_ifs[] __initdata = {
 	NULL,
 };
 
+static struct omap_hwmod_ocp_if *am43xx_rtc_hwmod_ocp_ifs[] __initdata = {
+	&am33xx_l4_wkup__rtc,
+	NULL,
+};
+
 int __init am43xx_hwmod_init(void)
 {
+	int ret;
+
 	omap_hwmod_am43xx_reg();
 	omap_hwmod_init();
-	return omap_hwmod_register_links(am43xx_hwmod_ocp_ifs);
+	ret = omap_hwmod_register_links(am43xx_hwmod_ocp_ifs);
+
+	if (!soc_is_am438x())
+		ret = omap_hwmod_register_links(am43xx_rtc_hwmod_ocp_ifs);
+
+	return ret;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] ARM: dts: AM437x-gp-evm: Enable RTC

[Keerthy]

Enable RTC and select external clock as the source so that RTC
ticks even during suspend/resume.

Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts
index 816027b..873dc29 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -794,3 +794,8 @@
 		};
 	};
 };
+
+&rtc {
+	status = "okay";
+	ext-clk-src;
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Tony Lindgren]

* Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
> Based on the board property switch the source from internal
> to external clock. Switching to external source is needed for
> rtcwake to work in low power modes.

I think this is better handled based on the compatible string
in the device driver rather than introducing a custom dts
property for it. You can just set the quirk flag in the driver
probe based on the compatible.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] ARM: AM43xx: Introduce a separate soc_is function for am438x series of SoCs

[Tony Lindgren]

* Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -228,6 +228,8 @@ static void __init omap3_cpuinfo(void)
>  		cpu_name =  "AM335X";
>  	} else if (soc_is_am437x()) {
>  		cpu_name =  "AM437x";
> +	} else if (soc_is_am438x()) {
> +		cpu_name =  "AM438X";
>  	} else if (cpu_is_ti814x()) {
>  		cpu_name = "TI814X";
>  	} else if (omap3_has_iva() && omap3_has_sgx()) {
> --- a/arch/arm/mach-omap2/soc.h
> +++ b/arch/arm/mach-omap2/soc.h
> @@ -238,6 +238,7 @@ IS_AM_SUBCLASS(437x, 0x437)
>  #define soc_is_am335x()			0
>  #define soc_is_am43xx()			0
>  #define soc_is_am437x()			0
> +#define soc_is_am438x()			0
>  #define cpu_is_omap44xx()		0
>  #define cpu_is_omap443x()		0
>  #define cpu_is_omap446x()		0
> @@ -371,8 +372,10 @@ IS_OMAP_TYPE(3430, 0x3430)
>  #ifdef	CONFIG_SOC_AM43XX
>  # undef soc_is_am43xx
>  # undef soc_is_am437x
> -# define soc_is_am43xx()		is_am43xx()
> -# define soc_is_am437x()		is_am437x()
> +# undef soc_is_am438x
> +# define soc_is_am43xx()		of_machine_is_compatible("ti,am43")
> +# define soc_is_am437x()		of_machine_is_compatible("ti,am4372")
> +# define soc_is_am438x()		of_machine_is_compatible("ti,am438x")
>  #endif
>  
>  # if defined(CONFIG_ARCH_OMAP4)

Nice if we can start getting rid of the early SoC detection. But
With this change to use of_machine_is_compatible we need to make
sure nothing relies on it too early in the init. And we should
then do it for all the SoCs that boot in dts mode to avoid making
them behave in a different way.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] ARM: dts: am437x-gp-evm: Add regulator-always-on and regulator-boot-on for RTC DCDCs

[Tony Lindgren]

* Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
> From: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> 
> DCDC5 and DCDC6 supply rtc and need to be on for accessing the module.
> 
> On A1 revision of the TPS65218, FSEAL bit would be undefined without
> coin-cell present which in many cases led to it being set, causing DCDC5
> and DCDC6 to stay active, but also leading to unexplained failures when
> it was not. On B1 revision, FSEAL is always 0 when no coin-cell is present
> so this patch is required on boards with B1 revision to ever work. This
> implementation works on boards with either A1 or B1 revision and makes
> sure that DCDC5 and DCDC6 always stay active.
> 
> Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org>

Applying patches 1 & 2 into omap-for-v4.3/dt thanks.

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Alexandre Belloni]

Hi,

On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
> * Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
> > Based on the board property switch the source from internal
> > to external clock. Switching to external source is needed for
> > rtcwake to work in low power modes.
> 
> I think this is better handled based on the compatible string
> in the device driver rather than introducing a custom dts
> property for it. You can just set the quirk flag in the driver
> probe based on the compatible.
> 

Why not use the clocks property? Then you can pass an external clock. If
it is present you can even get its rate if this is needed at some point
in the future. You could also disable it when going to suspend.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Alexandre Belloni]

On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :
> Hi,
> 
> On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
> > * Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
> > > Based on the board property switch the source from internal
> > > to external clock. Switching to external source is needed for
> > > rtcwake to work in low power modes.
> > 
> > I think this is better handled based on the compatible string
> > in the device driver rather than introducing a custom dts
> > property for it. You can just set the quirk flag in the driver
> > probe based on the compatible.
> > 
> 
> Why not use the clocks property? Then you can pass an external clock. If
> it is present you can even get its rate if this is needed at some point
> in the future. You could also disable it when going to suspend.
> 

Actually, that was already my suggestion back in april:
http://patchwork.ozlabs.org/patch/445631/

(Please Cc: the rtc mailing list for RTC related patches so that they
get picked up by patchwork).

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Keerthy]

On Wednesday 05 August 2015 05:13 PM, Alexandre Belloni wrote:
> On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :
>> Hi,
>>
>> On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
>>> * Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
>>>> Based on the board property switch the source from internal
>>>> to external clock. Switching to external source is needed for
>>>> rtcwake to work in low power modes.
>>>
>>> I think this is better handled based on the compatible string
>>> in the device driver rather than introducing a custom dts
>>> property for it. You can just set the quirk flag in the driver
>>> probe based on the compatible.
>>>
>>
>> Why not use the clocks property? Then you can pass an external clock. If
>> it is present you can even get its rate if this is needed at some point
>> in the future. You could also disable it when going to suspend.
>>
>
> Actually, that was already my suggestion back in april:
> http://patchwork.ozlabs.org/patch/445631/
>
> (Please Cc: the rtc mailing list for RTC related patches so that they
> get picked up by patchwork).

Hi Alexandre,

This is a special one where in the enable bit is present in the rtc 
register space and not in the prcm register space. Since there was a 
concern on the external clock not being present i added a board dts flag.

Regards,
Keerthy
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Alexandre Belloni]

On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :
> This is a special one where in the enable bit is present in the rtc register
> space and not in the prcm register space. Since there was a concern on the
> external clock not being present i added a board dts flag.
> 

So you mean this external clock is coming internally from the SoC?

Do you have a link to the datasheet?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Felipe Balbi]

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

On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
> Compared to da830-rtc compatibility am3352-rtc is more compatible to
> the one in am437x. Hence adding the am3352-rtc compatible to cover the
> entire feature set.
> 
> The ti,am4372-rtc has no Documentation and not used even in the driver
> hence removing it.

why don't you do the inverse ? Document am4372-rtc and make driver use
it ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Keerthy]

Felipe,

On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
> On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
>> Compared to da830-rtc compatibility am3352-rtc is more compatible to
>> the one in am437x. Hence adding the am3352-rtc compatible to cover the
>> entire feature set.
>>
>> The ti,am4372-rtc has no Documentation and not used even in the driver
>> hence removing it.
>
> why don't you do the inverse ? Document am4372-rtc and make driver use
> it ?

am3352-rtc suffices for am4372 too. No need to add additional one for 
am4372.


>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Felipe Balbi]

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

On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
> Felipe,
> 
> On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
> >On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
> >>Compared to da830-rtc compatibility am3352-rtc is more compatible to
> >>the one in am437x. Hence adding the am3352-rtc compatible to cover the
> >>entire feature set.
> >>
> >>The ti,am4372-rtc has no Documentation and not used even in the driver
> >>hence removing it.
> >
> >why don't you do the inverse ? Document am4372-rtc and make driver use
> >it ?
> 
> am3352-rtc suffices for am4372 too. No need to add additional one for
> am4372.

Until we end up needing it, right ? :-)

Besides, it's already used in a DTS. What happens if someone branched
from that DTS and ships that in a product. RTC will just stop working
for them. Sure, it wasn't documented, but that's a problem of commit
73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
nodes) which, essentially, added that compatible flag without
documenting it.

BTW, this compatible has been in tree since August 2013, IMO it's unfar
to drop it just like that. Documenting it would be a better approach.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Keerthy]

On Wednesday 05 August 2015 09:44 PM, Felipe Balbi wrote:
> On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
>> Felipe,
>>
>> On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
>>> On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
>>>> Compared to da830-rtc compatibility am3352-rtc is more compatible to
>>>> the one in am437x. Hence adding the am3352-rtc compatible to cover the
>>>> entire feature set.
>>>>
>>>> The ti,am4372-rtc has no Documentation and not used even in the driver
>>>> hence removing it.
>>>
>>> why don't you do the inverse ? Document am4372-rtc and make driver use
>>> it ?
>>
>> am3352-rtc suffices for am4372 too. No need to add additional one for
>> am4372.
>
> Until we end up needing it, right ? :-)
>
> Besides, it's already used in a DTS. What happens if someone branched
> from that DTS and ships that in a product. RTC will just stop working
> for them. Sure, it wasn't documented, but that's a problem of commit
> 73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
> nodes) which, essentially, added that compatible flag without
> documenting it.
>
> BTW, this compatible has been in tree since August 2013, IMO it's unfar
> to drop it just like that. Documenting it would be a better approach.

Okay. Can you point me to a file which is already accessing it in dts?

>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Felipe Balbi]

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

On Wed, Aug 05, 2015 at 09:48:08PM +0530, Keerthy wrote:
> 
> 
> On Wednesday 05 August 2015 09:44 PM, Felipe Balbi wrote:
> >On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
> >>Felipe,
> >>
> >>On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
> >>>On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
> >>>>Compared to da830-rtc compatibility am3352-rtc is more compatible to
> >>>>the one in am437x. Hence adding the am3352-rtc compatible to cover the
> >>>>entire feature set.
> >>>>
> >>>>The ti,am4372-rtc has no Documentation and not used even in the driver
> >>>>hence removing it.
> >>>
> >>>why don't you do the inverse ? Document am4372-rtc and make driver use
> >>>it ?
> >>
> >>am3352-rtc suffices for am4372 too. No need to add additional one for
> >>am4372.
> >
> >Until we end up needing it, right ? :-)
> >
> >Besides, it's already used in a DTS. What happens if someone branched
> >from that DTS and ships that in a product. RTC will just stop working
> >for them. Sure, it wasn't documented, but that's a problem of commit
> >73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
> >nodes) which, essentially, added that compatible flag without
> >documenting it.
> >
> >BTW, this compatible has been in tree since August 2013, IMO it's unfar
> >to drop it just like that. Documenting it would be a better approach.
> 
> Okay. Can you point me to a file which is already accessing it in dts?

Accessing what ? Also, once DTS reaches a major kernel release, it's
deemed stable and should be supported. Are we dropping that ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Keerthy]

On Wednesday 05 August 2015 10:21 PM, Felipe Balbi wrote:
> On Wed, Aug 05, 2015 at 09:48:08PM +0530, Keerthy wrote:
>>
>>
>> On Wednesday 05 August 2015 09:44 PM, Felipe Balbi wrote:
>>> On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
>>>> Felipe,
>>>>
>>>> On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
>>>>> On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
>>>>>> Compared to da830-rtc compatibility am3352-rtc is more compatible to
>>>>>> the one in am437x. Hence adding the am3352-rtc compatible to cover the
>>>>>> entire feature set.
>>>>>>
>>>>>> The ti,am4372-rtc has no Documentation and not used even in the driver
>>>>>> hence removing it.
>>>>>
>>>>> why don't you do the inverse ? Document am4372-rtc and make driver use
>>>>> it ?
>>>>
>>>> am3352-rtc suffices for am4372 too. No need to add additional one for
>>>> am4372.
>>>
>>> Until we end up needing it, right ? :-)
>>>
>>> Besides, it's already used in a DTS. What happens if someone branched
>> >from that DTS and ships that in a product. RTC will just stop working
>>> for them. Sure, it wasn't documented, but that's a problem of commit
>>> 73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
>>> nodes) which, essentially, added that compatible flag without
>>> documenting it.
>>>
>>> BTW, this compatible has been in tree since August 2013, IMO it's unfar
>>> to drop it just like that. Documenting it would be a better approach.
>>
>> Okay. Can you point me to a file which is already accessing it in dts?
>
> Accessing what ? Also, once DTS reaches a major kernel release, it's
> deemed stable and should be supported. Are we dropping that ?

I meant getting used in any other dts files than the one i just dropped 
it. I checked the driver for rtc-omap and the compatibles are 
ti,am3352-rtc, ti,da830-rtc. So instead of appending it i dropped it as 
it was unused before. Tony i guess has already pulled in this patch. So 
i will let Tony comment on this. If the am4372-rtc compatible needs to 
be there i can add more documentation and append am3352-rtc.

Regards,
Keerthy
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Keerthy]

On Wednesday 05 August 2015 06:05 PM, Alexandre Belloni wrote:
> On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :
>> This is a special one where in the enable bit is present in the rtc register
>> space and not in the prcm register space. Since there was a concern on the
>> external clock not being present i added a board dts flag.
>>
>
> So you mean this external clock is coming internally from the SoC?

No what i meant is external clock is coming from Oscillator OSC1 
@32.768KHz but the controlling bits are part of rtc register space.

TRM: http://www.ti.com/lit/ug/spruhl7c/spruhl7c.pdf

Section: 19.4.3.2 Clock Source Page 2836

Also register details:
19.4.5.19 RTCSS_OSC_REG Register (offset = 54h) [reset = 10h]

Page 2865.

Regards,
Keerthy

>
> Do you have a link to the datasheet?
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Alexandre Belloni]

On 06/08/2015 at 07:39:52 +0530, Keerthy wrote :
> On Wednesday 05 August 2015 06:05 PM, Alexandre Belloni wrote:
> >On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :
> >>This is a special one where in the enable bit is present in the rtc register
> >>space and not in the prcm register space. Since there was a concern on the
> >>external clock not being present i added a board dts flag.
> >>
> >
> >So you mean this external clock is coming internally from the SoC?
> 
> No what i meant is external clock is coming from Oscillator OSC1 @32.768KHz
> but the controlling bits are part of rtc register space.
> 
> TRM: http://www.ti.com/lit/ug/spruhl7c/spruhl7c.pdf
> 
> Section: 19.4.3.2 Clock Source Page 2836
> 
> Also register details:
> 19.4.5.19 RTCSS_OSC_REG Register (offset = 54h) [reset = 10h]
> 
> Page 2865.
> 

This confirms what I'm saying. Your issue here is that the driver is not
properly taking the clocks so when the PRCM is disabling CLK_32KHZ, you
end up without any clock.

You can use the clocks property in the device tree and pass two clocks,
the prcm one and the external crystal/external oscillator.
In the driver, you get both clock, clk_get_rate on the external one will
help you know whether it is populated or not (this will be 0 or 32768).
It is is populated, use it by writing 32KCLK_SEL.

Bonus points if you use the clock-accuracy and decide to switch between
PRCM and the external clock when going to suspend and resuming. I guess
an external RC oscillator is quite bad versus the PRCM.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Grygorii Strashko]

Hi Alexandre,
On 08/05/2015 02:43 PM, Alexandre Belloni wrote:
> On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :
>> Hi,
>>
>> On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
>>> * Keerthy <j-keerthy-l0cyMroinI0@public.gmane.org> [150805 03:53]:
>>>> Based on the board property switch the source from internal
>>>> to external clock. Switching to external source is needed for
>>>> rtcwake to work in low power modes.
>>>
>>> I think this is better handled based on the compatible string
>>> in the device driver rather than introducing a custom dts
>>> property for it. You can just set the quirk flag in the driver
>>> probe based on the compatible.
>>>
>>
>> Why not use the clocks property? Then you can pass an external clock. If
>> it is present you can even get its rate if this is needed at some point
>> in the future. You could also disable it when going to suspend.
>>
> 
> Actually, that was already my suggestion back in april:
> http://patchwork.ozlabs.org/patch/445631/
> 
> (Please Cc: the rtc mailing list for RTC related patches so that they
> get picked up by patchwork).
> 

Pls, correct me if I'm not right. Is below what you propose?

Doard dts:
/ {
 rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
	#clock-cells = <0>;
	compatible = "fixed-clock";
	clock-frequency = <32000>;
  	clock-output-names = "rtc_osc_xi_clkin32";
  };
}

 &rtc {
 	status = "okay";
	clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
	[optional] clock-names = "int-clk", "ext-clk";
 };

Driver:
1) clk0 is mandatory, internal clock source
2) clk1 is optional, external clock source, so
if present - RTC driver can switch to use ext clock source

-- 
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Alexandre Belloni]

On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
> Pls, correct me if I'm not right. Is below what you propose?
> 
> Doard dts:
> / {
>  rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
> 	#clock-cells = <0>;
> 	compatible = "fixed-clock";
> 	clock-frequency = <32000>;
>   	clock-output-names = "rtc_osc_xi_clkin32";
>   };
> }
> 
>  &rtc {
>  	status = "okay";
> 	clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
> 	[optional] clock-names = "int-clk", "ext-clk";
>  };
> 
> Driver:
> 1) clk0 is mandatory, internal clock source
> 2) clk1 is optional, external clock source, so
> if present - RTC driver can switch to use ext clock source
> 

Absolutely!

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Tony Lindgren]

* Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> [150806 02:50]:
> On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
> > Pls, correct me if I'm not right. Is below what you propose?
> > 
> > Doard dts:
> > / {
> >  rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
> > 	#clock-cells = <0>;
> > 	compatible = "fixed-clock";
> > 	clock-frequency = <32000>;
> >   	clock-output-names = "rtc_osc_xi_clkin32";
> >   };
> > }
> > 
> >  &rtc {
> >  	status = "okay";
> > 	clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
> > 	[optional] clock-names = "int-clk", "ext-clk";
> >  };
> > 
> > Driver:
> > 1) clk0 is mandatory, internal clock source
> > 2) clk1 is optional, external clock source, so
> > if present - RTC driver can switch to use ext clock source
> > 
> 
> Absolutely!

Sounds good to me too.

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Afzal Mohammed]

Hi,

On Wed, Aug 05, 2015 at 11:14:45AM -0500, Felipe Balbi wrote:

> for them. Sure, it wasn't documented, but that's a problem of commit
> 73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
> nodes) which, essentially, added that compatible flag without
> documenting it.

It was required to be done that way that time, probably author didn't
want to be loyal to the king than the king himself ;), see,

1. http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/191206.html
2. http://www.mail-archive.com/linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg89473.html

Regards
Afzal
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Felipe Balbi]

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

On Thu, Aug 06, 2015 at 06:55:57AM +0530, Keerthy wrote:
> 
> 
> On Wednesday 05 August 2015 10:21 PM, Felipe Balbi wrote:
> >On Wed, Aug 05, 2015 at 09:48:08PM +0530, Keerthy wrote:
> >>
> >>
> >>On Wednesday 05 August 2015 09:44 PM, Felipe Balbi wrote:
> >>>On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
> >>>>Felipe,
> >>>>
> >>>>On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
> >>>>>On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
> >>>>>>Compared to da830-rtc compatibility am3352-rtc is more compatible to
> >>>>>>the one in am437x. Hence adding the am3352-rtc compatible to cover the
> >>>>>>entire feature set.
> >>>>>>
> >>>>>>The ti,am4372-rtc has no Documentation and not used even in the driver
> >>>>>>hence removing it.
> >>>>>
> >>>>>why don't you do the inverse ? Document am4372-rtc and make driver use
> >>>>>it ?
> >>>>
> >>>>am3352-rtc suffices for am4372 too. No need to add additional one for
> >>>>am4372.
> >>>
> >>>Until we end up needing it, right ? :-)
> >>>
> >>>Besides, it's already used in a DTS. What happens if someone branched
> >>>from that DTS and ships that in a product. RTC will just stop working
> >>>for them. Sure, it wasn't documented, but that's a problem of commit
> >>>73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
> >>>nodes) which, essentially, added that compatible flag without
> >>>documenting it.
> >>>
> >>>BTW, this compatible has been in tree since August 2013, IMO it's unfar
> >>>to drop it just like that. Documenting it would be a better approach.
> >>
> >>Okay. Can you point me to a file which is already accessing it in dts?
> >
> >Accessing what ? Also, once DTS reaches a major kernel release, it's
> >deemed stable and should be supported. Are we dropping that ?
> 
> I meant getting used in any other dts files than the one i just dropped it.

how can you ever know that for sure ? There are already quite a few
third party platforms based on AM437x, how can you be sure those
companies don't have their own DTS ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Keerthy]

On Thursday 06 August 2015 07:46 PM, Felipe Balbi wrote:
> On Thu, Aug 06, 2015 at 06:55:57AM +0530, Keerthy wrote:
>>
>>
>> On Wednesday 05 August 2015 10:21 PM, Felipe Balbi wrote:
>>> On Wed, Aug 05, 2015 at 09:48:08PM +0530, Keerthy wrote:
>>>>
>>>>
>>>> On Wednesday 05 August 2015 09:44 PM, Felipe Balbi wrote:
>>>>> On Wed, Aug 05, 2015 at 09:21:05PM +0530, Keerthy wrote:
>>>>>> Felipe,
>>>>>>
>>>>>> On Wednesday 05 August 2015 09:01 PM, Felipe Balbi wrote:
>>>>>>> On Wed, Aug 05, 2015 at 04:19:45PM +0530, Keerthy wrote:
>>>>>>>> Compared to da830-rtc compatibility am3352-rtc is more compatible to
>>>>>>>> the one in am437x. Hence adding the am3352-rtc compatible to cover the
>>>>>>>> entire feature set.
>>>>>>>>
>>>>>>>> The ti,am4372-rtc has no Documentation and not used even in the driver
>>>>>>>> hence removing it.
>>>>>>>
>>>>>>> why don't you do the inverse ? Document am4372-rtc and make driver use
>>>>>>> it ?
>>>>>>
>>>>>> am3352-rtc suffices for am4372 too. No need to add additional one for
>>>>>> am4372.
>>>>>
>>>>> Until we end up needing it, right ? :-)
>>>>>
>>>>> Besides, it's already used in a DTS. What happens if someone branched
>>>> >from that DTS and ships that in a product. RTC will just stop working
>>>>> for them. Sure, it wasn't documented, but that's a problem of commit
>>>>> 73456012734b80442b33916406cfd13bf1b73acb (ARM: dts: AM4372: add few
>>>>> nodes) which, essentially, added that compatible flag without
>>>>> documenting it.
>>>>>
>>>>> BTW, this compatible has been in tree since August 2013, IMO it's unfar
>>>>> to drop it just like that. Documenting it would be a better approach.
>>>>
>>>> Okay. Can you point me to a file which is already accessing it in dts?
>>>
>>> Accessing what ? Also, once DTS reaches a major kernel release, it's
>>> deemed stable and should be supported. Are we dropping that ?
>>
>> I meant getting used in any other dts files than the one i just dropped it.
>
> how can you ever know that for sure ? There are already quite a few
> third party platforms based on AM437x, how can you be sure those
> companies don't have their own DTS ?

Felipe,

If that is a concern i can re-do this.

Tony,

Shall i re-do this patch without removing am4372-rtc? If you can drop 
this i can re-do without removing the original am4372 compatible.

Regards,
Keerthy

>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] ARM: dts: AM4372: Reorder the rtc compatible string

[Tony Lindgren]

* Keerthy <a0393675-l0cyMroinI0@public.gmane.org> [150806 09:51]:
> 
> Shall i re-do this patch without removing am4372-rtc? If you can drop this i
> can re-do without removing the original am4372 compatible.

OK please send a patch reverting the compatible change against the
current omap-for-v4.3/dt-v2 branch. Please also describe why that's
needed :)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

[Keerthy]

On Thursday 06 August 2015 03:21 PM, Tony Lindgren wrote:
> * Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> [150806 02:50]:
>> On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
>>> Pls, correct me if I'm not right. Is below what you propose?
>>>
>>> Doard dts:
>>> / {
>>>   rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
>>> 	#clock-cells = <0>;
>>> 	compatible = "fixed-clock";
>>> 	clock-frequency = <32000>;
>>>    	clock-output-names = "rtc_osc_xi_clkin32";
>>>    };
>>> }
>>>
>>>   &rtc {
>>>   	status = "okay";
>>> 	clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
>>> 	[optional] clock-names = "int-clk", "ext-clk";
>>>   };
>>>
>>> Driver:
>>> 1) clk0 is mandatory, internal clock source
>>> 2) clk1 is optional, external clock source, so
>>> if present - RTC driver can switch to use ext clock source

Thanks Grygorii. I will implement it this way.

>>>
>>
>> Absolutely!
>
> Sounds good to me too.
>
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html