[PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tero Kristo]

Hi,

This is an alternative solution against v3 [1] of this series.

[1] https://www.spinics.net/lists/arm-kernel/msg581965.html

This series is different in that it parses the hwmod data in the fly
and swaps the main_clk info with clkctrl implementations if those
are present. Patch #1 / #2 are old from v3 of the series, rest are
new. Patches #3 / #4 add some infrastructure support for parsing
the clock data, and patch #5 does the clock swapping.

With this series, it is not necessary to populate clkctrl clock
entries to DT at all, thus the transition should be smoother.

Integrated test branch for omap4 pushed here (contains 3 extra DT
patches on top.)

Tree: https://github.com/t-kristo/linux-pm.git
Branch: 4.12-rc1-clkctrl-integ-v3b

-Tero


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tero Kristo]

Hi,

This is an alternative solution against v3 [1] of this series.

[1]?https://www.spinics.net/lists/arm-kernel/msg581965.html

This series is different in that it parses the hwmod data in the fly
and swaps the main_clk info with clkctrl implementations if those
are present. Patch #1 / #2 are old from v3 of the series, rest are
new. Patches #3 / #4 add some infrastructure support for parsing
the clock data, and patch #5 does the clock swapping.

With this series, it is not necessary to populate clkctrl clock
entries to DT at all, thus the transition should be smoother.

Integrated test branch for omap4 pushed here (contains 3 extra DT
patches on top.)

Tree: https://github.com/t-kristo/linux-pm.git
Branch: 4.12-rc1-clkctrl-integ-v3b

-Tero

[PATCHv3b 1/5] ARM: OMAP2+: timer: add support for fetching fck handle from DT

[Tero Kristo]

The mux clock handle shall be provided via "fck" DT handle. This avoids
the need to lookup the main clock via hwmod core, which will not work
with the clkctrl clock support anymore; the main clock is not going to
be a mux.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/timer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 07dd692..af90f95 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -255,6 +255,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 		timer->io_base = of_iomap(np, 0);
 
+		timer->fclk = of_clk_get_by_name(np, "fck");
+
 		of_node_put(np);
 	} else {
 		if (omap_dm_timer_reserve_systimer(timer->id))
@@ -292,7 +294,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	omap_hwmod_setup_one(oh_name);
 
 	/* After the dmtimer is using hwmod these clocks won't be needed */
-	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
+	if (IS_ERR_OR_NULL(timer->fclk))
+		timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
 	if (IS_ERR(timer->fclk))
 		return PTR_ERR(timer->fclk);
 
-- 
1.9.1

[PATCHv3b 1/5] ARM: OMAP2+: timer: add support for fetching fck handle from DT

[Tero Kristo]

The mux clock handle shall be provided via "fck" DT handle. This avoids
the need to lookup the main clock via hwmod core, which will not work
with the clkctrl clock support anymore; the main clock is not going to
be a mux.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/timer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 07dd692..af90f95 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -255,6 +255,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 
 		timer->io_base = of_iomap(np, 0);
 
+		timer->fclk = of_clk_get_by_name(np, "fck");
+
 		of_node_put(np);
 	} else {
 		if (omap_dm_timer_reserve_systimer(timer->id))
@@ -292,7 +294,8 @@ static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
 	omap_hwmod_setup_one(oh_name);
 
 	/* After the dmtimer is using hwmod these clocks won't be needed */
-	timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
+	if (IS_ERR_OR_NULL(timer->fclk))
+		timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh));
 	if (IS_ERR(timer->fclk))
 		return PTR_ERR(timer->fclk);
 
-- 
1.9.1

[PATCHv3b 2/5] ARM: OMAP4: hwmod_data: add opt clks for dss_hdmi and dss_venc

[Tero Kristo]

These extra optional clocks are required as main clock for these modules
are going to be routed to the main module clock. Otherwise, the hdmi / tv
clocks are not going to be enabled during usage, leading to failure.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 94f09c7..0855434 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -775,6 +775,7 @@
 
 static struct omap_hwmod_opt_clk dss_hdmi_opt_clks[] = {
 	{ .role = "sys_clk", .clk = "dss_sys_clk" },
+	{ .role = "hdmi_clk", .clk = "dss_48mhz_clk" },
 };
 
 static struct omap_hwmod omap44xx_dss_hdmi_hwmod = {
@@ -785,7 +786,7 @@
 	 * HDMI audio requires to use no-idle mode. Hence,
 	 * set idle mode by software.
 	 */
-	.flags		= HWMOD_SWSUP_SIDLE,
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_OPT_CLKS_NEEDED,
 	.mpu_irqs	= omap44xx_dss_hdmi_irqs,
 	.xlate_irq	= omap4_xlate_irq,
 	.sdma_reqs	= omap44xx_dss_hdmi_sdma_reqs,
@@ -858,11 +859,16 @@
 };
 
 /* dss_venc */
+static struct omap_hwmod_opt_clk dss_venc_opt_clks[] = {
+	{ .role = "tv_clk", .clk = "dss_tv_clk" },
+};
+
 static struct omap_hwmod omap44xx_dss_venc_hwmod = {
 	.name		= "dss_venc",
 	.class		= &omap44xx_venc_hwmod_class,
 	.clkdm_name	= "l3_dss_clkdm",
 	.main_clk	= "dss_tv_clk",
+	.flags		= HWMOD_OPT_CLKS_NEEDED,
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_DSS_DSS_CLKCTRL_OFFSET,
@@ -870,6 +876,8 @@
 		},
 	},
 	.parent_hwmod	= &omap44xx_dss_hwmod,
