[PATCH v2 0/4] DRA7 Timer12 Support

[PATCH v2 0/4] DRA7 Timer12 Support

[Suman Anna]

Hi Tony,

This is a revised version of the DRA7 Timer12 support patch series [1].
The series mainly is respun to fix a build issue caused by Patch1 [2]
with omap1_defconfig due to the absence of Common Clock framework for
OMAP1.

Only Patch 1 is modified from the previous series. I have also revised
the check slightly to fix another issue seen when requesting Timer12
on OMAP3, as that is a fixed clock and does return back with a parent
count of 1.

Series baselined on v4.3-rc3 just like the original series, and tested
on all generations (AMx3x, OMAP4, OMAP5 and DRA7) except OMAP1 and OMAP2
generations. 

regards
Suman

[1] http://marc.info/?l=linux-omap&m=144372799601354&w=2
[2] https://patchwork.kernel.org/patch/7311021/

Suman Anna (4):
  ARM: OMAP: dmtimer: check for fixed timers during config
  ARM: OMAP2+: timer: Remove secure timer for DRA7xx HS devices
  ARM: dts: DRA7: Add timer12 node
  ARM: DRA7: hwmod: Add data for GPTimer 12

 arch/arm/boot/dts/dra7.dtsi               | 10 +++++++++
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +++++++++++++++++++++++++++++--
 arch/arm/mach-omap2/timer.c               |  6 +++---
 arch/arm/plat-omap/dmtimer.c              |  7 ++++++
 4 files changed, 54 insertions(+), 5 deletions(-)

-- 
2.6.0

[PATCH v2 1/4] ARM: OMAP: dmtimer: check for fixed timers during config

[Suman Anna]

The omap_dm_timer_set_source() function provides a means for client
users to configure the mux parent for a GPTimer's functional clock.
However, not all timers are configurable (Eg: Timer12 on DRA7 is fed
by an internal 32k oscillator clock, and does not have configurable
parent clocks). So, check for such cases and proceed with out throwing
an error.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/plat-omap/dmtimer.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 8ca94d379bc3..4c48b52c4a7c 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -36,6 +36,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/device.h>
@@ -504,6 +505,12 @@ int omap_dm_timer_set_source(struct omap_dm_timer *timer, int source)
 	if (IS_ERR(timer->fclk))
 		return -EINVAL;
 
