public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
From: Rajendra Nayak <rnayak@ti.com>
To: Rajendra Nayak <rnayak@ti.com>,
	Sumit Semwal <sumit.semwal@ti.com>,
	tomi.valkeinen@nokia.com, paul@pwsan.com,
	Kevin Hilman <khilman@ti.com>, Vaibhav Hiremath <hvaibhav@ti.com>,
	linux-
Subject: RE: [PATCH v2 1/4] OMAP2PLUS: opt-clocks: align dss clock roles and names
Date: Thu, 27 Jan 2011 21:13:58 +0530	[thread overview]
Message-ID: <4f3e9441abaf0813b2a7c4d7e9e2613d@mail.gmail.com> (raw)
In-Reply-To: <251c5094ce38ddb6473cc35940e6bcec@mail.gmail.com>

> -----Original Message-----
> From: Rajendra Nayak [mailto:rnayak@ti.com]
> Sent: Thursday, January 27, 2011 11:24 AM
> To: Sumit Semwal; tomi.valkeinen@nokia.com; paul@pwsan.com; Kevin
Hilman; Vaibhav Hiremath; linux-
> omap@vger.kernel.org; Benoit Cousson
> Subject: RE: [PATCH v2 1/4] OMAP2PLUS: opt-clocks: align dss clock roles
and names
>
> Hi Sumit,
>
> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Sumit Semwal
> > Sent: Tuesday, January 25, 2011 6:30 PM
> > To: tomi.valkeinen@nokia.com; paul@pwsan.com; khilman@ti.com;
> hvaibhav@ti.com; linux-omap@vger.kernel.org; b-
> > cousson@ti.com
> > Cc: Sumit Semwal
> > Subject: [PATCH v2 1/4] OMAP2PLUS: opt-clocks: align dss clock roles
and
> names
> >
> > opt clocks require (NULL, act-clock-name) as entries in clock
database,
> > so that hwmod can replace it with (dev, role) tuple during hwmod data
> init.
> >
> > These role names are aligned to be same across OMAP2420, 2430, 3xxx,
> 44xx
> > platforms, and also with dss clk handling code, so
> clk_get/put/enable/disable
> > APIs in dss can use uniform role names.
> >
> > Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> > ---
> >  arch/arm/mach-omap2/clock2420_data.c       |   10 +++++++---
> >  arch/arm/mach-omap2/clock2430_data.c       |   10 +++++++---
> >  arch/arm/mach-omap2/clock3xxx_data.c       |   22
> +++++++++++++++-------
> >  arch/arm/mach-omap2/clock44xx_data.c       |    7 ++++++-
> >  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    2 +-
> >  drivers/video/omap2/dss/dss.c              |    8 ++++----
> >  6 files changed, 40 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/clock2420_data.c
> b/arch/arm/mach-omap2/clock2420_data.c
> > index d2abc2f..72a1872 100644
> > --- a/arch/arm/mach-omap2/clock2420_data.c
> > +++ b/arch/arm/mach-omap2/clock2420_data.c
> > @@ -1787,9 +1787,13 @@ static struct omap_clk omap2420_clks[] = {
> >  	CLK(NULL,	"gfx_ick",	&gfx_ick,	CK_242X),
> >  	/* DSS domain clocks */
> >  	CLK("omap_dss",	"ick",		&dss_ick,	CK_242X),
> > -	CLK("omap_dss",	"dss1_fck",	&dss1_fck,	CK_242X),
> > -	CLK("omap_dss",	"dss2_fck",	&dss2_fck,	CK_242X),
> > -	CLK("omap_dss",	"tv_fck",	&dss_54m_fck,	CK_242X),
> > +	CLK("omap_dss",	"fck",		&dss1_fck,	CK_242X),
> > +	/*
> > +	 * clocks handled via hwmod opt_clk mechanism need dev=NULL,
> > +	 * con=clock name as per actual clk structure, NOT role
> > +	 */
>
> I don't think that's true. The hmmod framework uses the
> omap_clk_get_by_name api which does not look up the
> clkdev table at all. It just looks up the clk->name, so the
> clkdev entries need not be changed with role=clk->name.
> Same applies to all other such instances in this patch.
>

An offline chat with Sumit showed that the problem exists
in the way clkdev aliases are added for optional clks in the
omap device layer.
Have a patch to fix this, will post soon after some more
testing.

Regards,
Rajendra

> Regards,
> Rajendra
>
> > +	CLK(NULL,	"dss2_fck",	&dss2_fck,	CK_242X),
> > +	CLK(NULL,	"dss_54m_fck",	&dss_54m_fck,	CK_242X),
> >  	/* L3 domain clocks */
> >  	CLK(NULL,	"core_l3_ck",	&core_l3_ck,	CK_242X),
> >  	CLK(NULL,	"ssi_fck",	&ssi_ssr_sst_fck, CK_242X),
> > diff --git a/arch/arm/mach-omap2/clock2430_data.c
> b/arch/arm/mach-omap2/clock2430_data.c
> > index 663f298..b99f881 100644
> > --- a/arch/arm/mach-omap2/clock2430_data.c
> > +++ b/arch/arm/mach-omap2/clock2430_data.c
> > @@ -1891,9 +1891,13 @@ static struct omap_clk omap2430_clks[] = {
> >  	CLK(NULL,	"mdm_osc_ck",	&mdm_osc_ck,	CK_243X),
> >  	/* DSS domain clocks */
> >  	CLK("omap_dss",	"ick",		&dss_ick,	CK_243X),
> > -	CLK("omap_dss",	"dss1_fck",	&dss1_fck,	CK_243X),
> > -	CLK("omap_dss",	"dss2_fck",	&dss2_fck,	CK_243X),
> > -	CLK("omap_dss",	"tv_fck",	&dss_54m_fck,	CK_243X),
> > +	CLK("omap_dss",	"fck",		&dss1_fck,	CK_243X),
> > +	/*
> > +	 * clocks handled via hwmod opt_clk mechanism need dev=NULL,
> > +	 * con=clock name as per actual clk structure, NOT role
> > +	 */
> > +	CLK(NULL,	"dss2_fck",	&dss2_fck,	CK_243X),
> > +	CLK(NULL,	"dss_54m_fck",	&dss_54m_fck,	CK_243X),
> >  	/* L3 domain clocks */
> >  	CLK(NULL,	"core_l3_ck",	&core_l3_ck,	CK_243X),
> >  	CLK(NULL,	"ssi_fck",	&ssi_ssr_sst_fck, CK_243X),
> > diff --git a/arch/arm/mach-omap2/clock3xxx_data.c
> b/arch/arm/mach-omap2/clock3xxx_data.c
> > index 5c97b93..c32df5d 100644
> > --- a/arch/arm/mach-omap2/clock3xxx_data.c
> > +++ b/arch/arm/mach-omap2/clock3xxx_data.c
> > @@ -3357,13 +3357,21 @@ static struct omap_clk omap3xxx_clks[] = {
> >  	CLK("omap_rng",	"ick",		&rng_ick,	CK_34XX |
> CK_36XX),
> >  	CLK(NULL,	"sha11_ick",	&sha11_ick,	CK_34XX |
> CK_36XX),
> >  	CLK(NULL,	"des1_ick",	&des1_ick,	CK_34XX |
> CK_36XX),
> > -	CLK("omap_dss",	"dss1_fck",	&dss1_alwon_fck_3430es1,
> CK_3430ES1),
> > -	CLK("omap_dss",	"dss1_fck",	&dss1_alwon_fck_3430es2,
> CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
> > -	CLK("omap_dss",	"tv_fck",	&dss_tv_fck,	CK_3XXX),
> > -	CLK("omap_dss",	"video_fck",	&dss_96m_fck,	CK_3XXX),
> > -	CLK("omap_dss",	"dss2_fck",	&dss2_alwon_fck, CK_3XXX),
> > -	CLK("omap_dss",	"ick",		&dss_ick_3430es1,
> CK_3430ES1),
> > -	CLK("omap_dss",	"ick",		&dss_ick_3430es2,
> CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
> > +	/* DSS clocks */
> > +	CLK("omap_dss",	"fck",		&dss1_alwon_fck_3430es1,
> CK_3430ES1),
> > +	CLK("omap_dss",	"fck",		&dss1_alwon_fck_3430es2,
> CK_3430ES2PLUS
> > +								|
> CK_AM35XX),
> > +	CLK("omap_dss",	"ick",		&dss_ick_3430es1, CK_3430ES1),
> > +	CLK("omap_dss",	"ick",		&dss_ick_3430es2, CK_3430ES2PLUS
> > +								|
> CK_AM35XX),
> > +	/*
> > +	 * clocks handled via hwmod opt_clk mechanism need dev=NULL,
> > +	 * con=clock name as per actual clk structure, NOT role
> > +	 */
> > +	CLK(NULL,       "dss_tv_fck",   &dss_tv_fck,    CK_3XXX),
> > +	CLK(NULL,       "dss_96m_fck",  &dss_96m_fck,   CK_3XXX),
> > +	CLK(NULL,       "dss2_alwon_fck",       &dss2_alwon_fck,
> CK_3XXX),
> > +
> >  	CLK(NULL,	"cam_mclk",	&cam_mclk,	CK_34XX |
> CK_36XX),
> >  	CLK(NULL,	"cam_ick",	&cam_ick,	CK_34XX |
> CK_36XX),
> >  	CLK(NULL,	"csi2_96m_fck",	&csi2_96m_fck,	CK_34XX |
> CK_36XX),
> > diff --git a/arch/arm/mach-omap2/clock44xx_data.c
> b/arch/arm/mach-omap2/clock44xx_data.c
> > index e8cb32f..74db324 100644
> > --- a/arch/arm/mach-omap2/clock44xx_data.c
> > +++ b/arch/arm/mach-omap2/clock44xx_data.c
> > @@ -3107,11 +3107,16 @@ static struct omap_clk omap44xx_clks[] = {
> >  	CLK(NULL,	"dmic_sync_mux_ck",		&dmic_sync_mux_ck,
> CK_443X),
> >  	CLK(NULL,	"dmic_fck",			&dmic_fck,
> CK_443X),
> >  	CLK(NULL,	"dsp_fck",			&dsp_fck,
> CK_443X),
> > +	/* dss clocks */
> > +	CLK(NULL,	"fck",				&dss_fck,
> CK_443X),
> > +	/*
> > +	 * clocks handled via hwmod opt_clk mechanism need dev=NULL,
> > +	 * con=clock name as per actual clk structure, NOT role
> > +	 */
> >  	CLK(NULL,	"dss_sys_clk",			&dss_sys_clk,
> CK_443X),
> >  	CLK(NULL,	"dss_tv_clk",			&dss_tv_clk,
> CK_443X),
> >  	CLK(NULL,	"dss_dss_clk",			&dss_dss_clk,
> CK_443X),
> >  	CLK(NULL,	"dss_48mhz_clk",		&dss_48mhz_clk,
> CK_443X),
> > -	CLK(NULL,	"dss_fck",			&dss_fck,
> CK_443X),
> >  	CLK(NULL,	"efuse_ctrl_cust_fck",
> &efuse_ctrl_cust_fck,	CK_443X),
> >  	CLK(NULL,	"emif1_fck",			&emif1_fck,
> CK_443X),
> >  	CLK(NULL,	"emif2_fck",			&emif2_fck,
> CK_443X),
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> > index 713165d..cb0c624 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> > @@ -770,7 +770,7 @@ static struct omap_hwmod_ocp_if
> *omap3xxx_dss_slaves[] = {
> >
> >  static struct omap_hwmod_opt_clk dss_opt_clks[] = {
> >  	{ .role = "tv_clk", .clk = "dss_tv_fck" },
> > -	{ .role = "dssclk", .clk = "dss_96m_fck" },
> > +	{ .role = "video_clk", .clk = "dss_96m_fck" },
> >  	{ .role = "sys_clk", .clk = "dss2_alwon_fck" },
> >  };
> >
> > diff --git a/drivers/video/omap2/dss/dss.c
> b/drivers/video/omap2/dss/dss.c
> > index f9390b4..91f8cf7 100644
> > --- a/drivers/video/omap2/dss/dss.c
> > +++ b/drivers/video/omap2/dss/dss.c
> > @@ -758,19 +758,19 @@ static int dss_get_clocks(void)
> >  	if (r)
> >  		goto err;
> >
> > -	r = dss_get_clock(&dss.dss1_fck, "dss1_fck");
> > +	r = dss_get_clock(&dss.dss1_fck, "fck");
> >  	if (r)
> >  		goto err;
> >
> > -	r = dss_get_clock(&dss.dss2_fck, "dss2_fck");
> > +	r = dss_get_clock(&dss.dss2_fck, "sys_clk");
> >  	if (r)
> >  		goto err;
> >
> > -	r = dss_get_clock(&dss.dss_54m_fck, "tv_fck");
> > +	r = dss_get_clock(&dss.dss_54m_fck, "tv_clk");
> >  	if (r)
> >  		goto err;
> >
> > -	r = dss_get_clock(&dss.dss_96m_fck, "video_fck");
> > +	r = dss_get_clock(&dss.dss_96m_fck, "video_clk");
> >  	if (r)
> >  		goto err;
> >
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap"
in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2011-01-27 15:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-25 12:59 [PATCH v2 0/4] OMAP2PLUS: DSS: Generalize clock names Sumit Semwal
2011-01-25 12:59 ` [PATCH v2 1/4] OMAP2PLUS: opt-clocks: align dss clock roles and names Sumit Semwal
2011-01-27  5:53   ` Rajendra Nayak
2011-01-27 15:43     ` Rajendra Nayak [this message]
2011-01-25 12:59 ` [PATCH v2 2/4] OMAP2PLUS: DSS2: Generalize naming of PRCM related clock enums in DSS driver Sumit Semwal
2011-01-25 12:59 ` [PATCH v2 3/4] OMAP2PLUS: DSS2: Generalize external clock names in struct dss of dss.c Sumit Semwal
2011-01-25 12:59 ` [PATCH v2 4/4] OMAP4: DSS2: clocks: Add ick as dummy clock Sumit Semwal
2011-01-27 17:46   ` Kevin Hilman
2011-01-28  4:59     ` Semwal, Sumit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f3e9441abaf0813b2a7c4d7e9e2613d@mail.gmail.com \
    --to=rnayak@ti.com \
    --cc=hvaibhav@ti.com \
    --cc=khilman@ti.com \
    --cc=paul@pwsan.com \
    --cc=sumit.semwal@ti.com \
    --cc=tomi.valkeinen@nokia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox