* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
@ 2012-07-12 10:54 Paul Walmsley
2012-07-12 17:50 ` Kevin Hilman
0 siblings, 1 reply; 11+ messages in thread
From: Paul Walmsley @ 2012-07-12 10:54 UTC (permalink / raw)
To: linux-arm-kernel
Commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
'omap-cleanup-for-v3.6' into tmp-merge") inadvertently caused a
clockdomain to be registered twice on OMAP3 boards. This causes warnings
on boot, wild pointer dereferences, and PM regressions. Fix the double
registration and add some clockdomain code to prevent this from happening
again.
Reported-by: Joe Woodward <jw@terrafix.co.uk>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Kevin Hilman <khilman@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
Intended for v3.6 merge window. Applies on linux-omap commit
3dd50d0545bd5a8ad83d4339f07935cd3e883271. Boot-tested on 2430 SDP,
3517 EVM, 37xx EVM, 5912 OSK, and CM-T 3517.
arch/arm/mach-omap2/clockdomain.c | 19 ++++++++++++++-----
arch/arm/mach-omap2/clockdomain.h | 1 +
arch/arm/mach-omap2/clockdomains3xxx_data.c | 1 -
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index 8664f5a..b851ba4 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -63,9 +63,10 @@ static struct clockdomain *_clkdm_lookup(const char *name)
* _clkdm_register - register a clockdomain
* @clkdm: struct clockdomain * to register
*
- * Adds a clockdomain to the internal clockdomain list.
- * Returns -EINVAL if given a null pointer, -EEXIST if a clockdomain is
- * already registered by the provided name, or 0 upon success.
+ * Adds a clockdomain to the internal clockdomain list. Returns
+ * -EINVAL if given a null pointer, -EEXIST if a clockdomain is
+ * already registered by the provided name or if @clkdm has already
+ * been registered, or 0 upon success.
*/
static int _clkdm_register(struct clockdomain *clkdm)
{
@@ -74,6 +75,11 @@ static int _clkdm_register(struct clockdomain *clkdm)
if (!clkdm || !clkdm->name)
return -EINVAL;
+ if (clkdm->_flags & _CLKDM_FLAG_REGISTERED) {
+ pr_err("clockdomain: %s: already registered\n", clkdm->name);
+ return -EEXIST;
+ }
+
pwrdm = pwrdm_lookup(clkdm->pwrdm.name);
if (!pwrdm) {
pr_err("clockdomain: %s: powerdomain %s does not exist\n",
@@ -82,15 +88,18 @@ static int _clkdm_register(struct clockdomain *clkdm)
}
clkdm->pwrdm.ptr = pwrdm;
- /* Verify that the clockdomain is not already registered */
- if (_clkdm_lookup(clkdm->name))
+ if (_clkdm_lookup(clkdm->name)) {
+ pr_err("clockdomain: %s: name already registered\n",
+ clkdm->name);
return -EEXIST;
+ }
list_add(&clkdm->node, &clkdm_list);
pwrdm_add_clkdm(pwrdm, clkdm);
spin_lock_init(&clkdm->lock);
+ clkdm->_flags |= _CLKDM_FLAG_REGISTERED;
pr_debug("clockdomain: registered %s\n", clkdm->name);
diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h
index 5601dc1..7b3c1d2 100644
--- a/arch/arm/mach-omap2/clockdomain.h
+++ b/arch/arm/mach-omap2/clockdomain.h
@@ -86,6 +86,7 @@ struct clkdm_dep {
/* Possible flags for struct clockdomain._flags */
#define _CLKDM_FLAG_HWSUP_ENABLED BIT(0)
+#define _CLKDM_FLAG_REGISTERED BIT(1)
/**
* struct clockdomain - OMAP clockdomain
diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c b/arch/arm/mach-omap2/clockdomains3xxx_data.c
index d625844..56089c4 100644
--- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
@@ -454,7 +454,6 @@ static struct clkdm_autodep clkdm_am35x_autodeps[] = {
static struct clockdomain *clockdomains_common[] __initdata = {
&wkup_common_clkdm,
- &mpu_3xxx_clkdm,
&neon_clkdm,
&core_l3_3xxx_clkdm,
&core_l4_3xxx_clkdm,
--
1.7.10
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-12 10:54 [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration Paul Walmsley
@ 2012-07-12 17:50 ` Kevin Hilman
2012-07-13 6:47 ` Tony Lindgren
0 siblings, 1 reply; 11+ messages in thread
From: Kevin Hilman @ 2012-07-12 17:50 UTC (permalink / raw)
To: linux-arm-kernel
Paul Walmsley <paul@pwsan.com> writes:
> Commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
> 'omap-cleanup-for-v3.6' into tmp-merge") inadvertently caused a
> clockdomain to be registered twice on OMAP3 boards. This causes warnings
> on boot, wild pointer dereferences, and PM regressions. Fix the double
> registration and add some clockdomain code to prevent this from happening
> again.
>
> Reported-by: Joe Woodward <jw@terrafix.co.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
Acked-by: Kevin Hilman <khilman@ti.com>
This fixes the problem I reported here:
http://marc.info/?l=linux-omap&m=134203768012711&w=2
Thanks,
Kevin
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-12 17:50 ` Kevin Hilman
@ 2012-07-13 6:47 ` Tony Lindgren
2012-07-14 8:52 ` Tony Lindgren
0 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2012-07-13 6:47 UTC (permalink / raw)
To: linux-arm-kernel
* Kevin Hilman <khilman@ti.com> [120712 10:55]:
> Paul Walmsley <paul@pwsan.com> writes:
>
> > Commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
> > 'omap-cleanup-for-v3.6' into tmp-merge") inadvertently caused a
> > clockdomain to be registered twice on OMAP3 boards. This causes warnings
> > on boot, wild pointer dereferences, and PM regressions. Fix the double
> > registration and add some clockdomain code to prevent this from happening
> > again.
> >
> > Reported-by: Joe Woodward <jw@terrafix.co.uk>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Kevin Hilman <khilman@ti.com>
> > Signed-off-by: Paul Walmsley <paul@pwsan.com>
>
> Acked-by: Kevin Hilman <khilman@ti.com>
>
> This fixes the problem I reported here:
>
> http://marc.info/?l=linux-omap&m=134203768012711&w=2
OK thanks good to hear.
Regards,
Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-13 6:47 ` Tony Lindgren
@ 2012-07-14 8:52 ` Tony Lindgren
2012-07-14 17:54 ` Paul Walmsley
0 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2012-07-14 8:52 UTC (permalink / raw)
To: linux-arm-kernel
* Tony Lindgren <tony@atomide.com> [120712 23:52]:
> * Kevin Hilman <khilman@ti.com> [120712 10:55]:
> > Paul Walmsley <paul@pwsan.com> writes:
> >
> > > Commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
> > > 'omap-cleanup-for-v3.6' into tmp-merge") inadvertently caused a
> > > clockdomain to be registered twice on OMAP3 boards. This causes warnings
> > > on boot, wild pointer dereferences, and PM regressions. Fix the double
> > > registration and add some clockdomain code to prevent this from happening
> > > again.
As the tmp-merge branch has other branches and never goes upstream, the
commit should say:
Commit 472fd5401561f94698f4c8f9dbbbfbf76ab55626 (Merge branch 'cleanup-hwmod'
into cleanup)... So I've updated the patch and applied it into l-o master.
The patch seems to produce a new warning against arm soc tree next/cleanup
branch:
warning: ?mpu_3xxx_clkdm? defined but not used
Paul, care to check if a change is needed for the arm soc tree
next/cleanup branch version of this patch?
Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-14 8:52 ` Tony Lindgren
@ 2012-07-14 17:54 ` Paul Walmsley
2012-07-16 8:36 ` Tony Lindgren
0 siblings, 1 reply; 11+ messages in thread
From: Paul Walmsley @ 2012-07-14 17:54 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 14 Jul 2012, Tony Lindgren wrote:
> As the tmp-merge branch has other branches and never goes upstream, the
> commit should say:
>
> Commit 472fd5401561f94698f4c8f9dbbbfbf76ab55626 (Merge branch 'cleanup-hwmod'
> into cleanup)... So I've updated the patch and applied it into l-o master.
Thanks for fixing it.
> The patch seems to produce a new warning against arm soc tree
> next/cleanup branch:
>
> warning: ?mpu_3xxx_clkdm? defined but not used
>
> Paul, care to check if a change is needed for the arm soc tree
> next/cleanup branch version of this patch?
arm-soc next/cleanup branch doesn't include commit 16e5e2c4 ("ARM: OMAP
AM35x: clockdomain data: Fix clockdomain dependencies"). So that patch
won't apply there, and the copy of mach-omap2/clockdomains3xxx_data.c in
that branch is clean.
- Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-14 17:54 ` Paul Walmsley
@ 2012-07-16 8:36 ` Tony Lindgren
2012-07-18 9:53 ` Paul Walmsley
0 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2012-07-16 8:36 UTC (permalink / raw)
To: linux-arm-kernel
* Paul Walmsley <paul@pwsan.com> [120714 10:59]:
> On Sat, 14 Jul 2012, Tony Lindgren wrote:
>
> > The patch seems to produce a new warning against arm soc tree
> > next/cleanup branch:
> >
> > warning: ?mpu_3xxx_clkdm? defined but not used
> >
> > Paul, care to check if a change is needed for the arm soc tree
> > next/cleanup branch version of this patch?
>
> arm-soc next/cleanup branch doesn't include commit 16e5e2c4 ("ARM: OMAP
> AM35x: clockdomain data: Fix clockdomain dependencies"). So that patch
> won't apply there, and the copy of mach-omap2/clockdomains3xxx_data.c in
> that branch is clean.
Hmm well it seems that we should apply this fix into arm-soc next/cleanup
branch if that's where the mismerge happened? It seems the same mismerge
is there even without 16e5e2c4?
You patch applies into arm-soc next/cleanup with fuzz:
patching file arch/arm/mach-omap2/clockdomain.c
patching file arch/arm/mach-omap2/clockdomain.h
Hunk #1 succeeded at 82 (offset -4 lines).
patching file arch/arm/mach-omap2/clockdomains3xxx_data.c
Hunk #1 succeeded at 347 with fuzz 2 (offset -107 lines).
So that's why I'm wondering if it needs some changes.
Regards,
Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-16 8:36 ` Tony Lindgren
@ 2012-07-18 9:53 ` Paul Walmsley
2012-07-19 11:52 ` Tony Lindgren
0 siblings, 1 reply; 11+ messages in thread
From: Paul Walmsley @ 2012-07-18 9:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi
On Mon, 16 Jul 2012, Tony Lindgren wrote:
> Hmm well it seems that we should apply this fix into arm-soc next/cleanup
> branch if that's where the mismerge happened? It seems the same mismerge
> is there even without 16e5e2c4?
The arm-soc next/cleanup copy of mach-omap2/clockdomains3xxx_data.c is
correct. The problem that my patch was designed to fix doesn't exist in
that version of the file (868c157df9721675c19729eed2c96bac6c3f1d01).
> You patch applies into arm-soc next/cleanup with fuzz:
>
> patching file arch/arm/mach-omap2/clockdomain.c
> patching file arch/arm/mach-omap2/clockdomain.h
> Hunk #1 succeeded at 82 (offset -4 lines).
> patching file arch/arm/mach-omap2/clockdomains3xxx_data.c
> Hunk #1 succeeded at 347 with fuzz 2 (offset -107 lines).
>
> So that's why I'm wondering if it needs some changes.
OK, I understand why you're asking.
I went back and researched it. The patch that I sent is only needed
because the conflict resolution in merge commit
3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
'omap-cleanup-for-v3.6' into tmp-merge") adds &mpu_3xxx_clkdm back into
the clockdomains_common list. The previous commit on that file
16e5e2c471ab889f838bfe1c44032d0481c115e1 removed &mpu_3xxx_clkdm from the
common list, because the AM35xx chips needed to use a different MPU
clockdomain structure from the OMAP3xxx chips.
Or, put differently: Before 16e5e2c471ab889f838bfe1c44032d0481c115e1, it
was correct to have &mpu_3xxx_clkdm in the common list. That's
what's in arm-soc next/cleanup and that data is correct.
After 16e5e2c471ab889f838bfe1c44032d0481c115e1, it's incorrect to have
&mpu_3xxx_clkdm in the common list.
Hope this isn't even more confusing :-)
- Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-18 9:53 ` Paul Walmsley
@ 2012-07-19 11:52 ` Tony Lindgren
2012-07-19 19:12 ` Paul Walmsley
0 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2012-07-19 11:52 UTC (permalink / raw)
To: linux-arm-kernel
* Paul Walmsley <paul@pwsan.com> [120718 02:58]:
> Hi
>
> On Mon, 16 Jul 2012, Tony Lindgren wrote:
>
> > Hmm well it seems that we should apply this fix into arm-soc next/cleanup
> > branch if that's where the mismerge happened? It seems the same mismerge
> > is there even without 16e5e2c4?
>
> The arm-soc next/cleanup copy of mach-omap2/clockdomains3xxx_data.c is
> correct. The problem that my patch was designed to fix doesn't exist in
> that version of the file (868c157df9721675c19729eed2c96bac6c3f1d01).
OK
> > You patch applies into arm-soc next/cleanup with fuzz:
> >
> > patching file arch/arm/mach-omap2/clockdomain.c
> > patching file arch/arm/mach-omap2/clockdomain.h
> > Hunk #1 succeeded at 82 (offset -4 lines).
> > patching file arch/arm/mach-omap2/clockdomains3xxx_data.c
> > Hunk #1 succeeded at 347 with fuzz 2 (offset -107 lines).
> >
> > So that's why I'm wondering if it needs some changes.
>
> OK, I understand why you're asking.
>
> I went back and researched it. The patch that I sent is only needed
> because the conflict resolution in merge commit
> 3dd50d0545bd5a8ad83d4339f07935cd3e883271 ("Merge tag
> 'omap-cleanup-for-v3.6' into tmp-merge") adds &mpu_3xxx_clkdm back into
> the clockdomains_common list. The previous commit on that file
> 16e5e2c471ab889f838bfe1c44032d0481c115e1 removed &mpu_3xxx_clkdm from the
> common list, because the AM35xx chips needed to use a different MPU
> clockdomain structure from the OMAP3xxx chips.
>
> Or, put differently: Before 16e5e2c471ab889f838bfe1c44032d0481c115e1, it
> was correct to have &mpu_3xxx_clkdm in the common list. That's
> what's in arm-soc next/cleanup and that data is correct.
>
> After 16e5e2c471ab889f838bfe1c44032d0481c115e1, it's incorrect to have
> &mpu_3xxx_clkdm in the common list.
>
> Hope this isn't even more confusing :-)
Well I'm still a bit confused :)
Which branch in arm-soc tree should this fix be applied then?
Or do we actually need two fixes into arm-soc tree?
Regards,
Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-19 11:52 ` Tony Lindgren
@ 2012-07-19 19:12 ` Paul Walmsley
2012-07-24 8:16 ` Tony Lindgren
0 siblings, 1 reply; 11+ messages in thread
From: Paul Walmsley @ 2012-07-19 19:12 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 19 Jul 2012, Tony Lindgren wrote:
> Well I'm still a bit confused :)
>
> Which branch in arm-soc tree should this fix be applied then?
In terms of arm-soc, it's needed in arm-soc for-next, due to commit
066b6eba6d58ad1cb9ec3917b6ee79730c3c3310 ("Merge branch 'next/cleanup'
into for-next"). That merge commit resolves the conflict the same way
that the linux-omap tree commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271
("Merge tag 'omap-cleanup-for-v3.6' into tmp-merge") did.
066b6eba is also present in a few other arm-soc branches:
arm-soc/staging/io-cleanup-pci
arm-soc/tmp
I did not do an exhaustive search for similar mismerges under different
commit IDs.
- Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-19 19:12 ` Paul Walmsley
@ 2012-07-24 8:16 ` Tony Lindgren
2012-07-24 20:52 ` Paul Walmsley
0 siblings, 1 reply; 11+ messages in thread
From: Tony Lindgren @ 2012-07-24 8:16 UTC (permalink / raw)
To: linux-arm-kernel
* Paul Walmsley <paul@pwsan.com> [120719 12:17]:
> On Thu, 19 Jul 2012, Tony Lindgren wrote:
>
> > Well I'm still a bit confused :)
> >
> > Which branch in arm-soc tree should this fix be applied then?
>
> In terms of arm-soc, it's needed in arm-soc for-next, due to commit
> 066b6eba6d58ad1cb9ec3917b6ee79730c3c3310 ("Merge branch 'next/cleanup'
> into for-next"). That merge commit resolves the conflict the same way
> that the linux-omap tree commit 3dd50d0545bd5a8ad83d4339f07935cd3e883271
> ("Merge tag 'omap-cleanup-for-v3.6' into tmp-merge") did.
>
> 066b6eba is also present in a few other arm-soc branches:
>
> arm-soc/staging/io-cleanup-pci
> arm-soc/tmp
>
> I did not do an exhaustive search for similar mismerges under different
> commit IDs.
OK looks like Linus fixed up part of it, care to check what's needed
against current mainline now?
Tony
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration
2012-07-24 8:16 ` Tony Lindgren
@ 2012-07-24 20:52 ` Paul Walmsley
0 siblings, 0 replies; 11+ messages in thread
From: Paul Walmsley @ 2012-07-24 20:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 24 Jul 2012, Tony Lindgren wrote:
> OK looks like Linus fixed up part of it, care to check what's needed
> against current mainline now?
Current mainline seems to be okay here.
- Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-07-24 20:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-12 10:54 [PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration Paul Walmsley
2012-07-12 17:50 ` Kevin Hilman
2012-07-13 6:47 ` Tony Lindgren
2012-07-14 8:52 ` Tony Lindgren
2012-07-14 17:54 ` Paul Walmsley
2012-07-16 8:36 ` Tony Lindgren
2012-07-18 9:53 ` Paul Walmsley
2012-07-19 11:52 ` Tony Lindgren
2012-07-19 19:12 ` Paul Walmsley
2012-07-24 8:16 ` Tony Lindgren
2012-07-24 20:52 ` Paul Walmsley
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).