| Message ID | 20130625184539.GB2718@n2100.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 25, 2013 at 07:45:39PM +0100, Russell King - ARM Linux wrote: > On Tue, Jun 25, 2013 at 12:30:08PM +0100, Jonathan Austin wrote: > > There's a patch making its way to mainline via Russell's tree > > (8d96250700: ARM: mm: Transparent huge page support for LPAE systems) > > that breaks the build of the amba-pl08x driver (drivers/dma/amba-pl08x.c) > > because the 'get_signal' macro from include/linux/signal.h is now in the > > driver's scope and it clobbers a (previously) valid function call. > > Well, here's the change to asm/pgtable.h in that patch: > > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 9bcd262..eaedce7 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -24,6 +24,9 @@ > #include <asm/memory.h> > #include <asm/pgtable-hwdef.h> > > + > +#include <asm/tlbflush.h> > + > #ifdef CONFIG_ARM_LPAE > #include <asm/pgtable-3level.h> > #else > > And the question is - if that's all that is going on in that file, why > is asm/tlbflush.h being added to it? What in _that_ file uses anything > from asm/tlbflush.h (nothing apparantly from what I can see)? > > So, I'm tempted to kill this change off unless someone can justify why > that addition happened - it looks completely inappropriate to me. Yup. This is fixed in slave-dma tree by a patch from mark by renaming it. This should not show in the -next tree -- ~Vinod
On Wed, Jun 26, 2013 at 10:18:42AM +0530, Vinod Koul wrote: > Yup. This is fixed in slave-dma tree by a patch from mark by renaming it. > > This should not show in the -next tree Except, that change probably is probably responsible for this new error: drivers/dma/amba-pl08x.c: In function 'pl08x_request_mux': drivers/dma/amba-pl08x.c:304:13: error: expected identifier before '(' token
On Tue, Jun 25, 2013 at 07:45:39PM +0100, Russell King - ARM Linux wrote: > On Tue, Jun 25, 2013 at 12:30:08PM +0100, Jonathan Austin wrote: > > There's a patch making its way to mainline via Russell's tree > > (8d96250700: ARM: mm: Transparent huge page support for LPAE systems) > > that breaks the build of the amba-pl08x driver (drivers/dma/amba-pl08x.c) > > because the 'get_signal' macro from include/linux/signal.h is now in the > > driver's scope and it clobbers a (previously) valid function call. > > Well, here's the change to asm/pgtable.h in that patch: > > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index 9bcd262..eaedce7 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -24,6 +24,9 @@ > #include <asm/memory.h> > #include <asm/pgtable-hwdef.h> > > + > +#include <asm/tlbflush.h> > + > #ifdef CONFIG_ARM_LPAE > #include <asm/pgtable-3level.h> > #else > > And the question is - if that's all that is going on in that file, why > is asm/tlbflush.h being added to it? What in _that_ file uses anything > from asm/tlbflush.h (nothing apparantly from what I can see)? > > So, I'm tempted to kill this change off unless someone can justify why > that addition happened - it looks completely inappropriate to me. > Hi Russell, I needed tlbflush.h for the definition of flush_pmd_entry, called by set_pmd_at in pgtable-3level.h. It was pulled into pgtable.h as it would also be needed for the equivalent set_pmd_at function for 2-levels of paging. Best,
On Wed, Jun 26, 2013 at 09:40:25AM +0100, Steve Capper wrote: > On Tue, Jun 25, 2013 at 07:45:39PM +0100, Russell King - ARM Linux wrote: > > On Tue, Jun 25, 2013 at 12:30:08PM +0100, Jonathan Austin wrote: > > > There's a patch making its way to mainline via Russell's tree > > > (8d96250700: ARM: mm: Transparent huge page support for LPAE systems) > > > that breaks the build of the amba-pl08x driver (drivers/dma/amba-pl08x.c) > > > because the 'get_signal' macro from include/linux/signal.h is now in the > > > driver's scope and it clobbers a (previously) valid function call. > > > > Well, here's the change to asm/pgtable.h in that patch: > > > > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > > index 9bcd262..eaedce7 100644 > > --- a/arch/arm/include/asm/pgtable.h > > +++ b/arch/arm/include/asm/pgtable.h > > @@ -24,6 +24,9 @@ > > #include <asm/memory.h> > > #include <asm/pgtable-hwdef.h> > > > > + > > +#include <asm/tlbflush.h> > > + > > #ifdef CONFIG_ARM_LPAE > > #include <asm/pgtable-3level.h> > > #else > > > > And the question is - if that's all that is going on in that file, why > > is asm/tlbflush.h being added to it? What in _that_ file uses anything > > from asm/tlbflush.h (nothing apparantly from what I can see)? > > > > So, I'm tempted to kill this change off unless someone can justify why > > that addition happened - it looks completely inappropriate to me. > > > > Hi Russell, > I needed tlbflush.h for the definition of flush_pmd_entry, called by set_pmd_at > in pgtable-3level.h. > > It was pulled into pgtable.h as it would also be needed for the equivalent > set_pmd_at function for 2-levels of paging. > I was a little quick sending this sorry.... I have tried converting set_pmd_at into a macro to allow for delayed expansion. (As I've noticed that other functions in pgtable-3level.h do this to call flush_pmd_entry.) Unfortunately, even with set_pmd_at defined as a macro, I get undefined references to flush_pmd_entry and clean_pmd_entry (called from pmdp_test_and_clear_young and pmdp_get_and_clear in asm-generic/pgtable.h). I'm having a look to see if there's anything else I can do. Apologies for causing this problem. Best,
On Wed, Jun 26, 2013 at 09:19:09AM +0100, Russell King - ARM Linux wrote: > On Wed, Jun 26, 2013 at 10:18:42AM +0530, Vinod Koul wrote: > > Yup. This is fixed in slave-dma tree by a patch from mark by renaming it. > > > > This should not show in the -next tree > > Except, that change probably is probably responsible for this new error: > > drivers/dma/amba-pl08x.c: In function 'pl08x_request_mux': > drivers/dma/amba-pl08x.c:304:13: error: expected identifier before '(' token This was pushed four days ago and havent seen any error report expect this. Anyway I rebuild my -next and looks fine for me CC drivers/dma/dmaengine.o CC drivers/dma/virt-dma.o CC drivers/dma/iovlock.o CC [M] drivers/dma/dmatest.o CC drivers/dma/amba-pl08x.o LD drivers/dma/built-in.o Can you share your config which failed for you? Mark, I believe you are testing for this, have you seen above error? -- ~Vinod
On Wed, Jun 26, 2013 at 04:09:30PM +0530, Vinod Koul wrote: > On Wed, Jun 26, 2013 at 09:19:09AM +0100, Russell King - ARM Linux wrote: > > Except, that change probably is probably responsible for this new error: > > drivers/dma/amba-pl08x.c: In function 'pl08x_request_mux': > > drivers/dma/amba-pl08x.c:304:13: error: expected identifier before '(' token > This was pushed four days ago and havent seen any error report expect this. > Anyway I rebuild my -next and looks fine for me > CC drivers/dma/dmaengine.o > CC drivers/dma/virt-dma.o > CC drivers/dma/iovlock.o > CC [M] drivers/dma/dmatest.o > CC drivers/dma/amba-pl08x.o > LD drivers/dma/built-in.o > Can you share your config which failed for you? > Mark, I believe you are testing for this, have you seen above error? No, that looks like the original error that was being fixed. The driver build tested OK for me at the time and currently in -next, though a tree which has the additional include but not the rename away from get_signal will obviously still have the issue. BTW for build coverage it'd be handy if AMBA could be enabled without having to be explicitly selected.
On Wed, Jun 26, 2013 at 04:09:30PM +0530, Vinod Koul wrote: > On Wed, Jun 26, 2013 at 09:19:09AM +0100, Russell King - ARM Linux wrote: > > On Wed, Jun 26, 2013 at 10:18:42AM +0530, Vinod Koul wrote: > > > Yup. This is fixed in slave-dma tree by a patch from mark by renaming it. > > > > > > This should not show in the -next tree > > > > Except, that change probably is probably responsible for this new error: > > > > drivers/dma/amba-pl08x.c: In function 'pl08x_request_mux': > > drivers/dma/amba-pl08x.c:304:13: error: expected identifier before '(' token > This was pushed four days ago and havent seen any error report expect this. > > Anyway I rebuild my -next and looks fine for me > > CC drivers/dma/dmaengine.o > CC drivers/dma/virt-dma.o > CC drivers/dma/iovlock.o > CC [M] drivers/dma/dmatest.o > CC drivers/dma/amba-pl08x.o > LD drivers/dma/built-in.o > > Can you share your config which failed for you? > > Mark, I believe you are testing for this, have you seen above error? Right, what's going on is that I have a commit in my tree which adds an include to asm/pgtable.h which introduces a #define for get_signal. This becomes visible to amba-pl08x.c. However, my build test tree fails *every* time because what I'm building is Linus' tree, my tree plus arm-soc, and it doesn't include your tree. This makes my build testing for the next merge window impossible. So, I need the fix to pl08x.c merged into my tree.
Hi, On Thu, Jun 27, 2013 at 2:46 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Jun 26, 2013 at 04:09:30PM +0530, Vinod Koul wrote: >> On Wed, Jun 26, 2013 at 09:19:09AM +0100, Russell King - ARM Linux wrote: >> > On Wed, Jun 26, 2013 at 10:18:42AM +0530, Vinod Koul wrote: >> > > Yup. This is fixed in slave-dma tree by a patch from mark by renaming it. >> > > >> > > This should not show in the -next tree >> > >> > Except, that change probably is probably responsible for this new error: >> > >> > drivers/dma/amba-pl08x.c: In function 'pl08x_request_mux': >> > drivers/dma/amba-pl08x.c:304:13: error: expected identifier before '(' token >> This was pushed four days ago and havent seen any error report expect this. >> >> Anyway I rebuild my -next and looks fine for me >> >> CC drivers/dma/dmaengine.o >> CC drivers/dma/virt-dma.o >> CC drivers/dma/iovlock.o >> CC [M] drivers/dma/dmatest.o >> CC drivers/dma/amba-pl08x.o >> LD drivers/dma/built-in.o >> >> Can you share your config which failed for you? >> >> Mark, I believe you are testing for this, have you seen above error? > > Right, what's going on is that I have a commit in my tree which adds an > include to asm/pgtable.h which introduces a #define for get_signal. > This becomes visible to amba-pl08x.c. > > However, my build test tree fails *every* time because what I'm building > is Linus' tree, my tree plus arm-soc, and it doesn't include your tree. > This makes my build testing for the next merge window impossible. > > So, I need the fix to pl08x.c merged into my tree. This has now hit the mainline kernel, and several defconfigs (nhk815, lpc32xx and the spear ones) are broken there. Vinod, when are you sending up your pull request with the fix? It'd be good to see it go in soon. -Olof
On Wed, Jul 03, 2013 at 11:27:12AM -0700, Olof Johansson wrote: > This has now hit the mainline kernel, and several defconfigs (nhk815, > lpc32xx and the spear ones) are broken there. > > Vinod, when are you sending up your pull request with the fix? It'd be > good to see it go in soon. It should be on its way by the weekend Thanks ~Vinod
On Fri, Jul 05, 2013 at 11:48:02AM +0530, Vinod Koul wrote: > On Wed, Jul 03, 2013 at 11:27:12AM -0700, Olof Johansson wrote: > > This has now hit the mainline kernel, and several defconfigs (nhk815, > > lpc32xx and the spear ones) are broken there. > > > > Vinod, when are you sending up your pull request with the fix? It'd be > > good to see it go in soon. > It should be on its way by the weekend It still isn't in the tree. Personally, I think that the fix should have come via my tree, because the breakage was caused my the changes in my tree - and for the last week or more I've not been able to build any of these ARM Ltd devel platforms because of this bug. It also means that I'm not able to build test a load of changes I'm currently working on against a recent kernel (or indeed any v3.10 kernel.) In other words, the lack of this patch being merged is stopping me from doing my job properly. (Consider from your perspective: if I were to break something you relied upon for a couple of weeks, how would you feel about it?) Please get this fix into mainline ASAP - even if you have to send it as a patch separately to Linus rather than putting it as part of your normal pull request. Thanks.
On Sun, Jul 07, 2013 at 01:32:40PM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 05, 2013 at 11:48:02AM +0530, Vinod Koul wrote: > > On Wed, Jul 03, 2013 at 11:27:12AM -0700, Olof Johansson wrote: > > > This has now hit the mainline kernel, and several defconfigs (nhk815, > > > lpc32xx and the spear ones) are broken there. > > > > > > Vinod, when are you sending up your pull request with the fix? It'd be > > > good to see it go in soon. > > It should be on its way by the weekend > > It still isn't in the tree. I sent this morning, depends on how soon can linus merge this > > Personally, I think that the fix should have come via my tree, because > the breakage was caused my the changes in my tree - and for the last week > or more I've not been able to build any of these ARM Ltd devel platforms > because of this bug. > > It also means that I'm not able to build test a load of changes I'm > currently working on against a recent kernel (or indeed any v3.10 kernel.) > In other words, the lack of this patch being merged is stopping me from > doing my job properly. (Consider from your perspective: if I were to > break something you relied upon for a couple of weeks, how would you > feel about it?) > > Please get this fix into mainline ASAP - even if you have to send it as a > patch separately to Linus rather than putting it as part of your normal > pull request. Yes it could have gone thur your tree as well, or you could have merged my tree to resolve your breakage -- Thanks ~Vinod
On Sun, Jul 07, 2013 at 07:04:53PM +0530, Vinod Koul wrote: > On Sun, Jul 07, 2013 at 01:32:40PM +0100, Russell King - ARM Linux wrote: > > On Fri, Jul 05, 2013 at 11:48:02AM +0530, Vinod Koul wrote: > > > On Wed, Jul 03, 2013 at 11:27:12AM -0700, Olof Johansson wrote: > > > > This has now hit the mainline kernel, and several defconfigs (nhk815, > > > > lpc32xx and the spear ones) are broken there. > > > > > > > > Vinod, when are you sending up your pull request with the fix? It'd be > > > > good to see it go in soon. > > > It should be on its way by the weekend > > > > It still isn't in the tree. > I sent this morning, depends on how soon can linus merge this Linus has merged this, so please merge the linus's tree and you should be happy :) ~Vinod
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h index 9bcd262..eaedce7 100644 --- a/arch/arm/include/asm/pgtable.h +++ b/arch/arm/include/asm/pgtable.h @@ -24,6 +24,9 @@ #include <asm/memory.h> #include <asm/pgtable-hwdef.h> + +#include <asm/tlbflush.h> + #ifdef CONFIG_ARM_LPAE #include <asm/pgtable-3level.h> #else