+	.opt_clks	= dss_venc_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(dss_venc_opt_clks),
 };
 
 /*
-- 
1.9.1

[PATCHv3b 2/5] ARM: OMAP4: hwmod_data: add opt clks for dss_hdmi and dss_venc

[Tero Kristo]

These extra optional clocks are required as main clock for these modules
are going to be routed to the main module clock. Otherwise, the hdmi / tv
clocks are not going to be enabled during usage, leading to failure.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 94f09c7..0855434 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -775,6 +775,7 @@
 
 static struct omap_hwmod_opt_clk dss_hdmi_opt_clks[] = {
 	{ .role = "sys_clk", .clk = "dss_sys_clk" },
+	{ .role = "hdmi_clk", .clk = "dss_48mhz_clk" },
 };
 
 static struct omap_hwmod omap44xx_dss_hdmi_hwmod = {
@@ -785,7 +786,7 @@
 	 * HDMI audio requires to use no-idle mode. Hence,
 	 * set idle mode by software.
 	 */
-	.flags		= HWMOD_SWSUP_SIDLE,
+	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_OPT_CLKS_NEEDED,
 	.mpu_irqs	= omap44xx_dss_hdmi_irqs,
 	.xlate_irq	= omap4_xlate_irq,
 	.sdma_reqs	= omap44xx_dss_hdmi_sdma_reqs,
@@ -858,11 +859,16 @@
 };
 
 /* dss_venc */
+static struct omap_hwmod_opt_clk dss_venc_opt_clks[] = {
+	{ .role = "tv_clk", .clk = "dss_tv_clk" },
+};
+
 static struct omap_hwmod omap44xx_dss_venc_hwmod = {
 	.name		= "dss_venc",
 	.class		= &omap44xx_venc_hwmod_class,
 	.clkdm_name	= "l3_dss_clkdm",
 	.main_clk	= "dss_tv_clk",
+	.flags		= HWMOD_OPT_CLKS_NEEDED,
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_DSS_DSS_CLKCTRL_OFFSET,
@@ -870,6 +876,8 @@
 		},
 	},
 	.parent_hwmod	= &omap44xx_dss_hwmod,
+	.opt_clks	= dss_venc_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(dss_venc_opt_clks),
 };
 
 /*
-- 
1.9.1

[PATCHv3b 3/5] ARM: omap2+: clockdomain: add clkdm_xlate_address

[Tero Kristo]

This new function can be used to get the physical address of a
clockdomain. Required for mapping the clkctrl clocks under hwmod
without modification to DT data.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c | 8 ++++++++
 arch/arm/mach-omap2/clockdomain.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index b79b1ca..5189264 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -1224,6 +1224,14 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 	return 0;
 }
 
+u32 clkdm_xlate_address(struct clockdomain *clkdm)
+{
+	if (arch_clkdm->clkdm_xlate_address)
+		return arch_clkdm->clkdm_xlate_address(clkdm);
+
+	return 0;
+}
+
 /**
  * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm
  * @clkdm: struct clockdomain *
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 24667a5..827f01e 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -175,6 +175,7 @@ struct clkdm_ops {
 	void	(*clkdm_deny_idle)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_enable)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_disable)(struct clockdomain *clkdm);
+	u32	(*clkdm_xlate_address)(struct clockdomain *clkdm);
 };
 
 int clkdm_register_platform_funcs(struct clkdm_ops *co);
@@ -213,6 +214,7 @@ int clkdm_for_each(int (*fn)(struct clockdomain *clkdm, void *user),
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh);
 int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh);
+u32 clkdm_xlate_address(struct clockdomain *clkdm);
 
 extern void __init omap242x_clockdomains_init(void);
 extern void __init omap243x_clockdomains_init(void);
-- 
1.9.1

[PATCHv3b 3/5] ARM: omap2+: clockdomain: add clkdm_xlate_address

[Tero Kristo]

This new function can be used to get the physical address of a
clockdomain. Required for mapping the clkctrl clocks under hwmod
without modification to DT data.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/clockdomain.c | 8 ++++++++
 arch/arm/mach-omap2/clockdomain.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index b79b1ca..5189264 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -1224,6 +1224,14 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
 	return 0;
 }
 
+u32 clkdm_xlate_address(struct clockdomain *clkdm)
+{
+	if (arch_clkdm->clkdm_xlate_address)
+		return arch_clkdm->clkdm_xlate_address(clkdm);
+
+	return 0;
+}
+
 /**
  * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm
  * @clkdm: struct clockdomain *
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 24667a5..827f01e 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -175,6 +175,7 @@ struct clkdm_ops {
 	void	(*clkdm_deny_idle)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_enable)(struct clockdomain *clkdm);
 	int	(*clkdm_clk_disable)(struct clockdomain *clkdm);
+	u32	(*clkdm_xlate_address)(struct clockdomain *clkdm);
 };
 
 int clkdm_register_platform_funcs(struct clkdm_ops *co);
@@ -213,6 +214,7 @@ int clkdm_for_each(int (*fn)(struct clockdomain *clkdm, void *user),
 int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk);
 int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh);
 int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh);
+u32 clkdm_xlate_address(struct clockdomain *clkdm);
 
 extern void __init omap242x_clockdomains_init(void);
 extern void __init omap243x_clockdomains_init(void);
-- 
1.9.1

[PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tero Kristo]

This function gets the physical base address of a clockdomain.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/cminst44xx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 2ab27ad..3df268e 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -31,6 +31,7 @@
 #include "prm44xx.h"
 #include "prcm_mpu44xx.h"
 #include "prcm-common.h"
+#include "iomap.h"
 
 #define OMAP4430_IDLEST_SHIFT		16
 #define OMAP4430_IDLEST_MASK		(0x3 << 16)
@@ -475,6 +476,14 @@ static int omap4_clkdm_clk_disable(struct clockdomain *clkdm)
 	return 0;
 }
 
+static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
+{
+	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
+		clkdm->cm_inst + clkdm->clkdm_offs;
+
+	return addr;
+}
+
 struct clkdm_ops omap4_clkdm_operations = {
 	.clkdm_add_wkdep	= omap4_clkdm_add_wkup_sleep_dep,
 	.clkdm_del_wkdep	= omap4_clkdm_del_wkup_sleep_dep,
@@ -490,6 +499,7 @@ struct clkdm_ops omap4_clkdm_operations = {
 	.clkdm_deny_idle	= omap4_clkdm_deny_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
+	.clkdm_xlate_address	= omap4_clkdm_xlate_address,
 };
 
 struct clkdm_ops am43xx_clkdm_operations = {
@@ -499,6 +509,7 @@ struct clkdm_ops am43xx_clkdm_operations = {
 	.clkdm_deny_idle	= omap4_clkdm_deny_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
+	.clkdm_xlate_address	= omap4_clkdm_xlate_address,
 };
 
 static struct cm_ll_data omap4xxx_cm_ll_data = {
-- 
1.9.1

[PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tero Kristo]

This function gets the physical base address of a clockdomain.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/cminst44xx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 2ab27ad..3df268e 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -31,6 +31,7 @@
 #include "prm44xx.h"
 #include "prcm_mpu44xx.h"
 #include "prcm-common.h"
+#include "iomap.h"
 
 #define OMAP4430_IDLEST_SHIFT		16
 #define OMAP4430_IDLEST_MASK		(0x3 << 16)
@@ -475,6 +476,14 @@ static int omap4_clkdm_clk_disable(struct clockdomain *clkdm)
 	return 0;
 }
 
+static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
+{
+	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
+		clkdm->cm_inst + clkdm->clkdm_offs;
+
+	return addr;
+}
+
 struct clkdm_ops omap4_clkdm_operations = {
 	.clkdm_add_wkdep	= omap4_clkdm_add_wkup_sleep_dep,
 	.clkdm_del_wkdep	= omap4_clkdm_del_wkup_sleep_dep,
@@ -490,6 +499,7 @@ struct clkdm_ops omap4_clkdm_operations = {
 	.clkdm_deny_idle	= omap4_clkdm_deny_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
+	.clkdm_xlate_address	= omap4_clkdm_xlate_address,
 };
 
 struct clkdm_ops am43xx_clkdm_operations = {
@@ -499,6 +509,7 @@ struct clkdm_ops am43xx_clkdm_operations = {
 	.clkdm_deny_idle	= omap4_clkdm_deny_idle,
 	.clkdm_clk_enable	= omap4_clkdm_clk_enable,
 	.clkdm_clk_disable	= omap4_clkdm_clk_disable,
+	.clkdm_xlate_address	= omap4_clkdm_xlate_address,
 };
 
 static struct cm_ll_data omap4xxx_cm_ll_data = {
-- 
1.9.1

Re: [PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tony Lindgren]

Hi,

* Tero Kristo <t-kristo@ti.com> [170530 07:51]:
> This function gets the physical base address of a clockdomain.
..
> +static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
> +{
> +	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
> +		clkdm->cm_inst + clkdm->clkdm_offs;
> +
> +	return addr;
> +}
> +

Hmm this will be flakey as it assumes fixed mappings
for all these which we do not have on all SoCs.

It would be best to pass the domain bases around as
struct resource then just ioremap locally.

It might be enough if we changed _cm_bases[] locally
to be an array of:

struct omap_domain_base {
       struct resource res;
       void __iomem *va;
};

Or do you have better ideas?

Regards,

Tony

[PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tony Lindgren]

Hi,

* Tero Kristo <t-kristo@ti.com> [170530 07:51]:
> This function gets the physical base address of a clockdomain.
..
> +static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
> +{
> +	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
> +		clkdm->cm_inst + clkdm->clkdm_offs;
> +
> +	return addr;
> +}
> +

Hmm this will be flakey as it assumes fixed mappings
for all these which we do not have on all SoCs.

It would be best to pass the domain bases around as
struct resource then just ioremap locally.

It might be enough if we changed _cm_bases[] locally
to be an array of:

struct omap_domain_base {
       struct resource res;
       void __iomem *va;
};

Or do you have better ideas?

Regards,

Tony

Re: [PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tero Kristo]

On 30/05/17 21:10, Tony Lindgren wrote:
> Hi,
> 
> * Tero Kristo <t-kristo@ti.com> [170530 07:51]:
>> This function gets the physical base address of a clockdomain.
> ..
>> +static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
>> +{
>> +	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
>> +		clkdm->cm_inst + clkdm->clkdm_offs;
>> +
>> +	return addr;
>> +}
>> +
> 
> Hmm this will be flakey as it assumes fixed mappings
> for all these which we do not have on all SoCs.
> 
> It would be best to pass the domain bases around as
> struct resource then just ioremap locally.
> 
> It might be enough if we changed _cm_bases[] locally
> to be an array of:
> 
> struct omap_domain_base {
>         struct resource res;
>         void __iomem *va;
> };
> 
> Or do you have better ideas?
> 
> Regards,
> 
> Tony
> 

Ok, I have a patch available that saves the physical addresses during 
probe and uses this info during the xlate phase. Will post in a bit.

-Tero

[PATCHv3b 4/5] ARM: OMAP4: cminst: add support for clkdm_xlate_address

[Tero Kristo]

On 30/05/17 21:10, Tony Lindgren wrote:
> Hi,
> 
> * Tero Kristo <t-kristo@ti.com> [170530 07:51]:
>> This function gets the physical base address of a clockdomain.
> ..
>> +static u32 omap4_clkdm_xlate_address(struct clockdomain *clkdm)
>> +{
>> +	u32 addr = (u32)_cm_bases[clkdm->prcm_partition] - OMAP2_L4_IO_OFFSET +
>> +		clkdm->cm_inst + clkdm->clkdm_offs;
>> +
>> +	return addr;
>> +}
>> +
> 
> Hmm this will be flakey as it assumes fixed mappings
> for all these which we do not have on all SoCs.
> 
> It would be best to pass the domain bases around as
> struct resource then just ioremap locally.
> 
> It might be enough if we changed _cm_bases[] locally
> to be an array of:
> 
> struct omap_domain_base {
>         struct resource res;
>         void __iomem *va;
> };
> 
> Or do you have better ideas?
> 
> Regards,
> 
> Tony
> 

Ok, I have a patch available that saves the physical addresses during 
probe and uses this info during the xlate phase. Will post in a bit.

-Tero

[PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tero Kristo]

If clkctrl clocks are available on a device, populate these automatically
to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
compatible nodes and maps these against existing clockdomain data. Once
done, individual hwmod init routines can search for a clkctrl clock
handle based on the clockdomain info and the created mapping.

This patch also drops the obsolete "_mod_ck" search as the implementation
required for this was not accepted usptream.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c | 161 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 8bcea0d..4d752d8 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -141,6 +141,7 @@
 #include <linux/cpu.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/bootmem.h>
 
 #include <asm/system_misc.h>
 
@@ -182,6 +183,24 @@
 #define MOD_CLK_MAX_NAME_LEN		32
 
 /**
+ * struct clkctrl_provider - clkctrl provider mapping data
+ * @addr: base address for the provider
+ * @offset: base offset for the provider
+ * @clkdm: base clockdomain for provider
+ * @node: device node associated with the provider
+ * @link: list link
+ */
+struct clkctrl_provider {
+	u32			addr;
+	u16			offset;
+	struct clockdomain	*clkdm;
+	struct device_node	*node;
+	struct list_head	link;
+};
+
+static LIST_HEAD(clkctrl_providers);
+
+/**
  * struct omap_hwmod_soc_ops - fn ptrs for some SoC-specific operations
  * @enable_module: function to enable a module (via MODULEMODE)
  * @disable_module: function to disable a module (via MODULEMODE)
@@ -204,6 +223,8 @@ struct omap_hwmod_soc_ops {
 	void (*update_context_lost)(struct omap_hwmod *oh);
 	int (*get_context_lost)(struct omap_hwmod *oh);
 	int (*disable_direct_prcm)(struct omap_hwmod *oh);
+	u32 (*xlate_clkctrl)(struct omap_hwmod *oh,
+			     struct clkctrl_provider *provider);
 };
 
 /* soc_ops: adapts the omap_hwmod code to the currently-booted SoC */
@@ -690,34 +711,131 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
 	return clkdm_del_sleepdep(clkdm, init_clkdm);
 }
 
+static const struct of_device_id ti_clkctrl_match_table[] __initconst = {
+	{ .compatible = "ti,clkctrl" },
+	{ }
+};
+
+static int _match_clkdm(struct clockdomain *clkdm, void *user)
+{
+	struct clkctrl_provider *provider = user;
+
+	if (clkdm_xlate_address(clkdm) == provider->addr) {
+		pr_debug("%s: Matched clkdm %s for addr %x (%s)\n", __func__,
+			 clkdm->name, provider->addr,
+			 provider->node->parent->name);
+		provider->clkdm = clkdm;
+
+		return -1;
+	}
+
+	return 0;
+}
+
+static int _setup_clkctrl_provider(struct device_node *np)
+{
+	const __be32 *addrp;
+	struct clkctrl_provider *provider;
+
+	provider = memblock_virt_alloc(sizeof(*provider), 0);
+	if (!provider)
+		return -ENOMEM;
+
+	addrp = of_get_address(np, 0, NULL, NULL);
+	provider->addr = (u32)of_translate_address(np, addrp);
+	provider->offset = provider->addr & 0xff;
+	provider->addr &= ~0xff;
+	provider->node = np;
+
+	clkdm_for_each(_match_clkdm, provider);
+
+	if (!provider->clkdm) {
+		pr_err("%s: nothing matched for node %s (%x)\n",
+		       __func__, np->parent->name, provider->addr);
+		memblock_free_early(__pa(provider), sizeof(*provider));
+		return -EINVAL;
+	}
+
+	list_add(&provider->link, &clkctrl_providers);
+
+	return 0;
+}
+
+static int _init_clkctrl_providers(void)
+{
+	struct device_node *np;
+	int ret = 0;
+
+	for_each_matching_node(np, ti_clkctrl_match_table) {
+		ret = _setup_clkctrl_provider(np);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static u32 _omap4_xlate_clkctrl(struct omap_hwmod *oh,
+				struct clkctrl_provider *provider)
+{
+	return oh->prcm.omap4.clkctrl_offs -
+	       provider->offset - provider->clkdm->clkdm_offs;
+}
+
+static struct clk *_lookup_clkctrl_clk(struct omap_hwmod *oh,
+				       struct device_node *np)
+{
+	struct clkctrl_provider *provider;
+	struct clk *clk;
+
+	if (!soc_ops.xlate_clkctrl)
+		return NULL;
+
+	list_for_each_entry(provider, &clkctrl_providers, link) {
+		if (provider->clkdm == oh->clkdm) {
+			struct of_phandle_args clkspec;
+
+			clkspec.np = provider->node;
+			clkspec.args_count = 2;
+			clkspec.args[0] = soc_ops.xlate_clkctrl(oh, provider);
+			clkspec.args[1] = 0;
+
+			clk = of_clk_get_from_provider(&clkspec);
+
+			return clk;
+		}
+	}
+
+	return NULL;
+}
+
 /**
  * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
  * @oh: struct omap_hwmod *
+ * @np: device node mapped to this hwmod
  *
  * Called from _init_clocks().  Populates the @oh _clk (main
  * functional clock pointer) if a clock matching the hwmod name is found,
  * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
  */
-static int _init_main_clk(struct omap_hwmod *oh)
+static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
 {
 	int ret = 0;
-	char name[MOD_CLK_MAX_NAME_LEN];
-	struct clk *clk;
-	static const char modck[] = "_mod_ck";
+	struct clk *clk = NULL;
 
-	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
-		pr_warn("%s: warning: cropping name for %s\n", __func__,
-			oh->name);
-
-	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
-	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
+	if (np) {
+		clk = _lookup_clkctrl_clk(oh, np);
+
+		if (!IS_ERR_OR_NULL(clk)) {
+			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
+				 __clk_get_name(clk), oh->name);
+			oh->main_clk = __clk_get_name(clk);
+			oh->_clk = clk;
+			soc_ops.disable_direct_prcm(oh);
+		}
+	}
 
-	clk = clk_get(NULL, name);
-	if (!IS_ERR(clk)) {
-		oh->_clk = clk;
-		soc_ops.disable_direct_prcm(oh);
-		oh->main_clk = kstrdup(name, GFP_KERNEL);
-	} else {
+	if (IS_ERR_OR_NULL(clk)) {
 		if (!oh->main_clk)
 			return 0;
 
@@ -1482,13 +1600,13 @@ static int _init_clkdm(struct omap_hwmod *oh)
  * _init_clocks - clk_get() all clocks associated with this hwmod. Retrieve as
  * well the clockdomain.
  * @oh: struct omap_hwmod *
- * @data: not used; pass NULL
+ * @np: device_node mapped to this hwmod
  *
  * Called by omap_hwmod_setup_*() (after omap2_clk_init()).
  * Resolves all clock names embedded in the hwmod.  Returns 0 on
  * success, or a negative error code on failure.
  */
-static int _init_clocks(struct omap_hwmod *oh, void *data)
+static int _init_clocks(struct omap_hwmod *oh, struct device_node *np)
 {
 	int ret = 0;
 
@@ -1500,7 +1618,7 @@ static int _init_clocks(struct omap_hwmod *oh, void *data)
 	if (soc_ops.init_clkdm)
 		ret |= soc_ops.init_clkdm(oh);
 
-	ret |= _init_main_clk(oh);
+	ret |= _init_main_clk(oh, np);
 	ret |= _init_interface_clks(oh);
 	ret |= _init_opt_clks(oh);
 
@@ -2360,7 +2478,7 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 		return 0;
 	}
 
-	r = _init_clocks(oh, NULL);
+	r = _init_clocks(oh, np);
 	if (r < 0) {
 		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
 		return -EINVAL;
@@ -3722,6 +3840,7 @@ void __init omap_hwmod_init(void)
 		soc_ops.update_context_lost = _omap4_update_context_lost;
 		soc_ops.get_context_lost = _omap4_get_context_lost;
 		soc_ops.disable_direct_prcm = _omap4_disable_direct_prcm;
+		soc_ops.xlate_clkctrl = _omap4_xlate_clkctrl;
 	} else if (cpu_is_ti814x() || cpu_is_ti816x() || soc_is_am33xx() ||
 		   soc_is_am43xx()) {
 		soc_ops.enable_module = _omap4_enable_module;
@@ -3736,6 +3855,8 @@ void __init omap_hwmod_init(void)
 		WARN(1, "omap_hwmod: unknown SoC type\n");
 	}
 
+	_init_clkctrl_providers();
+
 	inited = true;
 }
 
-- 
1.9.1

[PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tero Kristo]

If clkctrl clocks are available on a device, populate these automatically
to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
compatible nodes and maps these against existing clockdomain data. Once
done, individual hwmod init routines can search for a clkctrl clock
handle based on the clockdomain info and the created mapping.

This patch also drops the obsolete "_mod_ck" search as the implementation
required for this was not accepted usptream.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c | 161 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 8bcea0d..4d752d8 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -141,6 +141,7 @@
 #include <linux/cpu.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/bootmem.h>
 
 #include <asm/system_misc.h>
 
@@ -182,6 +183,24 @@
 #define MOD_CLK_MAX_NAME_LEN		32
 
 /**
+ * struct clkctrl_provider - clkctrl provider mapping data
+ * @addr: base address for the provider
+ * @offset: base offset for the provider
+ * @clkdm: base clockdomain for provider
+ * @node: device node associated with the provider
+ * @link: list link
+ */
+struct clkctrl_provider {
+	u32			addr;
+	u16			offset;
+	struct clockdomain	*clkdm;
+	struct device_node	*node;
+	struct list_head	link;
+};
+
+static LIST_HEAD(clkctrl_providers);
+
+/**
  * struct omap_hwmod_soc_ops - fn ptrs for some SoC-specific operations
  * @enable_module: function to enable a module (via MODULEMODE)
  * @disable_module: function to disable a module (via MODULEMODE)
@@ -204,6 +223,8 @@ struct omap_hwmod_soc_ops {
 	void (*update_context_lost)(struct omap_hwmod *oh);
 	int (*get_context_lost)(struct omap_hwmod *oh);
 	int (*disable_direct_prcm)(struct omap_hwmod *oh);
+	u32 (*xlate_clkctrl)(struct omap_hwmod *oh,
+			     struct clkctrl_provider *provider);
 };
 
 /* soc_ops: adapts the omap_hwmod code to the currently-booted SoC */
@@ -690,34 +711,131 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
 	return clkdm_del_sleepdep(clkdm, init_clkdm);
 }
 
+static const struct of_device_id ti_clkctrl_match_table[] __initconst = {
+	{ .compatible = "ti,clkctrl" },
+	{ }
+};
+
+static int _match_clkdm(struct clockdomain *clkdm, void *user)
+{
+	struct clkctrl_provider *provider = user;
+
+	if (clkdm_xlate_address(clkdm) == provider->addr) {
+		pr_debug("%s: Matched clkdm %s for addr %x (%s)\n", __func__,
+			 clkdm->name, provider->addr,
+			 provider->node->parent->name);
+		provider->clkdm = clkdm;
+
+		return -1;
+	}
+
+	return 0;
+}
+
+static int _setup_clkctrl_provider(struct device_node *np)
+{
+	const __be32 *addrp;
+	struct clkctrl_provider *provider;
+
+	provider = memblock_virt_alloc(sizeof(*provider), 0);
+	if (!provider)
+		return -ENOMEM;
+
+	addrp = of_get_address(np, 0, NULL, NULL);
+	provider->addr = (u32)of_translate_address(np, addrp);
+	provider->offset = provider->addr & 0xff;
+	provider->addr &= ~0xff;
+	provider->node = np;
+
+	clkdm_for_each(_match_clkdm, provider);
+
+	if (!provider->clkdm) {
+		pr_err("%s: nothing matched for node %s (%x)\n",
+		       __func__, np->parent->name, provider->addr);
+		memblock_free_early(__pa(provider), sizeof(*provider));
+		return -EINVAL;
+	}
+
+	list_add(&provider->link, &clkctrl_providers);
+
+	return 0;
+}
+
+static int _init_clkctrl_providers(void)
+{
+	struct device_node *np;
+	int ret = 0;
+
+	for_each_matching_node(np, ti_clkctrl_match_table) {
+		ret = _setup_clkctrl_provider(np);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static u32 _omap4_xlate_clkctrl(struct omap_hwmod *oh,
+				struct clkctrl_provider *provider)
+{
+	return oh->prcm.omap4.clkctrl_offs -
+	       provider->offset - provider->clkdm->clkdm_offs;
+}
+
+static struct clk *_lookup_clkctrl_clk(struct omap_hwmod *oh,
+				       struct device_node *np)
+{
+	struct clkctrl_provider *provider;
+	struct clk *clk;
+
+	if (!soc_ops.xlate_clkctrl)
+		return NULL;
+
+	list_for_each_entry(provider, &clkctrl_providers, link) {
+		if (provider->clkdm == oh->clkdm) {
+			struct of_phandle_args clkspec;
+
+			clkspec.np = provider->node;
+			clkspec.args_count = 2;
+			clkspec.args[0] = soc_ops.xlate_clkctrl(oh, provider);
+			clkspec.args[1] = 0;
+
+			clk = of_clk_get_from_provider(&clkspec);
+
+			return clk;
+		}
+	}
+
+	return NULL;
+}
+
 /**
  * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
  * @oh: struct omap_hwmod *
+ * @np: device node mapped to this hwmod
  *
  * Called from _init_clocks().  Populates the @oh _clk (main
  * functional clock pointer) if a clock matching the hwmod name is found,
  * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
  */
-static int _init_main_clk(struct omap_hwmod *oh)
+static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
 {
 	int ret = 0;
-	char name[MOD_CLK_MAX_NAME_LEN];
-	struct clk *clk;
-	static const char modck[] = "_mod_ck";
+	struct clk *clk = NULL;
 
-	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
-		pr_warn("%s: warning: cropping name for %s\n", __func__,
-			oh->name);
-
-	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
-	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
+	if (np) {
+		clk = _lookup_clkctrl_clk(oh, np);
+
+		if (!IS_ERR_OR_NULL(clk)) {
+			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
+				 __clk_get_name(clk), oh->name);
+			oh->main_clk = __clk_get_name(clk);
+			oh->_clk = clk;
+			soc_ops.disable_direct_prcm(oh);
+		}
+	}
 
-	clk = clk_get(NULL, name);
-	if (!IS_ERR(clk)) {
-		oh->_clk = clk;
-		soc_ops.disable_direct_prcm(oh);
-		oh->main_clk = kstrdup(name, GFP_KERNEL);
-	} else {
+	if (IS_ERR_OR_NULL(clk)) {
 		if (!oh->main_clk)
 			return 0;
 
@@ -1482,13 +1600,13 @@ static int _init_clkdm(struct omap_hwmod *oh)
  * _init_clocks - clk_get() all clocks associated with this hwmod. Retrieve as
  * well the clockdomain.
  * @oh: struct omap_hwmod *
- * @data: not used; pass NULL
+ * @np: device_node mapped to this hwmod
  *
  * Called by omap_hwmod_setup_*() (after omap2_clk_init()).
  * Resolves all clock names embedded in the hwmod.  Returns 0 on
  * success, or a negative error code on failure.
  */
-static int _init_clocks(struct omap_hwmod *oh, void *data)
+static int _init_clocks(struct omap_hwmod *oh, struct device_node *np)
 {
 	int ret = 0;
 
@@ -1500,7 +1618,7 @@ static int _init_clocks(struct omap_hwmod *oh, void *data)
 	if (soc_ops.init_clkdm)
 		ret |= soc_ops.init_clkdm(oh);
 
-	ret |= _init_main_clk(oh);
+	ret |= _init_main_clk(oh, np);
 	ret |= _init_interface_clks(oh);
 	ret |= _init_opt_clks(oh);
 
@@ -2360,7 +2478,7 @@ static int __init _init(struct omap_hwmod *oh, void *data)
 		return 0;
 	}
 
-	r = _init_clocks(oh, NULL);
+	r = _init_clocks(oh, np);
 	if (r < 0) {
 		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
 		return -EINVAL;
@@ -3722,6 +3840,7 @@ void __init omap_hwmod_init(void)
 		soc_ops.update_context_lost = _omap4_update_context_lost;
 		soc_ops.get_context_lost = _omap4_get_context_lost;
 		soc_ops.disable_direct_prcm = _omap4_disable_direct_prcm;
+		soc_ops.xlate_clkctrl = _omap4_xlate_clkctrl;
 	} else if (cpu_is_ti814x() || cpu_is_ti816x() || soc_is_am33xx() ||
 		   soc_is_am43xx()) {
 		soc_ops.enable_module = _omap4_enable_module;
@@ -3736,6 +3855,8 @@ void __init omap_hwmod_init(void)
 		WARN(1, "omap_hwmod: unknown SoC type\n");
 	}
 
+	_init_clkctrl_providers();
+
 	inited = true;
 }
 
-- 
1.9.1

Re: [PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [170530 07:52]:
> If clkctrl clocks are available on a device, populate these automatically
> to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
> compatible nodes and maps these against existing clockdomain data. Once
> done, individual hwmod init routines can search for a clkctrl clock
> handle based on the clockdomain info and the created mapping.
...
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>  /**
>   * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
>   * @oh: struct omap_hwmod *
> + * @np: device node mapped to this hwmod
>   *
>   * Called from _init_clocks().  Populates the @oh _clk (main
>   * functional clock pointer) if a clock matching the hwmod name is found,
>   * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
>   */
> -static int _init_main_clk(struct omap_hwmod *oh)
> +static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
>  {
>  	int ret = 0;
> -	char name[MOD_CLK_MAX_NAME_LEN];
> -	struct clk *clk;
> -	static const char modck[] = "_mod_ck";
> +	struct clk *clk = NULL;
>  
> -	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
> -		pr_warn("%s: warning: cropping name for %s\n", __func__,
> -			oh->name);
> -
> -	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
> -	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
> +	if (np) {
> +		clk = _lookup_clkctrl_clk(oh, np);
> +
> +		if (!IS_ERR_OR_NULL(clk)) {
> +			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
> +				 __clk_get_name(clk), oh->name);
> +			oh->main_clk = __clk_get_name(clk);
> +			oh->_clk = clk;
> +			soc_ops.disable_direct_prcm(oh);
> +		}
> +	}

Shouldn't we just parse all "ti,clkctrl" matches to
a list, then match against that list?

Again I'm worried that the "struct device_node *np"
here won't really map to struct omap_hwmod but to one of
the child devices. Or what is the np here?

Regards,

Tony

[PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [170530 07:52]:
> If clkctrl clocks are available on a device, populate these automatically
> to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
> compatible nodes and maps these against existing clockdomain data. Once
> done, individual hwmod init routines can search for a clkctrl clock
> handle based on the clockdomain info and the created mapping.
...
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>  /**
>   * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
>   * @oh: struct omap_hwmod *
> + * @np: device node mapped to this hwmod
>   *
>   * Called from _init_clocks().  Populates the @oh _clk (main
>   * functional clock pointer) if a clock matching the hwmod name is found,
>   * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
>   */
> -static int _init_main_clk(struct omap_hwmod *oh)
> +static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
>  {
>  	int ret = 0;
> -	char name[MOD_CLK_MAX_NAME_LEN];
> -	struct clk *clk;
> -	static const char modck[] = "_mod_ck";
> +	struct clk *clk = NULL;
>  
> -	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
> -		pr_warn("%s: warning: cropping name for %s\n", __func__,
> -			oh->name);
> -
> -	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
> -	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
> +	if (np) {
> +		clk = _lookup_clkctrl_clk(oh, np);
> +
> +		if (!IS_ERR_OR_NULL(clk)) {
> +			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
> +				 __clk_get_name(clk), oh->name);
> +			oh->main_clk = __clk_get_name(clk);
> +			oh->_clk = clk;
> +			soc_ops.disable_direct_prcm(oh);
> +		}
> +	}

Shouldn't we just parse all "ti,clkctrl" matches to
a list, then match against that list?

Again I'm worried that the "struct device_node *np"
here won't really map to struct omap_hwmod but to one of
the child devices. Or what is the np here?

Regards,

Tony

Re: [PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tero Kristo]

On 30/05/17 21:15, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [170530 07:52]:
>> If clkctrl clocks are available on a device, populate these automatically
>> to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
>> compatible nodes and maps these against existing clockdomain data. Once
>> done, individual hwmod init routines can search for a clkctrl clock
>> handle based on the clockdomain info and the created mapping.
> ...
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>   /**
>>    * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
>>    * @oh: struct omap_hwmod *
>> + * @np: device node mapped to this hwmod
>>    *
>>    * Called from _init_clocks().  Populates the @oh _clk (main
>>    * functional clock pointer) if a clock matching the hwmod name is found,
>>    * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
>>    */
>> -static int _init_main_clk(struct omap_hwmod *oh)
>> +static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
>>   {
>>   	int ret = 0;
>> -	char name[MOD_CLK_MAX_NAME_LEN];
>> -	struct clk *clk;
>> -	static const char modck[] = "_mod_ck";
>> +	struct clk *clk = NULL;
>>   
>> -	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
>> -		pr_warn("%s: warning: cropping name for %s\n", __func__,
>> -			oh->name);
>> -
>> -	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
>> -	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
>> +	if (np) {
>> +		clk = _lookup_clkctrl_clk(oh, np);
This func call above maps the hwmod to a clkctrl clock. See below.

>> +
>> +		if (!IS_ERR_OR_NULL(clk)) {
>> +			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
>> +				 __clk_get_name(clk), oh->name);
>> +			oh->main_clk = __clk_get_name(clk);
>> +			oh->_clk = clk;
>> +			soc_ops.disable_direct_prcm(oh);
>> +		}
>> +	}
> 
> Shouldn't we just parse all "ti,clkctrl" matches to
> a list, then match against that list?

Yeah, this is actually what we are doing. Check out what 
_init_clkctrl_providers() is doing. _lookup_clkctrl_clk() is then used 
to match each individual hwmod to its main clkctrl clk.

> 
> Again I'm worried that the "struct device_node *np"
> here won't really map to struct omap_hwmod but to one of
> the child devices. Or what is the np here?

The np is actually not needed for anything anymore here, so I will drop 
it from v4.

-Tero

[PATCHv3b 5/5] ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available

[Tero Kristo]

On 30/05/17 21:15, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [170530 07:52]:
>> If clkctrl clocks are available on a device, populate these automatically
>> to replace hwmod main_clk info. First, the patch parses all "ti,clkctrl"
>> compatible nodes and maps these against existing clockdomain data. Once
>> done, individual hwmod init routines can search for a clkctrl clock
>> handle based on the clockdomain info and the created mapping.
> ...
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>   /**
>>    * _init_main_clk - get a struct clk * for the the hwmod's main functional clk
>>    * @oh: struct omap_hwmod *
>> + * @np: device node mapped to this hwmod
>>    *
>>    * Called from _init_clocks().  Populates the @oh _clk (main
>>    * functional clock pointer) if a clock matching the hwmod name is found,
>>    * or a main_clk is present.  Returns 0 on success or -EINVAL on error.
>>    */
>> -static int _init_main_clk(struct omap_hwmod *oh)
>> +static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
>>   {
>>   	int ret = 0;
>> -	char name[MOD_CLK_MAX_NAME_LEN];
>> -	struct clk *clk;
>> -	static const char modck[] = "_mod_ck";
>> +	struct clk *clk = NULL;
>>   
>> -	if (strlen(oh->name) >= MOD_CLK_MAX_NAME_LEN - strlen(modck))
>> -		pr_warn("%s: warning: cropping name for %s\n", __func__,
>> -			oh->name);
>> -
>> -	strlcpy(name, oh->name, MOD_CLK_MAX_NAME_LEN - strlen(modck));
>> -	strlcat(name, modck, MOD_CLK_MAX_NAME_LEN);
>> +	if (np) {
>> +		clk = _lookup_clkctrl_clk(oh, np);
This func call above maps the hwmod to a clkctrl clock. See below.

>> +
>> +		if (!IS_ERR_OR_NULL(clk)) {
>> +			pr_debug("%s: mapped main_clk %s for %s\n", __func__,
>> +				 __clk_get_name(clk), oh->name);
>> +			oh->main_clk = __clk_get_name(clk);
>> +			oh->_clk = clk;
>> +			soc_ops.disable_direct_prcm(oh);
>> +		}
>> +	}
> 
> Shouldn't we just parse all "ti,clkctrl" matches to
> a list, then match against that list?

Yeah, this is actually what we are doing. Check out what 
_init_clkctrl_providers() is doing. _lookup_clkctrl_clk() is then used 
to match each individual hwmod to its main clkctrl clk.

> 
> Again I'm worried that the "struct device_node *np"
> here won't really map to struct omap_hwmod but to one of
> the child devices. Or what is the np here?

The np is actually not needed for anything anymore here, so I will drop 
it from v4.

-Tero

Re: [PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [170530 07:51]:
> Hi,
> 
> This is an alternative solution against v3 [1] of this series.
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg581965.html
> 
> This series is different in that it parses the hwmod data in the fly
> and swaps the main_clk info with clkctrl implementations if those
> are present. Patch #1 / #2 are old from v3 of the series, rest are
> new. Patches #3 / #4 add some infrastructure support for parsing
> the clock data, and patch #5 does the clock swapping.
> 
> With this series, it is not necessary to populate clkctrl clock
> entries to DT at all, thus the transition should be smoother.

Yeah OK I think this is the way to go to avoid changing
device tree multiple times. The only issues I have are
for patches 4 and 5 that I already commented on there.

Regards,

Tony

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tony Lindgren]

* Tero Kristo <t-kristo@ti.com> [170530 07:51]:
> Hi,
> 
> This is an alternative solution against v3 [1] of this series.
> 
> [1]?https://www.spinics.net/lists/arm-kernel/msg581965.html
> 
> This series is different in that it parses the hwmod data in the fly
> and swaps the main_clk info with clkctrl implementations if those
> are present. Patch #1 / #2 are old from v3 of the series, rest are
> new. Patches #3 / #4 add some infrastructure support for parsing
> the clock data, and patch #5 does the clock swapping.
> 
> With this series, it is not necessary to populate clkctrl clock
> entries to DT at all, thus the transition should be smoother.

Yeah OK I think this is the way to go to avoid changing
device tree multiple times. The only issues I have are
for patches 4 and 5 that I already commented on there.

Regards,

Tony

Re: [PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tero Kristo]

On 30/05/17 21:16, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [170530 07:51]:
>> Hi,
>>
>> This is an alternative solution against v3 [1] of this series.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg581965.html
>>
>> This series is different in that it parses the hwmod data in the fly
>> and swaps the main_clk info with clkctrl implementations if those
>> are present. Patch #1 / #2 are old from v3 of the series, rest are
>> new. Patches #3 / #4 add some infrastructure support for parsing
>> the clock data, and patch #5 does the clock swapping.
>>
>> With this series, it is not necessary to populate clkctrl clock
>> entries to DT at all, thus the transition should be smoother.
> 
> Yeah OK I think this is the way to go to avoid changing
> device tree multiple times. The only issues I have are
> for patches 4 and 5 that I already commented on there.

Ok, I'll post v4 based on this series and fixup the issues with patch #4 
/ #5. I'll abandon the original v3 series.

-Tero

[PATCHv3b 0/5] ARM: OMAP2+: preparation for clk/clkctrl support

[Tero Kristo]

On 30/05/17 21:16, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [170530 07:51]:
>> Hi,
>>
>> This is an alternative solution against v3 [1] of this series.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg581965.html
>>
>> This series is different in that it parses the hwmod data in the fly
>> and swaps the main_clk info with clkctrl implementations if those
>> are present. Patch #1 / #2 are old from v3 of the series, rest are
>> new. Patches #3 / #4 add some infrastructure support for parsing
>> the clock data, and patch #5 does the clock swapping.
>>
>> With this series, it is not necessary to populate clkctrl clock
>> entries to DT at all, thus the transition should be smoother.
> 
> Yeah OK I think this is the way to go to avoid changing
> device tree multiple times. The only issues I have are
> for patches 4 and 5 that I already commented on there.

Ok, I'll post v4 based on this series and fixup the issues with patch #4 
/ #5. I'll abandon the original v3 series.

-Tero