[Patch v2 0/2] OMAP2PLUS:DSS2: use opt-clocks information from hwmod.

[Patch v2 0/2] OMAP2PLUS:DSS2: use opt-clocks information from hwmod.

[Sumit Semwal]

This series uses information about opt-clocks provided by omap_hwmod framework
to select which of the non-mandatory DSS clocks are available on a given
platform.

A function pointer opt_clock_available is exported via pdata, which checks the
clock roles returned by hwmod database.
In the driver, while doing clk-get, it is checked if as per hwmod a given clock
is an opt-clock or not, and is handled accordingly.

Tested on: OMAP3430sdp

Sumit Semwal (2):
  OMAP2PLUS:DSS2: add opt_clock_available in pdata
  OMAP2PLUS:DSS2: Use opt_clock_available from pdata

changes from v1:
- made opt_clock_available a function pointer in the omap_display_platform_data
  to avoid having to copy the opt-clock role names.

Based on top of:
http://gitorious.org/linux-omap-dss2/linux/commits/master
commit: ad1c76c: OMAP: Overo Palo43 LCD support


 arch/arm/mach-omap2/display.c             |   20 ++++++++++++
 arch/arm/plat-omap/include/plat/display.h |    2 +
 drivers/video/omap2/dss/dss.c             |   46 +++++++++++++++++++----------
 3 files changed, 52 insertions(+), 16 deletions(-)

[Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Sumit Semwal]

Provide a function in pdata to allow dss submodules to check if a given
clock is available on a platform as an optional clock.

Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
(based on implementation from Senthil)

Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
---
 arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
 arch/arm/plat-omap/include/plat/display.h |    2 ++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 5ab6a74..9f4f862 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
 	},
 };
 
