ARM: OMAP3: clockdomain: fix accidental duplicate registration

PATCH New Paul Walmsley 3 participants 11 messages Jul 12, 2012 – Jul 24, 2012

Subsystems (4)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP clock framework support

lore

Share & tools

lore

Paul Walmsley

author maintainer

2012-07-12 10:54 UTC #0 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

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 <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Signed-off-by Paul Walmsley <[email protected]>

---
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(-)

arch/arm/mach-omap2/clockdomain.c +14 -5

index 8664f5a..b851ba4 100644
@@ -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);

arch/arm/mach-omap2/clockdomain.h +1 -0

index 5601dc1..7b3c1d2 100644
@@ -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

arch/arm/mach-omap2/clockdomains3xxx_data.c +0 -1

index d625844..56089c4 100644
@@ -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,

Signature · 1 line

1.7.10

Kevin Hilman

2012-07-12 17:50 UTC #1 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 11 lines · 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 <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Signed-off-by Paul Walmsley <[email protected]>

Acked-by Kevin Hilman <[email protected]>

This fixes the problem I reported here:

     http://marc.info/?l=linux-omap&m=134203768012711&w=2

Thanks,

Kevin

Tony Lindgren

maintainer

2012-07-13 06:47 UTC #2 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 14 lines · Kevin Hilman

Quoted 11 lines · 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 <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Signed-off-by Paul Walmsley <[email protected]>

Quoted 5 lines

Acked-by Kevin Hilman <[email protected]>
This fixes the problem I reported here:
     http://marc.info/?l=linux-omap&m=134203768012711&w=2

OK thanks good to hear.

Regards,

Tony

Tony Lindgren

maintainer

2012-07-14 08:52 UTC #3 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 9 lines · Tony Lindgren

Quoted 8 lines · Kevin Hilman
Quoted 6 lines · 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.

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

Paul Walmsley

author maintainer

2012-07-14 17:54 UTC #4 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 5 lines · Tony Lindgren

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.

Quoted 7 lines

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

Tony Lindgren

maintainer

2012-07-16 08:36 UTC #5 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 7 lines · Paul Walmsley

Quoted 5 lines · Tony Lindgren
The patch seems to produce a new warning against arm soc tree next/cleanup branch:
warning: ?mpu_3xxx_clkdm? defined but not used

Quoted 7 lines

Quoted 2 lines
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

Paul Walmsley

author maintainer

2012-07-18 09:53 UTC #6 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 3 lines · Tony Lindgren

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).

Quoted 9 lines

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

Tony Lindgren

maintainer

2012-07-19 11:52 UTC #7 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 11 lines · Paul Walmsley

Hi
Quoted 3 lines · Tony Lindgren
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).

Quoted 21 lines

Quoted 9 lines
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.

Quoted 8 lines

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

Paul Walmsley

author maintainer

2012-07-19 19:12 UTC #8 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 3 lines · Tony Lindgren

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

Tony Lindgren

maintainer

2012-07-24 08:16 UTC #9 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 12 lines · Paul Walmsley

Quoted 3 lines · Tony Lindgren
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.

Quoted 7 lines

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

Paul Walmsley

author maintainer

2012-07-24 20:52 UTC #10 lore raw

PATCH 1/1 ARM: OMAP3: clockdomain: fix accidental duplicate registration

Quoted 2 lines · Tony Lindgren

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