[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Paul Walmsley]

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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Kevin Hilman]

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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Tony Lindgren]

* 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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Tony Lindgren]

* 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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Paul Walmsley]

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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Tony Lindgren]

* 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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Paul Walmsley]

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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Tony Lindgren]

* 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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Paul Walmsley]

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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Tony Lindgren]

* 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

[PATCH] ARM: OMAP3: clockdomain: fix accidental duplicate registration

[Paul Walmsley]

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