+static bool opt_clock_available(const char *clk_role)
+{
+	int i;
+	struct omap_hwmod *oh;
+
+	/* opt_clks are always associated with dss hwmod */
+	oh = omap_hwmod_lookup("dss_core");
+	if (!oh) {
+		pr_err("Could not look up dss_core.\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < oh->opt_clks_cnt; i++) {
+		if (!strcmp(oh->opt_clks[i].role, clk_role))
+			return true;
+	}
+	return false;
+}
+
 int __init omap_display_init(struct omap_dss_board_info *board_data)
 {
 	int r = 0;
@@ -76,6 +95,7 @@ int __init omap_display_init(struct omap_dss_board_info *board_data)
 
 	pdata.board_data = board_data;
 	pdata.board_data->get_last_off_on_transaction_id = NULL;
+	pdata.opt_clock_available = opt_clock_available;
 
 	for (i = 0; i < oh_count; i++) {
 		oh = omap_hwmod_lookup(oh_name[i]);
diff --git a/arch/arm/plat-omap/include/plat/display.h b/arch/arm/plat-omap/include/plat/display.h
index 2fb057e..4101bcd 100644
--- a/arch/arm/plat-omap/include/plat/display.h
+++ b/arch/arm/plat-omap/include/plat/display.h
@@ -240,6 +240,8 @@ static inline int omap_display_init(struct omap_dss_board_info *board_data)
 struct omap_display_platform_data {
 	struct omap_dss_board_info *board_data;
 	/* TODO: Additional members to be added when PM is considered */
+
+	bool (*opt_clock_available)(const char *clk_role);
 };
 
 struct omap_video_timings {
-- 
1.7.1

[Patch v2 2/2] OMAP2PLUS:DSS2: Use opt_clock_available from pdata

[Sumit Semwal]

hwmod databases provide information about which optional clocks are available
for a given platform. This is available via a function pointer opt_clock_enable
in pdata.

Use this information during get/enable/disable/put of clocks.

Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
---
 drivers/video/omap2/dss/dss.c |   46 ++++++++++++++++++++++++++--------------
 1 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 5a93e66..3d0277d 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -699,6 +699,7 @@ static int dss_get_clock(struct clk **clock, const char *clk_name)
 static int dss_get_clocks(void)
 {
 	int r;
+	struct omap_display_platform_data *pdata = dss.pdev->dev.platform_data;
 
 	dss.dss_ick = NULL;
 	dss.dss_fck = NULL;
@@ -714,17 +715,28 @@ static int dss_get_clocks(void)
 	if (r)
 		goto err;
 
-	r = dss_get_clock(&dss.dss_sys_clk, "sys_clk");
-	if (r)
+	if (!pdata->opt_clock_available) {
+		r = -ENODEV;
 		goto err;
+	}
 
-	r = dss_get_clock(&dss.dss_tv_fck, "tv_clk");
-	if (r)
-		goto err;
+	if (pdata->opt_clock_available("sys_clk")) {
+		r = dss_get_clock(&dss.dss_sys_clk, "sys_clk");
+		if (r)
+			goto err;
+	}
 
-	r = dss_get_clock(&dss.dss_video_fck, "video_clk");
-	if (r)
-		goto err;
+	if (pdata->opt_clock_available("tv_clk")) {
+		r = dss_get_clock(&dss.dss_tv_fck, "tv_clk");
+		if (r)
+			goto err;
+	}
+
+	if (pdata->opt_clock_available("video_clk")) {
+		r = dss_get_clock(&dss.dss_video_fck, "video_clk");
+		if (r)
+			goto err;
+	}
 
 	return 0;
 
@@ -747,9 +759,11 @@ static void dss_put_clocks(void)
 {
 	if (dss.dss_video_fck)
 		clk_put(dss.dss_video_fck);
-	clk_put(dss.dss_tv_fck);
+	if (dss.dss_tv_fck)
+		clk_put(dss.dss_tv_fck);
+	if (dss.dss_sys_clk)
+		clk_put(dss.dss_sys_clk);
 	clk_put(dss.dss_fck);
-	clk_put(dss.dss_sys_clk);
 	clk_put(dss.dss_ick);
 }
 
@@ -798,11 +812,11 @@ static void dss_clk_enable_no_ctx(enum dss_clock clks)
 		clk_enable(dss.dss_ick);
 	if (clks & DSS_CLK_FCK)
 		clk_enable(dss.dss_fck);
-	if (clks & DSS_CLK_SYSCK)
+	if ((clks & DSS_CLK_SYSCK) && dss.dss_sys_clk)
 		clk_enable(dss.dss_sys_clk);
-	if (clks & DSS_CLK_TVFCK)
+	if ((clks & DSS_CLK_TVFCK) && dss.dss_tv_fck)
 		clk_enable(dss.dss_tv_fck);
-	if (clks & DSS_CLK_VIDFCK)
+	if ((clks & DSS_CLK_VIDFCK) && dss.dss_video_fck)
 		clk_enable(dss.dss_video_fck);
 
 	dss.num_clks_enabled += num_clks;
@@ -826,11 +840,11 @@ static void dss_clk_disable_no_ctx(enum dss_clock clks)
 		clk_disable(dss.dss_ick);
 	if (clks & DSS_CLK_FCK)
 		clk_disable(dss.dss_fck);
-	if (clks & DSS_CLK_SYSCK)
+	if ((clks & DSS_CLK_SYSCK) && dss.dss_sys_clk)
 		clk_disable(dss.dss_sys_clk);
-	if (clks & DSS_CLK_TVFCK)
+	if ((clks & DSS_CLK_TVFCK) && dss.dss_tv_fck)
 		clk_disable(dss.dss_tv_fck);
-	if (clks & DSS_CLK_VIDFCK)
+	if ((clks & DSS_CLK_VIDFCK) && dss.dss_video_fck)
 		clk_disable(dss.dss_video_fck);
 
 	dss.num_clks_enabled -= num_clks;
-- 
1.7.1

Re: [Patch v2 0/2] OMAP2PLUS:DSS2: use opt-clocks information from hwmod.

[Semwal, Sumit]

Minor correction:

On Mon, Feb 28, 2011 at 8:32 PM, Sumit Semwal <sumit.semwal@ti.com> wrote:
<snip>
> Based on top of:
> http://gitorious.org/linux-omap-dss2/linux/commits/master
> commit: ad1c76c: OMAP: Overo Palo43 LCD support
The correct base is:
http://gitorious.org/linux-omap-dss2/linux/commits/for-next
commit: 799a626: MAINTAINERS: Update OMAP DSS maintainer

Please let me know if you want me to rebase on top of master.

BR,
~S.
<snip>

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Tomi Valkeinen]

On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
> Provide a function in pdata to allow dss submodules to check if a given
> clock is available on a platform as an optional clock.
> 
> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> (based on implementation from Senthil)
> 
> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> ---
>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
>  arch/arm/plat-omap/include/plat/display.h |    2 ++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> index 5ab6a74..9f4f862 100644
> --- a/arch/arm/mach-omap2/display.c
> +++ b/arch/arm/mach-omap2/display.c
> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
>  	},
>  };
>  
> +static bool opt_clock_available(const char *clk_role)
> +{
> +	int i;
> +	struct omap_hwmod *oh;
> +
> +	/* opt_clks are always associated with dss hwmod */
> +	oh = omap_hwmod_lookup("dss_core");
> +	if (!oh) {
> +		pr_err("Could not look up dss_core.\n");
> +		return -ENODEV;

The function returns a bool. Perhaps the lookup would be better done in
omap_display_init(), and stored in a static variable.

Or if it's clear that this should never happen, BUG_ON(!oh);

 Tomi

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Semwal, Sumit]

On Mon, Feb 28, 2011 at 9:33 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
>> Provide a function in pdata to allow dss submodules to check if a given
>> clock is available on a platform as an optional clock.
>>
>> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
>> (based on implementation from Senthil)
>>
>> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
>> ---
>>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
>>  arch/arm/plat-omap/include/plat/display.h |    2 ++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>> index 5ab6a74..9f4f862 100644
>> --- a/arch/arm/mach-omap2/display.c
>> +++ b/arch/arm/mach-omap2/display.c
>> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
>>       },
>>  };
>>
>> +static bool opt_clock_available(const char *clk_role)
>> +{
>> +     int i;
>> +     struct omap_hwmod *oh;
>> +
>> +     /* opt_clks are always associated with dss hwmod */
>> +     oh = omap_hwmod_lookup("dss_core");
>> +     if (!oh) {
>> +             pr_err("Could not look up dss_core.\n");
>> +             return -ENODEV;
>
> The function returns a bool. Perhaps the lookup would be better done in
> omap_display_init(), and stored in a static variable.
Yes; I guess I should go with your earlier suggestion - of keeping
this 'oh' as part of pdata, and then use it here.
>
> Or if it's clear that this should never happen, BUG_ON(!oh);
Well, multiple people have 'frowned' upon using BUG_ON in dss -
especially because some people might still want their kernel to keep
working despite a display failure?

~Sumit.
>
>  Tomi
>
>
> --
> 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
>
--
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

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Tomi Valkeinen]

On Mon, 2011-02-28 at 23:42 -0600, Semwal, Sumit wrote:
> On Mon, Feb 28, 2011 at 9:33 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
> >> Provide a function in pdata to allow dss submodules to check if a given
> >> clock is available on a platform as an optional clock.
> >>
> >> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> >> (based on implementation from Senthil)
> >>
> >> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> >> ---
> >>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
> >>  arch/arm/plat-omap/include/plat/display.h |    2 ++
> >>  2 files changed, 22 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> >> index 5ab6a74..9f4f862 100644
> >> --- a/arch/arm/mach-omap2/display.c
> >> +++ b/arch/arm/mach-omap2/display.c
> >> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
> >>       },
> >>  };
> >>
> >> +static bool opt_clock_available(const char *clk_role)
> >> +{
> >> +     int i;
> >> +     struct omap_hwmod *oh;
> >> +
> >> +     /* opt_clks are always associated with dss hwmod */
> >> +     oh = omap_hwmod_lookup("dss_core");
> >> +     if (!oh) {
> >> +             pr_err("Could not look up dss_core.\n");
> >> +             return -ENODEV;
> >
> > The function returns a bool. Perhaps the lookup would be better done in
> > omap_display_init(), and stored in a static variable.
> Yes; I guess I should go with your earlier suggestion - of keeping
> this 'oh' as part of pdata, and then use it here.

I don't think it needs to be part of pdata. It's internal to this file,
and if I'm not mistaken, omap_display_init() is called just once from
the current board file. Thus the data could as well be stored in a
static variable.

> > Or if it's clear that this should never happen, BUG_ON(!oh);
> Well, multiple people have 'frowned' upon using BUG_ON in dss -
> especially because some people might still want their kernel to keep
> working despite a display failure?

Yes, BUG is an easy, not good, way out =).

 Tomi

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Semwal, Sumit]

