diff mbox

3.9-rc1 regression in arm dtb build

Message ID 20130328195456.GA22527@quad.lixom.net (mailing list archive)
State New, archived
Headers show

Commit Message

Olof Johansson March 28, 2013, 7:54 p.m. UTC
On Thu, Mar 28, 2013 at 08:33:23PM +0100, Pavel Machek wrote:
> On Thu 2013-03-28 08:49:20, Stephen Warren wrote:
> > On 03/28/2013 03:29 AM, Pavel Machek wrote:
> > ...
> > > There's change that makes it pretty much impossible to build dtbs
> > > reliably between different kernel versions, because it leaves old dtbs
> > > around, and they survive even make clean.
> > 
> > One question here: Presumably the dtbs don't survive "make clean" if you
> > run that right after building the dtbs, but only if you check out a new
> > kernel version before running clean? Or is that not the case?
> 
> That was the case I seen, yes.
> 
> Have old kernel running, dtbs building. Apply a patch. It starts
> behaving weird. You do make clean... but you still get old dtbs.
> 
> (It would be nice if make socfpga_cyclone5.dtb would magically remove
> all the dtbs from old location; make dtbs does that if I understand
> stuff correctly).


This takes care of the "make dtbs" case (or just "make" case), but not
when you build a specific target.




ARM: dts: remove stale dtb files from arch/arm/boot

This will result in stale dtb files being removed from arch/arm/boot, which
can cause confusion when someone goes from an older kernel version to a newer
one without doing make clean on the old version (or git clean -f -d -x).


Signed-off-by: Olof Johansson <olof@lixom.net>

index 84aa2ca..b1127b8 100644

Comments

Stephen Warren March 29, 2013, 5:05 p.m. UTC | #1
On 03/28/2013 01:54 PM, Olof Johansson wrote:
> On Thu, Mar 28, 2013 at 08:33:23PM +0100, Pavel Machek wrote:
>> On Thu 2013-03-28 08:49:20, Stephen Warren wrote:
>>> On 03/28/2013 03:29 AM, Pavel Machek wrote:
>>> ...
>>>> There's change that makes it pretty much impossible to build dtbs
>>>> reliably between different kernel versions, because it leaves old dtbs
>>>> around, and they survive even make clean.
>>>
>>> One question here: Presumably the dtbs don't survive "make clean" if you
>>> run that right after building the dtbs, but only if you check out a new
>>> kernel version before running clean? Or is that not the case?
>>
>> That was the case I seen, yes.
>>
>> Have old kernel running, dtbs building. Apply a patch. It starts
>> behaving weird. You do make clean... but you still get old dtbs.
>>
>> (It would be nice if make socfpga_cyclone5.dtb would magically remove
>> all the dtbs from old location; make dtbs does that if I understand
>> stuff correctly).
> 
> 
> This takes care of the "make dtbs" case (or just "make" case), but not
> when you build a specific target.

I guess to really solve this, we need the following changes:

In the all kernels:

For both arch/*/boot and arch/*/boot/dts: clean removes *.dtb

In kernels that build *.dtb in the new location:

make dtbs and make foo.dtb both rm ../*.dtb

In kernels that build *.dtb in the old location:

make dtbs and make foo.dtb both rm dts/*.dtb

Is that too much to retrofit into all the stable kernels?
Pavel Machek March 29, 2013, 6:47 p.m. UTC | #2
> >> Have old kernel running, dtbs building. Apply a patch. It starts
> >> behaving weird. You do make clean... but you still get old dtbs.
> >>
> >> (It would be nice if make socfpga_cyclone5.dtb would magically remove
> >> all the dtbs from old location; make dtbs does that if I understand
> >> stuff correctly).
> > 
> > 
> > This takes care of the "make dtbs" case (or just "make" case), but not
> > when you build a specific target.
> 
> I guess to really solve this, we need the following changes:
> 
> In the all kernels:
> 
> For both arch/*/boot and arch/*/boot/dts: clean removes *.dtb
...
> make dtbs and make foo.dtb both rm dts/*.dtb
> 
> Is that too much to retrofit into all the stable kernels?

Well... since we need stable, anyway... what about:

Revert to good old location, fix 3.8 via stable tree, add "clean
removes *.dtb" to new kernels, so that people are not confused?

Yes, that means we keep old location, but arguably dtbs belong there,
and it is compatible with powerpc.... and we will not have to do so
much stable updates.
								Pavel
Olof Johansson March 29, 2013, 7:41 p.m. UTC | #3
On Fri, Mar 29, 2013 at 10:05 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/28/2013 01:54 PM, Olof Johansson wrote:
>> On Thu, Mar 28, 2013 at 08:33:23PM +0100, Pavel Machek wrote:
>>> On Thu 2013-03-28 08:49:20, Stephen Warren wrote:
>>>> On 03/28/2013 03:29 AM, Pavel Machek wrote:
>>>> ...
>>>>> There's change that makes it pretty much impossible to build dtbs
>>>>> reliably between different kernel versions, because it leaves old dtbs
>>>>> around, and they survive even make clean.
>>>>
>>>> One question here: Presumably the dtbs don't survive "make clean" if you
>>>> run that right after building the dtbs, but only if you check out a new
>>>> kernel version before running clean? Or is that not the case?
>>>
>>> That was the case I seen, yes.
>>>
>>> Have old kernel running, dtbs building. Apply a patch. It starts
>>> behaving weird. You do make clean... but you still get old dtbs.
>>>
>>> (It would be nice if make socfpga_cyclone5.dtb would magically remove
>>> all the dtbs from old location; make dtbs does that if I understand
>>> stuff correctly).
>>
>>
>> This takes care of the "make dtbs" case (or just "make" case), but not
>> when you build a specific target.
>
> I guess to really solve this, we need the following changes:
>
> In the all kernels:
>
> For both arch/*/boot and arch/*/boot/dts: clean removes *.dtb
>
> In kernels that build *.dtb in the new location:
>
> make dtbs and make foo.dtb both rm ../*.dtb

I don't think this is useful. If you're building a specific dtb
target, there should be no expectation that other dtb files should
automatically be removed.

The original complaint was that make clean didn't remove the old dtb
files. That's been resolved with the provided patch.

I don't think it's worth the hassle of trying to remove ../foo.dtb.

> In kernels that build *.dtb in the old location:
>
> make dtbs and make foo.dtb both rm dts/*.dtb
>
> Is that too much to retrofit into all the stable kernels?

I also don't think this is worth the effort. If you're going to check
out old and new kernels in the same tree, you should probably get into
the habit of using git clean.

Perhaps we need a "dtb_install" target instead, so people stop copying
from the source directories.


-Olof
Pavel Machek March 29, 2013, 10:18 p.m. UTC | #4
Hi!

> >> This takes care of the "make dtbs" case (or just "make" case), but not
> >> when you build a specific target.
> >
> > I guess to really solve this, we need the following changes:
> >
> > In the all kernels:
> >
> > For both arch/*/boot and arch/*/boot/dts: clean removes *.dtb
> >
> > In kernels that build *.dtb in the new location:
> >
> > make dtbs and make foo.dtb both rm ../*.dtb
> 
> I don't think this is useful. If you're building a specific dtb
> target, there should be no expectation that other dtb files should
> automatically be removed.

So... how do the scripts find that script location changed?

You do make foo.dtb.

There's nice foo.dtb waiting in arch/arm/boot/, only that it is
outdated. You use it, and it does not work.

> The original complaint was that make clean didn't remove the old dtb
> files. That's been resolved with the provided patch.

No, original complaint was that it is very very easy to use stale
files. "make clean" not removing them is just a part of problem.

> > In kernels that build *.dtb in the old location:
> >
> > make dtbs and make foo.dtb both rm dts/*.dtb
> >
> > Is that too much to retrofit into all the stable kernels?
> 
> I also don't think this is worth the effort. If you're going to check
> out old and new kernels in the same tree, you should probably get into
> the habit of using git clean.

Since when? make is expected to handle build dependencies. If you are
doing git bisect, make clean is going to slow you down, escpecially at
the end of cycle.

> Perhaps we need a "dtb_install" target instead, so people stop copying
> from the source directories.

dtb_install would be nice. But it will not help with current
problem...
									Pavel
diff mbox

Patch

--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -115,4 +115,5 @@  i:
 	$(CONFIG_SHELL) $(srctree)/$(src)/install.sh $(KERNELRELEASE) \
 	$(obj)/Image System.map "$(INSTALL_PATH)"
 
+clean-files := *.dtb
 subdir-	    := bootp compressed dts