* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP @ 2012-03-14 17:38 Maximilian Schwerin 2012-03-14 21:15 ` Kevin Hilman 0 siblings, 1 reply; 12+ messages in thread From: Maximilian Schwerin @ 2012-03-14 17:38 UTC (permalink / raw) To: linux-arm-kernel From: Steve Sakoman <steve@sakoman.com> Don't try to add IVA OPPs for OMAP3 versions not containing an IVA subsystem, as this would make omap_init_opp_table fail. Signed-off-by: Steve Sakoman <steve@sakoman.com> Signed-off-by: Maximilian Schwerin <mvs@tigris.de> --- arch/arm/mach-omap2/opp.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c index 9262a6b..414f2ec 100644 --- a/arch/arm/mach-omap2/opp.c +++ b/arch/arm/mach-omap2/opp.c @@ -62,6 +62,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, __func__, i); return -EINVAL; } + + if ((strcmp(opp_def->hwmod_name,"iva") == 0) && !omap3_has_iva()) + continue; + oh = omap_hwmod_lookup(opp_def->hwmod_name); if (!oh || !oh->od) { pr_warn("%s: no hwmod or odev for %s, [%d] " -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-14 17:38 [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP Maximilian Schwerin @ 2012-03-14 21:15 ` Kevin Hilman 2012-03-14 21:35 ` Menon, Nishanth 0 siblings, 1 reply; 12+ messages in thread From: Kevin Hilman @ 2012-03-14 21:15 UTC (permalink / raw) To: linux-arm-kernel Maximilian Schwerin <mvs@tigris.de> writes: > From: Steve Sakoman <steve@sakoman.com> > > Don't try to add IVA OPPs for OMAP3 versions not containing an IVA > subsystem, as this would make omap_init_opp_table fail. > > Signed-off-by: Steve Sakoman <steve@sakoman.com> > Signed-off-by: Maximilian Schwerin <mvs@tigris.de> Minor: patch subjects for arch/arm/* core code need to have the ARM: prefix also. Also, please run scripts/checkpatch.pl on your patch and fix the warnings. > --- > arch/arm/mach-omap2/opp.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index 9262a6b..414f2ec 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -62,6 +62,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, > __func__, i); > return -EINVAL; > } > + > + if ((strcmp(opp_def->hwmod_name,"iva") == 0) && !omap3_has_iva()) > + continue; > + > oh = omap_hwmod_lookup(opp_def->hwmod_name); > if (!oh || !oh->od) { > pr_warn("%s: no hwmod or odev for %s, [%d] " Wouldn't the one-liner below do the same thing? Actually, your patch makes it less noisy at boot time, avoiding the pr_warn(), so is probably better. Kevin diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c index 9262a6b..d3d4fa2 100644 --- a/arch/arm/mach-omap2/opp.c +++ b/arch/arm/mach-omap2/opp.c @@ -67,7 +67,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, pr_warn("%s: no hwmod or odev for %s, [%d] " "cannot add OPPs.\n", __func__, opp_def->hwmod_name, i); - return -EINVAL; + continue; } dev = &oh->od->pdev->dev; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-14 21:15 ` Kevin Hilman @ 2012-03-14 21:35 ` Menon, Nishanth 2012-03-14 22:06 ` Kevin Hilman 0 siblings, 1 reply; 12+ messages in thread From: Menon, Nishanth @ 2012-03-14 21:35 UTC (permalink / raw) To: linux-arm-kernel On Wed, Mar 14, 2012 at 16:15, Kevin Hilman <khilman@ti.com> wrote: > Maximilian Schwerin <mvs@tigris.de> writes: > >> From: Steve Sakoman <steve@sakoman.com> >> >> Don't try to add IVA OPPs for OMAP3 versions not containing an IVA >> subsystem, as this would make omap_init_opp_table fail. >> >> Signed-off-by: Steve Sakoman <steve@sakoman.com> >> Signed-off-by: Maximilian Schwerin <mvs@tigris.de> > > Minor: patch subjects for arch/arm/* core code need to have the ARM: > prefix also. > > Also, please run scripts/checkpatch.pl on your patch and fix the > warnings. > >> --- >> ?arch/arm/mach-omap2/opp.c | ? ?4 ++++ >> ?1 files changed, 4 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c >> index 9262a6b..414f2ec 100644 >> --- a/arch/arm/mach-omap2/opp.c >> +++ b/arch/arm/mach-omap2/opp.c >> @@ -62,6 +62,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, i); >> ? ? ? ? ? ? ? ? ? ? ? return -EINVAL; >> ? ? ? ? ? ? ? } >> + >> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == 0) && !omap3_has_iva()) >> + ? ? ? ? ? ? ? ? ? ? continue; >> + >> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); >> ? ? ? ? ? ? ? if (!oh || !oh->od) { >> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " > > Wouldn't the one-liner below do the same thing? > > Actually, your patch makes it less noisy at boot time, avoiding the > pr_warn(), so is probably better. The only issue i have with current patch is that it focusses to solve a specific device IVA. we could have similar variances if we had SGX/AESS device etc registered in the common table. a generic solution might be preferable - could we reduce the severity of pr_warn to pr_debug and do a continue instead? > > Kevin > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index 9262a6b..d3d4fa2 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -67,7 +67,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, > ? ? ? ? ? ? ? ? ? ? ? ?pr_warn("%s: no hwmod or odev for %s, [%d] " > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"cannot add OPPs.\n", __func__, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?opp_def->hwmod_name, i); > - ? ? ? ? ? ? ? ? ? ? ? return -EINVAL; > + ? ? ? ? ? ? ? ? ? ? ? continue; > ? ? ? ? ? ? ? ?} > ? ? ? ? ? ? ? ?dev = &oh->od->pdev->dev; Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-14 21:35 ` Menon, Nishanth @ 2012-03-14 22:06 ` Kevin Hilman 2012-03-16 9:26 ` AW: " Maximilian Schwerin 0 siblings, 1 reply; 12+ messages in thread From: Kevin Hilman @ 2012-03-14 22:06 UTC (permalink / raw) To: linux-arm-kernel "Menon, Nishanth" <nm@ti.com> writes: > On Wed, Mar 14, 2012 at 16:15, Kevin Hilman <khilman@ti.com> wrote: >> Maximilian Schwerin <mvs@tigris.de> writes: >> >>> From: Steve Sakoman <steve@sakoman.com> >>> >>> Don't try to add IVA OPPs for OMAP3 versions not containing an IVA >>> subsystem, as this would make omap_init_opp_table fail. >>> >>> Signed-off-by: Steve Sakoman <steve@sakoman.com> >>> Signed-off-by: Maximilian Schwerin <mvs@tigris.de> >> >> Minor: patch subjects for arch/arm/* core code need to have the ARM: >> prefix also. >> >> Also, please run scripts/checkpatch.pl on your patch and fix the >> warnings. >> >>> --- >>> ?arch/arm/mach-omap2/opp.c | ? ?4 ++++ >>> ?1 files changed, 4 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c >>> index 9262a6b..414f2ec 100644 >>> --- a/arch/arm/mach-omap2/opp.c >>> +++ b/arch/arm/mach-omap2/opp.c >>> @@ -62,6 +62,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, >>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, i); >>> ? ? ? ? ? ? ? ? ? ? ? return -EINVAL; >>> ? ? ? ? ? ? ? } >>> + >>> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == 0) && !omap3_has_iva()) >>> + ? ? ? ? ? ? ? ? ? ? continue; >>> + >>> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); >>> ? ? ? ? ? ? ? if (!oh || !oh->od) { >>> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " >> >> Wouldn't the one-liner below do the same thing? >> >> Actually, your patch makes it less noisy at boot time, avoiding the >> pr_warn(), so is probably better. > > The only issue i have with current patch is that it focusses to solve > a specific device IVA. > we could have similar variances if we had SGX/AESS device etc > registered in the common > table. a generic solution might be preferable - could we reduce the > severity of pr_warn to pr_debug and do a continue instead? I agree, that would be a better generic solution. Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* AW: [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-14 22:06 ` Kevin Hilman @ 2012-03-16 9:26 ` Maximilian Schwerin 2012-03-16 14:20 ` Nishanth Menon 0 siblings, 1 reply; 12+ messages in thread From: Maximilian Schwerin @ 2012-03-16 9:26 UTC (permalink / raw) To: linux-arm-kernel > Von: Kevin Hilman [mailto:khilman at ti.com] > > "Menon, Nishanth" <nm@ti.com> writes: > > > On Wed, Mar 14, 2012 at 16:15, Kevin Hilman <khilman@ti.com> wrote: > >> Maximilian Schwerin <mvs@tigris.de> writes: > >> > >>> From: Steve Sakoman <steve@sakoman.com> > >>> > >>> Don't try to add IVA OPPs for OMAP3 versions not containing an IVA > >>> subsystem, as this would make omap_init_opp_table fail. > >>> > >>> Signed-off-by: Steve Sakoman <steve@sakoman.com> > >>> Signed-off-by: Maximilian Schwerin <mvs@tigris.de> > >> > >> Minor: patch subjects for arch/arm/* core code need to > have the ARM: > >> prefix also. > >> > >> Also, please run scripts/checkpatch.pl on your patch and fix the > >> warnings. > >> > >>> --- > >>> ?arch/arm/mach-omap2/opp.c | ? ?4 ++++ > >>> ?1 files changed, 4 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > >>> index 9262a6b..414f2ec 100644 > >>> --- a/arch/arm/mach-omap2/opp.c > >>> +++ b/arch/arm/mach-omap2/opp.c > >>> @@ -62,6 +62,10 @@ int __init omap_init_opp_table(struct > omap_opp_def *opp_def, > >>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? __func__, i); > >>> ? ? ? ? ? ? ? ? ? ? ? return -EINVAL; > >>> ? ? ? ? ? ? ? } > >>> + > >>> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == > 0) && !omap3_has_iva()) > >>> + ? ? ? ? ? ? ? ? ? ? continue; > >>> + > >>> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); > >>> ? ? ? ? ? ? ? if (!oh || !oh->od) { > >>> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " > >> > >> Wouldn't the one-liner below do the same thing? > >> > >> Actually, your patch makes it less noisy at boot time, avoiding the > >> pr_warn(), so is probably better. > > > > The only issue i have with current patch is that it > focusses to solve > > a specific device IVA. > > we could have similar variances if we had SGX/AESS device etc > > registered in the common > > table. a generic solution might be preferable - could we reduce the > > severity of pr_warn to pr_debug and do a continue instead? > > I agree, that would be a better generic solution. > > Kevin > This is a pragmatic and simple solution for a well understood problem with no side effects. Why not fix the problem now and do the generic solution later on? I'm not a fulltime kernel dev. I have about two weeks to get my new board out to my customer... Every time I set up a new board, I have to fix problems using known patches that are sometimes years old. Every patch I have to find costs me hours of time I really don't have. Just my two cents (euro cents of course :-), Maximilian -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120316/16737b2b/attachment.sig> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-16 9:26 ` AW: " Maximilian Schwerin @ 2012-03-16 14:20 ` Nishanth Menon 2012-03-16 15:47 ` AW: " Maximilian Schwerin ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Nishanth Menon @ 2012-03-16 14:20 UTC (permalink / raw) To: linux-arm-kernel On 10:26-20120316, Maximilian Schwerin wrote: [...] > > >>> + > > >>> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == > > 0) && !omap3_has_iva()) > > >>> + ? ? ? ? ? ? ? ? ? ? continue; > > >>> + > > >>> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); > > >>> ? ? ? ? ? ? ? if (!oh || !oh->od) { > > >>> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " > > >> > > >> Wouldn't the one-liner below do the same thing? > > >> > > >> Actually, your patch makes it less noisy at boot time, avoiding the > > >> pr_warn(), so is probably better. > > > > > > The only issue i have with current patch is that it > > focusses to solve > > > a specific device IVA. > > > we could have similar variances if we had SGX/AESS device etc > > > registered in the common > > > table. a generic solution might be preferable - could we reduce the > > > severity of pr_warn to pr_debug and do a continue instead? > > > > I agree, that would be a better generic solution. > > > > Kevin > > > > This is a pragmatic and simple solution for a well understood problem with no side effects. Why not fix the problem now and do the generic solution later on? > > I'm not a fulltime kernel dev. I have about two weeks to get my new board out to my customer... Every time I set up a new board, I have to fix problems using known patches that are sometimes years old. Every patch I have to find costs me hours of time I really don't have. > > Just my two cents (euro cents of course :-), Maximilian ok, so lets fix it generically - here is the patch for it. Let us know if this works for you >From 5275d09c9f1a16c8f0814745e1c313c6cca049f6 Mon Sep 17 00:00:00 2001 From: Nishanth Menon <nm@ti.com> Date: Fri, 16 Mar 2012 09:13:24 -0500 Subject: [PATCH] OMAP2+: OPP: allow OPP enumeration to continue if device is not present On platforms such as OMAP3, certain variants may not have IVA, SGX or some specific component. We currently have a check to aid fixing wrong population of OPP entries for issues such as typos. This however causes a conflict with valid requirement where the SoC variant does not actually have the module present. So, reduce the severity of the print to a debug statement and skip registering that specific OPP, but continue down the list. Reported-by: Steve Sakoman <steve@sakoman.com> Reported-by: Maximilian Schwerin <mvs@tigris.de> Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm/mach-omap2/opp.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c index 9262a6b..de6d464 100644 --- a/arch/arm/mach-omap2/opp.c +++ b/arch/arm/mach-omap2/opp.c @@ -64,10 +64,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, } oh = omap_hwmod_lookup(opp_def->hwmod_name); if (!oh || !oh->od) { - pr_warn("%s: no hwmod or odev for %s, [%d] " + pr_debug("%s: no hwmod or odev for %s, [%d] " "cannot add OPPs.\n", __func__, opp_def->hwmod_name, i); - return -EINVAL; + continue; } dev = &oh->od->pdev->dev; -- 1.7.0.4 -- Regards, Nishanth Menon ^ permalink raw reply related [flat|nested] 12+ messages in thread
* AW: [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-16 14:20 ` Nishanth Menon @ 2012-03-16 15:47 ` Maximilian Schwerin 2012-03-16 15:57 ` Menon, Nishanth 2012-03-16 16:04 ` Steve Sakoman 2012-03-19 21:27 ` Kevin Hilman 2 siblings, 1 reply; 12+ messages in thread From: Maximilian Schwerin @ 2012-03-16 15:47 UTC (permalink / raw) To: linux-arm-kernel > -----Urspr?ngliche Nachricht----- > Von: Nishanth Menon [mailto:nm at ti.com] > Gesendet: Freitag, 16. M?rz 2012 15:21 > An: Maximilian Schwerin > Cc: Kevin Hilman; linux-omap at vger.kernel.org; > linux-arm-kernel at lists.infradead.org; Steve Sakoman > Betreff: Re: [PATCH] OMAP3: OPP: Test for IVA subsystem > before attempting to add IVA OPP > > On 10:26-20120316, Maximilian Schwerin wrote: > [...] > > > >>> + > > > >>> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == > > > 0) && !omap3_has_iva()) > > > >>> + ? ? ? ? ? ? ? ? ? ? continue; > > > >>> + > > > >>> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); > > > >>> ? ? ? ? ? ? ? if (!oh || !oh->od) { > > > >>> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev > for %s, [%d] " > > > >> > > > >> Wouldn't the one-liner below do the same thing? > > > >> > > > >> Actually, your patch makes it less noisy at boot time, > avoiding the > > > >> pr_warn(), so is probably better. > > > > > > > > The only issue i have with current patch is that it > > > focusses to solve > > > > a specific device IVA. > > > > we could have similar variances if we had SGX/AESS device etc > > > > registered in the common > > > > table. a generic solution might be preferable - could > we reduce the > > > > severity of pr_warn to pr_debug and do a continue instead? > > > > > > I agree, that would be a better generic solution. > > > > > > Kevin > > > > > > > This is a pragmatic and simple solution for a well > understood problem with no side effects. Why not fix the > problem now and do the generic solution later on? > > > > I'm not a fulltime kernel dev. I have about two weeks to > get my new board out to my customer... Every time I set up a > new board, I have to fix problems using known patches that > are sometimes years old. Every patch I have to find costs me > hours of time I really don't have. > > > > Just my two cents (euro cents of course :-), Maximilian > ok, so lets fix it generically - here is the patch for it. Let us know > if this works for you > > > From 5275d09c9f1a16c8f0814745e1c313c6cca049f6 Mon Sep 17 00:00:00 2001 > From: Nishanth Menon <nm@ti.com> > Date: Fri, 16 Mar 2012 09:13:24 -0500 > Subject: [PATCH] OMAP2+: OPP: allow OPP enumeration to > continue if device is not present > > On platforms such as OMAP3, certain variants may not have IVA, SGX > or some specific component. We currently have a check to aid fixing > wrong population of OPP entries for issues such as typos. This however > causes a conflict with valid requirement where the SoC variant does > not actually have the module present. > > So, reduce the severity of the print to a debug statement and skip > registering that specific OPP, but continue down the list. > > Reported-by: Steve Sakoman <steve@sakoman.com> > Reported-by: Maximilian Schwerin <mvs@tigris.de> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > arch/arm/mach-omap2/opp.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index 9262a6b..de6d464 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -64,10 +64,10 @@ int __init omap_init_opp_table(struct > omap_opp_def *opp_def, > } > oh = omap_hwmod_lookup(opp_def->hwmod_name); > if (!oh || !oh->od) { > - pr_warn("%s: no hwmod or odev for %s, [%d] " > + pr_debug("%s: no hwmod or odev for %s, [%d] " > "cannot add OPPs.\n", __func__, > opp_def->hwmod_name, i); > - return -EINVAL; > + continue; > } > dev = &oh->od->pdev->dev; > > -- > 1.7.0.4 > > > -- > Regards, > Nishanth Menon Hi, sorry my fault! This was not what I was thinking of as generic. Works as expected! Thanks, m. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120316/3c333903/attachment.sig> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-16 15:47 ` AW: " Maximilian Schwerin @ 2012-03-16 15:57 ` Menon, Nishanth 0 siblings, 0 replies; 12+ messages in thread From: Menon, Nishanth @ 2012-03-16 15:57 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 16, 2012 at 10:47, Maximilian Schwerin <Maximilian.Schwerin@tigris.de> wrote: > > sorry my fault! This was not what I was thinking of as generic. Works as > expected! Can i take it as an acked-by? Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-16 14:20 ` Nishanth Menon 2012-03-16 15:47 ` AW: " Maximilian Schwerin @ 2012-03-16 16:04 ` Steve Sakoman 2012-03-19 21:27 ` Kevin Hilman 2 siblings, 0 replies; 12+ messages in thread From: Steve Sakoman @ 2012-03-16 16:04 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 16, 2012 at 7:20 AM, Nishanth Menon <nm@ti.com> wrote: > From 5275d09c9f1a16c8f0814745e1c313c6cca049f6 Mon Sep 17 00:00:00 2001 > From: Nishanth Menon <nm@ti.com> > Date: Fri, 16 Mar 2012 09:13:24 -0500 > Subject: [PATCH] OMAP2+: OPP: allow OPP enumeration to continue if device is not present > > On platforms such as OMAP3, certain variants may not have IVA, SGX > or some specific component. We currently have a check to aid fixing > wrong population of OPP entries for issues such as typos. This however > causes a conflict with valid requirement where the SoC variant does > not actually have the module present. > > So, reduce the severity of the print to a debug statement and skip > registering that specific OPP, but continue down the list. > > Reported-by: Steve Sakoman <steve@sakoman.com> > Reported-by: Maximilian Schwerin <mvs@tigris.de> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > ?arch/arm/mach-omap2/opp.c | ? ?4 ++-- > ?1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index 9262a6b..de6d464 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -64,10 +64,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, > ? ? ? ? ? ? ? ?} > ? ? ? ? ? ? ? ?oh = omap_hwmod_lookup(opp_def->hwmod_name); > ? ? ? ? ? ? ? ?if (!oh || !oh->od) { > - ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " > + ? ? ? ? ? ? ? ? ? ? ? pr_debug("%s: no hwmod or odev for %s, [%d] " > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"cannot add OPPs.\n", __func__, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?opp_def->hwmod_name, i); > - ? ? ? ? ? ? ? ? ? ? ? return -EINVAL; > + ? ? ? ? ? ? ? ? ? ? ? continue; > ? ? ? ? ? ? ? ?} > ? ? ? ? ? ? ? ?dev = &oh->od->pdev->dev; > > -- > 1.7.0.4 Acked-by: Steve Sakoman <steve@sakoman.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-16 14:20 ` Nishanth Menon 2012-03-16 15:47 ` AW: " Maximilian Schwerin 2012-03-16 16:04 ` Steve Sakoman @ 2012-03-19 21:27 ` Kevin Hilman 2012-03-19 21:29 ` Menon, Nishanth 2 siblings, 1 reply; 12+ messages in thread From: Kevin Hilman @ 2012-03-19 21:27 UTC (permalink / raw) To: linux-arm-kernel Nishanth Menon <nm@ti.com> writes: > On 10:26-20120316, Maximilian Schwerin wrote: > [...] >> > >>> + >> > >>> + ? ? ? ? ? ? if ((strcmp(opp_def->hwmod_name,"iva") == >> > 0) && !omap3_has_iva()) >> > >>> + ? ? ? ? ? ? ? ? ? ? continue; >> > >>> + >> > >>> ? ? ? ? ? ? ? oh = omap_hwmod_lookup(opp_def->hwmod_name); >> > >>> ? ? ? ? ? ? ? if (!oh || !oh->od) { >> > >>> ? ? ? ? ? ? ? ? ? ? ? pr_warn("%s: no hwmod or odev for %s, [%d] " >> > >> >> > >> Wouldn't the one-liner below do the same thing? >> > >> >> > >> Actually, your patch makes it less noisy at boot time, avoiding the >> > >> pr_warn(), so is probably better. >> > > >> > > The only issue i have with current patch is that it >> > focusses to solve >> > > a specific device IVA. >> > > we could have similar variances if we had SGX/AESS device etc >> > > registered in the common >> > > table. a generic solution might be preferable - could we reduce the >> > > severity of pr_warn to pr_debug and do a continue instead? >> > >> > I agree, that would be a better generic solution. >> > >> > Kevin >> > >> >> This is a pragmatic and simple solution for a well understood problem with no side effects. Why not fix the problem now and do the generic solution later on? >> >> I'm not a fulltime kernel dev. I have about two weeks to get my new board out to my customer... Every time I set up a new board, I have to fix problems using known patches that are sometimes years old. Every patch I have to find costs me hours of time I really don't have. >> >> Just my two cents (euro cents of course :-), Maximilian > ok, so lets fix it generically - here is the patch for it. Let us know > if this works for you > > > From 5275d09c9f1a16c8f0814745e1c313c6cca049f6 Mon Sep 17 00:00:00 2001 > From: Nishanth Menon <nm@ti.com> > Date: Fri, 16 Mar 2012 09:13:24 -0500 > Subject: [PATCH] OMAP2+: OPP: allow OPP enumeration to continue if device is not present > > On platforms such as OMAP3, certain variants may not have IVA, SGX > or some specific component. We currently have a check to aid fixing > wrong population of OPP entries for issues such as typos. This however > causes a conflict with valid requirement where the SoC variant does > not actually have the module present. > > So, reduce the severity of the print to a debug statement and skip > registering that specific OPP, but continue down the list. > > Reported-by: Steve Sakoman <steve@sakoman.com> > Reported-by: Maximilian Schwerin <mvs@tigris.de> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > arch/arm/mach-omap2/opp.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c > index 9262a6b..de6d464 100644 > --- a/arch/arm/mach-omap2/opp.c > +++ b/arch/arm/mach-omap2/opp.c > @@ -64,10 +64,10 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def, > } > oh = omap_hwmod_lookup(opp_def->hwmod_name); > if (!oh || !oh->od) { > - pr_warn("%s: no hwmod or odev for %s, [%d] " > + pr_debug("%s: no hwmod or odev for %s, [%d] " > "cannot add OPPs.\n", __func__, > opp_def->hwmod_name, i); > - return -EINVAL; > + continue; > } > dev = &oh->od->pdev->dev; > Yes, thanks for doing this more genericly. Nishanth, can you collect the acks/tested-bys and repost and official patch. I'll queue this up. Thanks, Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-19 21:27 ` Kevin Hilman @ 2012-03-19 21:29 ` Menon, Nishanth 2012-03-20 0:15 ` Kevin Hilman 0 siblings, 1 reply; 12+ messages in thread From: Menon, Nishanth @ 2012-03-19 21:29 UTC (permalink / raw) To: linux-arm-kernel Kevin, [...] > Nishanth, can you collect the acks/tested-bys and repost and official > patch. > > I'll queue this up. Thanks. This is already done: http://marc.info/?l=linux-omap&m=133191481703750&w=2 Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP 2012-03-19 21:29 ` Menon, Nishanth @ 2012-03-20 0:15 ` Kevin Hilman 0 siblings, 0 replies; 12+ messages in thread From: Kevin Hilman @ 2012-03-20 0:15 UTC (permalink / raw) To: linux-arm-kernel "Menon, Nishanth" <nm@ti.com> writes: > Kevin, > [...] >> Nishanth, can you collect the acks/tested-bys and repost and official >> patch. >> >> I'll queue this up. > Thanks. This is already done: > http://marc.info/?l=linux-omap&m=133191481703750&w=2 Yeah, sorry for the noise. I saw it after I sent this request. Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-03-20 0:15 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-14 17:38 [PATCH] OMAP3: OPP: Test for IVA subsystem before attempting to add IVA OPP Maximilian Schwerin 2012-03-14 21:15 ` Kevin Hilman 2012-03-14 21:35 ` Menon, Nishanth 2012-03-14 22:06 ` Kevin Hilman 2012-03-16 9:26 ` AW: " Maximilian Schwerin 2012-03-16 14:20 ` Nishanth Menon 2012-03-16 15:47 ` AW: " Maximilian Schwerin 2012-03-16 15:57 ` Menon, Nishanth 2012-03-16 16:04 ` Steve Sakoman 2012-03-19 21:27 ` Kevin Hilman 2012-03-19 21:29 ` Menon, Nishanth 2012-03-20 0:15 ` Kevin Hilman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).