On Tue, Mar 1, 2011 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Mon, 2011-02-28 at 23:42 -0600, Semwal, Sumit wrote:
>> On Mon, Feb 28, 2011 at 9:33 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
>> >> Provide a function in pdata to allow dss submodules to check if a given
>> >> clock is available on a platform as an optional clock.
>> >>
>> >> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
>> >> (based on implementation from Senthil)
>> >>
>> >> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
>> >> ---
>> >>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
>> >>  arch/arm/plat-omap/include/plat/display.h |    2 ++
>> >>  2 files changed, 22 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>> >> index 5ab6a74..9f4f862 100644
>> >> --- a/arch/arm/mach-omap2/display.c
>> >> +++ b/arch/arm/mach-omap2/display.c
>> >> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
>> >>       },
>> >>  };
>> >>
>> >> +static bool opt_clock_available(const char *clk_role)
>> >> +{
>> >> +     int i;
>> >> +     struct omap_hwmod *oh;
>> >> +
>> >> +     /* opt_clks are always associated with dss hwmod */
>> >> +     oh = omap_hwmod_lookup("dss_core");
>> >> +     if (!oh) {
>> >> +             pr_err("Could not look up dss_core.\n");
>> >> +             return -ENODEV;
>> >
>> > The function returns a bool. Perhaps the lookup would be better done in
>> > omap_display_init(), and stored in a static variable.
>> Yes; I guess I should go with your earlier suggestion - of keeping
>> this 'oh' as part of pdata, and then use it here.
>
> I don't think it needs to be part of pdata. It's internal to this file,
> and if I'm not mistaken, omap_display_init() is called just once from
> the current board file. Thus the data could as well be stored in a
> static variable.
:) ok, that's correct; though we might need to make 'oh' part of pdata
based on our adaptation to the reset functionality - but that will
come later.
>
>> > Or if it's clear that this should never happen, BUG_ON(!oh);
>> Well, multiple people have 'frowned' upon using BUG_ON in dss -
>> especially because some people might still want their kernel to keep
>> working despite a display failure?
>
> Yes, BUG is an easy, not good, way out =).
:) hmmm... should I just return 'false' and print out a warning?
>
>  Tomi
>
>
>
--
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

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Tomi Valkeinen]