+#if defined(CONFIG_COMMON_CLK)
+	/* Check if the clock has configurable parents */
+	if (clk_hw_get_num_parents(__clk_get_hw(timer->fclk)) < 2)
+		return 0;
+#endif
+
 	switch (source) {
 	case OMAP_TIMER_SRC_SYS_CLK:
 		parent_name = "timer_sys_ck";
-- 
2.6.0

[PATCH v2 2/4] ARM: OMAP2+: timer: Remove secure timer for DRA7xx HS devices

[Suman Anna]

Timer 12 on DRA7 SoCs is reserved for secure usage on high-secure (HS)
devices. The timer cannot be used by the kernel on HS devices, but is
available on regular general purpose (GP) devices. This is similar to the
behavior on OMAP3 devices, so extend the logic used in commit ad24bde8f102
("ARM: OMAP3: Dynamically disable secure timer nodes for secure devices")
to remove the secure timer on DRA7xx SoCs at run-time based on the SoC
device type.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/timer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index a55655127ef2..1e346aa0a687 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -193,8 +193,8 @@ static struct device_node * __init omap_get_timer_dt(const struct of_device_id *
 /**
  * omap_dmtimer_init - initialisation function when device tree is used
  *
- * For secure OMAP3 devices, timers with device type "timer-secure" cannot
- * be used by the kernel as they are reserved. Therefore, to prevent the
+ * For secure OMAP3/DRA7xx devices, timers with device type "timer-secure"
+ * cannot be used by the kernel as they are reserved. Therefore, to prevent the
  * kernel registering these devices remove them dynamically from the device
  * tree on boot.
  */
@@ -202,7 +202,7 @@ static void __init omap_dmtimer_init(void)
 {
 	struct device_node *np;
 
-	if (!cpu_is_omap34xx())
+	if (!cpu_is_omap34xx() && !soc_is_dra7xx())
 		return;
 
 	/* If we are a secure device, remove any secure timer nodes */
-- 
2.6.0

[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node

[Suman Anna]

Add the DT node for Timer12 present on DRA7 family of
SoCs. Timer12 is present in PD_WKUPAON power domain, and
has the same capabilities as the other timers, except for
the fact that it serves as a secure timer on HS devices
and is clocked only from the secure 32K clock.

The node is marked disabled for now, and the kernel should
refrain from using this secure timer on HS devices.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index e289c706d27d..37d632dad576 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -762,6 +762,16 @@
 			ti,hwmods = "timer11";
 		};
 
+		timer12: timer at 4ae20000 {
+			compatible = "ti,omap5430-timer";
+			reg = <0x4ae20000 0x80>;
+			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "timer12";
+			ti,timer-alwon;
+			ti,timer-secure;
+			status = "disabled";
+		};
+
 		timer13: timer at 48828000 {
 			compatible = "ti,omap5430-timer";
 			reg = <0x48828000 0x80>;
-- 
2.6.0

[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node

[Felipe Balbi]

Suman Anna <s-anna@ti.com> writes:

> Add the DT node for Timer12 present on DRA7 family of
> SoCs. Timer12 is present in PD_WKUPAON power domain, and
> has the same capabilities as the other timers, except for
> the fact that it serves as a secure timer on HS devices
> and is clocked only from the secure 32K clock.
>
> The node is marked disabled for now, and the kernel should
> refrain from using this secure timer on HS devices.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index e289c706d27d..37d632dad576 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -762,6 +762,16 @@
>  			ti,hwmods = "timer11";
>  		};
>  
> +		timer12: timer at 4ae20000 {
> +			compatible = "ti,omap5430-timer";
> +			reg = <0x4ae20000 0x80>;
> +			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "timer12";
> +			ti,timer-alwon;
> +			ti,timer-secure;
> +			status = "disabled";

according to Tony we should avoid using status at all for in-SoC
devices.

Tony, can you confirm I understood you correctly ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151005/72a82d63/attachment.sig>

[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [151005 17:51]:
> 
> according to Tony we should avoid using status at all for in-SoC
> devices.
> 
> Tony, can you confirm I understood you correctly ?

Yes. With status = "disabled" kernel completely ignores the
device and struct device is not created at all even with the
device being there. In general we're better off trying to
probe the device and idle it.

The only time we really want to mark something with
status = "disabled" is if some coprocessor firmware is
using that device and the kernel should not touch it at
all.

Regards,

Tony

[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node

[Suman Anna]

On 10/06/2015 02:52 AM, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [151005 17:51]:
>>
>> according to Tony we should avoid using status at all for in-SoC
>> devices.
>>
>> Tony, can you confirm I understood you correctly ?
> 
> Yes. With status = "disabled" kernel completely ignores the
> device and struct device is not created at all even with the
> device being there. In general we're better off trying to
> probe the device and idle it.
> 
> The only time we really want to mark something with
> status = "disabled" is if some coprocessor firmware is
> using that device and the kernel should not touch it at
> all.

Not always, since some of the PM clocking logic depends on the state
machine variables within the kernel.

We are also using this to deal with paper-spins (atleast in the DRA7
case) and the DTS include model, wherein certain instances may not be
present on all variations of the SoC, and enabled specifically on the
instances that matter. Obviously, it could be done the other way too,
but as far as what Nishanth mentioned sometime back, we are following
the former for DRA7.

In anycase, the status property on the Timer12 node can be removed, it
doesn't fall into the above category, and we are fixing it up properly
on HS devices in the kernel.

regards
Suman

[PATCH v2 3/4] ARM: dts: DRA7: Add timer12 node

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [151006 09:06]:
> On 10/06/2015 02:52 AM, Tony Lindgren wrote:
> > * Felipe Balbi <balbi@ti.com> [151005 17:51]:
> >>
> >> according to Tony we should avoid using status at all for in-SoC
> >> devices.
> >>
> >> Tony, can you confirm I understood you correctly ?
> > 
> > Yes. With status = "disabled" kernel completely ignores the
> > device and struct device is not created at all even with the
> > device being there. In general we're better off trying to
> > probe the device and idle it.
> > 
> > The only time we really want to mark something with
> > status = "disabled" is if some coprocessor firmware is
> > using that device and the kernel should not touch it at
> > all.
> 
> Not always, since some of the PM clocking logic depends on the state
> machine variables within the kernel.
> 
> We are also using this to deal with paper-spins (atleast in the DRA7
> case) and the DTS include model, wherein certain instances may not be
> present on all variations of the SoC, and enabled specifically on the
> instances that matter. Obviously, it could be done the other way too,
> but as far as what Nishanth mentioned sometime back, we are following
> the former for DRA7.
> 
> In anycase, the status property on the Timer12 node can be removed, it
> doesn't fall into the above category, and we are fixing it up properly
> on HS devices in the kernel.

Yeah please remove the status property, that can be set to disabled
in the HS board specific file.

Applying the first two patches into omap-for-v4.5/soc thanks.

Tony

[PATCH v2 4/4] ARM: DRA7: hwmod: Add data for GPTimer 12

[Suman Anna]

Add the hwmod data for GPTimer 12. GPTimer 12 is present in
WKUPAON power domain and is clocked from a secure 32K clock.
GPTimer 12 serves as a secure timer on HS devices, but is
available for kernel on regular GP devices.

The hwmod link is registered only on GP devices. The hwmod data
also reused the existing timer class instead of reintroducing
the identical dra7xx_timer_secure_sysc class which was dropped
in commit edec17863362 ("ARM: DRA7: hwmod: Fix the hwmod class
for GPTimer4").

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 562247bced49..37a10f87fbcd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1929,6 +1929,20 @@ static struct omap_hwmod dra7xx_timer11_hwmod = {
 	},
 };
 
+/* timer12 */
+static struct omap_hwmod dra7xx_timer12_hwmod = {
+	.name		= "timer12",
+	.class		= &dra7xx_timer_hwmod_class,
+	.clkdm_name	= "wkupaon_clkdm",
+	.main_clk	= "secure_32k_clk_src_ck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = DRA7XX_CM_WKUPAON_TIMER12_CLKCTRL_OFFSET,
+			.context_offs = DRA7XX_RM_WKUPAON_TIMER12_CONTEXT_OFFSET,
+		},
+	},
+};
+
 /* timer13 */
 static struct omap_hwmod dra7xx_timer13_hwmod = {
 	.name		= "timer13",
@@ -3135,6 +3149,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__timer11 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l4_wkup -> timer12 */
+static struct omap_hwmod_ocp_if dra7xx_l4_wkup__timer12 = {
+	.master		= &dra7xx_l4_wkup_hwmod,
+	.slave		= &dra7xx_timer12_hwmod,
+	.clk		= "wkupaon_iclk_mux",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 /* l4_per3 -> timer13 */
 static struct omap_hwmod_ocp_if dra7xx_l4_per3__timer13 = {
 	.master		= &dra7xx_l4_per3_hwmod,
@@ -3429,6 +3451,13 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
 	NULL,
 };
 
+/* GP-only hwmod links */
+static struct omap_hwmod_ocp_if *dra7xx_gp_hwmod_ocp_ifs[] __initdata = {
+	&dra7xx_l4_wkup__timer12,
+	NULL,
+};
+
+/* SoC variant specific hwmod links */
 static struct omap_hwmod_ocp_if *dra74x_hwmod_ocp_ifs[] __initdata = {
 	&dra7xx_l4_per3__usb_otg_ss4,
 	NULL,
@@ -3446,9 +3475,12 @@ int __init dra7xx_hwmod_init(void)
 	ret = omap_hwmod_register_links(dra7xx_hwmod_ocp_ifs);
 
 	if (!ret && soc_is_dra74x())
-		return omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
+		ret = omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
 	else if (!ret && soc_is_dra72x())
-		return omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+		ret = omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
+
+	if (!ret && omap_type() == OMAP2_DEVICE_TYPE_GP)
+		ret = omap_hwmod_register_links(dra7xx_gp_hwmod_ocp_ifs);
 
 	return ret;
 }
-- 
2.6.0

[PATCH v2 4/4] ARM: DRA7: hwmod: Add data for GPTimer 12

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [151005 16:35]:
> Add the hwmod data for GPTimer 12. GPTimer 12 is present in
> WKUPAON power domain and is clocked from a secure 32K clock.
> GPTimer 12 serves as a secure timer on HS devices, but is
> available for kernel on regular GP devices.
> 
> The hwmod link is registered only on GP devices. The hwmod data
> also reused the existing timer class instead of reintroducing
> the identical dra7xx_timer_secure_sysc class which was dropped
> in commit edec17863362 ("ARM: DRA7: hwmod: Fix the hwmod class
> for GPTimer4").
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>

Paul should take a look at this one.

Regards,

Tony

> ---
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 36 +++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 562247bced49..37a10f87fbcd 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1929,6 +1929,20 @@ static struct omap_hwmod dra7xx_timer11_hwmod = {
>  	},
>  };
>  
> +/* timer12 */
> +static struct omap_hwmod dra7xx_timer12_hwmod = {
> +	.name		= "timer12",
> +	.class		= &dra7xx_timer_hwmod_class,
> +	.clkdm_name	= "wkupaon_clkdm",
> +	.main_clk	= "secure_32k_clk_src_ck",
> +	.prcm = {
> +		.omap4 = {
> +			.clkctrl_offs = DRA7XX_CM_WKUPAON_TIMER12_CLKCTRL_OFFSET,
> +			.context_offs = DRA7XX_RM_WKUPAON_TIMER12_CONTEXT_OFFSET,
> +		},
> +	},
> +};
> +
>  /* timer13 */
>  static struct omap_hwmod dra7xx_timer13_hwmod = {
>  	.name		= "timer13",
> @@ -3135,6 +3149,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__timer11 = {
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
> +/* l4_wkup -> timer12 */
> +static struct omap_hwmod_ocp_if dra7xx_l4_wkup__timer12 = {
> +	.master		= &dra7xx_l4_wkup_hwmod,
> +	.slave		= &dra7xx_timer12_hwmod,
> +	.clk		= "wkupaon_iclk_mux",
> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> +};
> +
>  /* l4_per3 -> timer13 */
>  static struct omap_hwmod_ocp_if dra7xx_l4_per3__timer13 = {
>  	.master		= &dra7xx_l4_per3_hwmod,
> @@ -3429,6 +3451,13 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>  	NULL,
>  };
>  
> +/* GP-only hwmod links */
> +static struct omap_hwmod_ocp_if *dra7xx_gp_hwmod_ocp_ifs[] __initdata = {
> +	&dra7xx_l4_wkup__timer12,
> +	NULL,
> +};
> +
> +/* SoC variant specific hwmod links */
>  static struct omap_hwmod_ocp_if *dra74x_hwmod_ocp_ifs[] __initdata = {
>  	&dra7xx_l4_per3__usb_otg_ss4,
>  	NULL,
> @@ -3446,9 +3475,12 @@ int __init dra7xx_hwmod_init(void)
>  	ret = omap_hwmod_register_links(dra7xx_hwmod_ocp_ifs);
>  
>  	if (!ret && soc_is_dra74x())
> -		return omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
> +		ret = omap_hwmod_register_links(dra74x_hwmod_ocp_ifs);
>  	else if (!ret && soc_is_dra72x())
> -		return omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
> +		ret = omap_hwmod_register_links(dra72x_hwmod_ocp_ifs);
> +
> +	if (!ret && omap_type() == OMAP2_DEVICE_TYPE_GP)
> +		ret = omap_hwmod_register_links(dra7xx_gp_hwmod_ocp_ifs);
>  
>  	return ret;
>  }
> -- 
> 2.6.0
>