| Message ID | alpine.DEB.2.00.1207120448120.25585@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
* 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
* 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
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
* 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
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
* 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
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
* 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
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
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,
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(-)