On Tue, 2011-03-01 at 00:27 -0600, Semwal, Sumit wrote:
> On Tue, Mar 1, 2011 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Mon, 2011-02-28 at 23:42 -0600, Semwal, Sumit wrote:
> >> On Mon, Feb 28, 2011 at 9:33 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
> >> >> Provide a function in pdata to allow dss submodules to check if a given
> >> >> clock is available on a platform as an optional clock.
> >> >>
> >> >> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
> >> >> (based on implementation from Senthil)
> >> >>
> >> >> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
> >> >> ---
> >> >>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
> >> >>  arch/arm/plat-omap/include/plat/display.h |    2 ++
> >> >>  2 files changed, 22 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
> >> >> index 5ab6a74..9f4f862 100644
> >> >> --- a/arch/arm/mach-omap2/display.c
> >> >> +++ b/arch/arm/mach-omap2/display.c
> >> >> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
> >> >>       },
> >> >>  };
> >> >>
> >> >> +static bool opt_clock_available(const char *clk_role)
> >> >> +{
> >> >> +     int i;
> >> >> +     struct omap_hwmod *oh;
> >> >> +
> >> >> +     /* opt_clks are always associated with dss hwmod */
> >> >> +     oh = omap_hwmod_lookup("dss_core");
> >> >> +     if (!oh) {
> >> >> +             pr_err("Could not look up dss_core.\n");
> >> >> +             return -ENODEV;
> >> >
> >> > The function returns a bool. Perhaps the lookup would be better done in
> >> > omap_display_init(), and stored in a static variable.
> >> Yes; I guess I should go with your earlier suggestion - of keeping
> >> this 'oh' as part of pdata, and then use it here.
> >
> > I don't think it needs to be part of pdata. It's internal to this file,
> > and if I'm not mistaken, omap_display_init() is called just once from
> > the current board file. Thus the data could as well be stored in a
> > static variable.
> :) ok, that's correct; though we might need to make 'oh' part of pdata
> based on our adaptation to the reset functionality - but that will
> come later.
> >
> >> > Or if it's clear that this should never happen, BUG_ON(!oh);
> >> Well, multiple people have 'frowned' upon using BUG_ON in dss -
> >> especially because some people might still want their kernel to keep
> >> working despite a display failure?
> >
> > Yes, BUG is an easy, not good, way out =).
> :) hmmm... should I just return 'false' and print out a warning?

>From opt_clock_available() if the hwmod was not found? Well, preferably
opt_clock_available() will never be called, if the hwmod is not found in
omap_display_init(). So omap_display_init() should fail -> DSS driver
refuses to start -> opt_clock_available() is never called.

 Tomi

Re: [Patch v2 1/2] OMAP2PLUS:DSS2: add opt_clock_available in pdata

[Semwal, Sumit]

On Tue, Mar 1, 2011 at 12:07 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Tue, 2011-03-01 at 00:27 -0600, Semwal, Sumit wrote:
>> On Tue, Mar 1, 2011 at 11:49 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Mon, 2011-02-28 at 23:42 -0600, Semwal, Sumit wrote:
>> >> On Mon, Feb 28, 2011 at 9:33 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> >> > On Mon, 2011-02-28 at 09:02 -0600, Semwal, Sumit wrote:
>> >> >> Provide a function in pdata to allow dss submodules to check if a given
>> >> >> clock is available on a platform as an optional clock.
>> >> >>
>> >> >> Signed-off-by: Senthilvadivu Guruswamy <svadivu@ti.com>
>> >> >> (based on implementation from Senthil)
>> >> >>
>> >> >> Signed-off-by: Sumit Semwal <sumit.semwal@ti.com>
>> >> >> ---
>> >> >>  arch/arm/mach-omap2/display.c             |   20 ++++++++++++++++++++
>> >> >>  arch/arm/plat-omap/include/plat/display.h |    2 ++
>> >> >>  2 files changed, 22 insertions(+), 0 deletions(-)
>> >> >>
>> >> >> diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
>> >> >> index 5ab6a74..9f4f862 100644
>> >> >> --- a/arch/arm/mach-omap2/display.c
>> >> >> +++ b/arch/arm/mach-omap2/display.c
>> >> >> @@ -42,6 +42,25 @@ static struct omap_device_pm_latency omap_dss_latency[] = {
>> >> >>       },
>> >> >>  };
>> >> >>
>> >> >> +static bool opt_clock_available(const char *clk_role)
>> >> >> +{
>> >> >> +     int i;
>> >> >> +     struct omap_hwmod *oh;
>> >> >> +
>> >> >> +     /* opt_clks are always associated with dss hwmod */
>> >> >> +     oh = omap_hwmod_lookup("dss_core");
>> >> >> +     if (!oh) {
>> >> >> +             pr_err("Could not look up dss_core.\n");
>> >> >> +             return -ENODEV;
>> >> >
>> >> > The function returns a bool. Perhaps the lookup would be better done in
>> >> > omap_display_init(), and stored in a static variable.
>> >> Yes; I guess I should go with your earlier suggestion - of keeping
>> >> this 'oh' as part of pdata, and then use it here.
>> >
>> > I don't think it needs to be part of pdata. It's internal to this file,
>> > and if I'm not mistaken, omap_display_init() is called just once from
>> > the current board file. Thus the data could as well be stored in a
>> > static variable.
>> :) ok, that's correct; though we might need to make 'oh' part of pdata
>> based on our adaptation to the reset functionality - but that will
>> come later.
>> >
>> >> > Or if it's clear that this should never happen, BUG_ON(!oh);
>> >> Well, multiple people have 'frowned' upon using BUG_ON in dss -
>> >> especially because some people might still want their kernel to keep
>> >> working despite a display failure?
>> >
>> > Yes, BUG is an easy, not good, way out =).
>> :) hmmm... should I just return 'false' and print out a warning?
>
> From opt_clock_available() if the hwmod was not found? Well, preferably
> opt_clock_available() will never be called, if the hwmod is not found in
> omap_display_init(). So omap_display_init() should fail -> DSS driver
> refuses to start -> opt_clock_available() is never called.
:) Yes, that's true. will leave out the check then.
>
>  Tomi
>
>
>
--
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