Skip to contentSkip to conversation

u-boot: add config option to disable patching

PATCH New Markus Heidelberg 4 participants 13 messages Nov 14, 2008 – Jan 9, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 u-boot: add config option to disable patching

The patches in target/u-boot/ can conflict with custom board specific patches, so this option is necessary.

---
 target/u-boot/Config.in   |    8 ++++++++
 target/u-boot/Makefile.in |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)
target/u-boot/Config.in +8 -0
index 8159485..84b2ef5 100644
@@ -12,6 +12,14 @@ config BR2_TARGET_UBOOT_BOARDNAME
 	  One of U-Boot supported boards to be built.
 	  This will be suffixed with _config to meet U-Boot standard naming.
 
+config BR2_TARGET_UBOOT_DISABLE_GENERAL_PATCHES
+	bool "disable general patches"
+	depends on BR2_TARGET_UBOOT
+	help
+	  Select this option to disable patching U-Boot with the patches from
+	  target/u-boot/. This can be necessary if they conflict with your
+	  custom patch.
+
 config BR2_TARGET_UBOOT_CUSTOM_PATCH
 	string "custom patch"
 	depends on BR2_TARGET_UBOOT
target/u-boot/Makefile.in +2 -0
index 4df575a..0ea4b75 100644
@@ -46,9 +46,11 @@ $(U_BOOT_DIR)/.unpacked: $(DL_DIR)/$(U_BOOT_SOURCE)
 	touch $@
 
 $(U_BOOT_DIR)/.patched: $(U_BOOT_DIR)/.unpacked
+ifneq ($(strip $(BR2_TARGET_UBOOT_DISABLE_GENERAL_PATCHES)),y)
 	toolchain/patch-kernel.sh $(U_BOOT_DIR) target/u-boot/ \
 		u-boot-$(U_BOOT_VERSION)-\*.patch \
 		u-boot-$(U_BOOT_VERSION)-\*.patch.$(ARCH)
+endif
 ifneq ($(strip $(BR2_TARGET_UBOOT_CUSTOM_PATCH)),"")
 	@mkdir -p $(U_BOOT_PATCH_DIR)
 	cp -dpr $(BR2_TARGET_UBOOT_CUSTOM_PATCH) $(U_BOOT_PATCH_DIR)
Signature · 1 line
1.6.0.4.620.g87fa
#1 lore raw

PATCH 1/1 u-boot: add config option to disable patching

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/ can conflict with custom board specific Markus> patches, so this option is necessary.

But there aren't any patches under target/u-boot?

Signature · 1 line
Bye, Peter Korsgaard
#2 lore raw

PATCH 1/1 u-boot: add config option to disable patching

Peter Korsgaard, 14.11.2008:

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/ can conflict with custom board specific Markus> patches, so this option is necessary.

But there aren't any patches under target/u-boot?

I use Atmel's buildroot for AVR32, they added a patch there. Then do you think we should just delete the 3 patch-kernel.sh lines I've surrounded with the new option because it is never used?

Markus

#3 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

The patches in target/u-boot/$(U_BOOT_VERSION)/ can conflict with custom board specific patches, so this option is necessary.

---

Markus Heidelberg, 15.11.2008:

Quoted 7 lines

Peter Korsgaard, 14.11.2008:

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/ can conflict with custom board specific Markus> patches, so this option is necessary.

But there aren't any patches under target/u-boot?

Now there are.

Quoted 3 lines

I use Atmel's buildroot for AVR32, they added a patch there. Then do you think we should just delete the 3 patch-kernel.sh lines I've surrounded with the new option because it is never used?

 target/u-boot/Config.in   |    8 ++++++++
 target/u-boot/Makefile.in |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)
target/u-boot/Config.in +8 -0
index 9ab6224..f916852 100644
@@ -54,6 +54,14 @@ config BR2_U_BOOT_SITE
 	default "$(BR2_ATMEL_MIRROR)"		if BR2_TARGET_UBOOT_1_2_0_ATMEL
 	default "ftp://ftp.denx.de/pub/u-boot"
 
+config BR2_TARGET_UBOOT_DISABLE_GENERAL_PATCHES
+	bool "disable general patches"
+	depends on BR2_TARGET_UBOOT
+	help
+	  Select this option to disable applying the patches from
+	  target/u-boot/$(U_BOOT_VERSION)/. This can be necessary if they
+	  conflict with your custom patch.
+
 config BR2_TARGET_UBOOT_CUSTOM_PATCH
 	string "custom patch"
 	depends on BR2_TARGET_UBOOT
target/u-boot/Makefile.in +2 -0
index b5bb8e1..6319cea 100644
@@ -85,9 +85,11 @@ $(U_BOOT_DIR)/.unpacked: $(DL_DIR)/$(U_BOOT_SOURCE)
 	touch $@
 
 $(U_BOOT_DIR)/.patched: $(U_BOOT_DIR)/.unpacked
+ifneq ($(BR2_TARGET_UBOOT_DISABLE_GENERAL_PATCHES),y)
 	toolchain/patch-kernel.sh $(U_BOOT_DIR) target/u-boot/$(U_BOOT_VERSION) \
 		u-boot-$(U_BOOT_VERSION)-\*.patch \
 		u-boot-$(U_BOOT_VERSION)-\*.patch.$(ARCH)
+endif
 ifneq ($(strip $(BR2_TARGET_UBOOT_CUSTOM_PATCH)),"")
 	@mkdir -p $(U_BOOT_PATCH_DIR)
 	cp -dpr $(BR2_TARGET_UBOOT_CUSTOM_PATCH) $(U_BOOT_PATCH_DIR)
Signature · 1 line
1.6.1.35.g0c23
#4 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/$(U_BOOT_VERSION)/ can conflict with custom Markus> board specific patches, so this option is necessary.

I would rather get rid of any patches that are not strictly needed. We should only have patches for criticial bugfixes or build issues.

The number of configuration options we have in target/u-boot nowadays is getty silly.

Ulf, let me know what you think.

Signature · 1 line
Bye, Peter Korsgaard
#5 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Peter Korsgaard, 08.01.2009:

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/$(U_BOOT_VERSION)/ can conflict with custom Markus> board specific patches, so this option is necessary.

I would rather get rid of any patches that are not strictly needed. We should only have patches for criticial bugfixes or build issues.

It's not that only big/many/feature patches can lead to conflicts, it's rather possible that patches containing fixes conflict with custom patches, at least with my current workflow. I don't base U-Boot development on the state in Buildroot, but on U-Boot's git tree and then create a patch against the release and put it into Buildroot.

It's the same as with the Linux kernel: you wouldn't like patches lying around in Buildroot, which you can't disable and always get applied.

Quoted 2 lines

The number of configuration options we have in target/u-boot nowadays is getty silly.

That's right, but this option is not one to be passed to U-Boot like all this board specific stuff currently in Config.in.

Markus

#6 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Hi,

Markus> It's the same as with the Linux kernel: you wouldn't like Markus> patches lying around in Buildroot, which you can't disable Markus> and always get applied.

I've been pondering about adding a bit of flexibility to the packages that often need platform specific handling
(E.G. Linux/U-Boot). Building upstream versions of those rarely make sense for custom boards, so having the option to apply custom patches or grabbing the entire tree from other version control system seems sensible.

Grant Likely once sent a patch series adding support for using other git trees to the kernel build, that I still need to look into.

Quoted 2 lines
Quoted 2 lines

The number of configuration options we have in target/u-boot nowadays is getty silly.

Markus> That's right, but this option is not one to be passed to Markus> U-Boot like all this board specific stuff currently in Markus> Config.in.

True.

Signature · 1 line
Bye, Peter Korsgaard
#7 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Peter Korsgaard, 09.01.2009:

Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Hi,

Quoted 8 lines

Markus> It's the same as with the Linux kernel: you wouldn't like Markus> patches lying around in Buildroot, which you can't disable Markus> and always get applied.

I've been pondering about adding a bit of flexibility to the packages that often need platform specific handling
(E.G. Linux/U-Boot). Building upstream versions of those rarely make sense for custom boards, so having the option to apply custom patches

Which is available, but only in addition to the general patches lying around.

Quoted 2 lines

or grabbing the entire tree from other version control system seems sensible.

Yes, that would be a possible alternative.

Quoted 2 lines

Grant Likely once sent a patch series adding support for using other git trees to the kernel build, that I still need to look into.

I just had a look at it. Some sort of checking for newer commits and then updating the cloned tree would be nice.

Markus

#8 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

(E.G. Linux/U-Boot). Building upstream versions of those rarely make sense for custom boards, so having the option to apply custom patches

Markus> Which is available, but only in addition to the general patches lying Markus> around.

Exactly, it should ofcourse be INSTEAD.

Quoted 2 lines
Quoted 2 lines

Grant Likely once sent a patch series adding support for using other git trees to the kernel build, that I still need to look into.

Markus> I just had a look at it. Some sort of checking for newer commits and Markus> then updating the cloned tree would be nice.

Yes, indeed.

Signature · 1 line
Bye, Peter Korsgaard
#9 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Peter Korsgaard, 09.01.2009:

Quoted 9 lines
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

(E.G. Linux/U-Boot). Building upstream versions of those rarely make sense for custom boards, so having the option to apply custom patches

Markus> Which is available, but only in addition to the general patches lying Markus> around.

Exactly, it should ofcourse be INSTEAD.

Which was the reason for the config option in my patch.

Or do you think of the following, without an additional option? Maybe this would make sense!?

target/u-boot/Makefile.in +4 -3
@@ -85,13 +85,14 @@ $(U_BOOT_DIR)/.unpacked: $(DL_DIR)/$(U_BOOT_SOURCE)
        touch $@
 
 $(U_BOOT_DIR)/.patched: $(U_BOOT_DIR)/.unpacked
-       toolchain/patch-kernel.sh $(U_BOOT_DIR) target/u-boot/$(U_BOOT_VERSION) \
-               u-boot-$(U_BOOT_VERSION)-\*.patch \
-               u-boot-$(U_BOOT_VERSION)-\*.patch.$(ARCH)
 ifneq ($(strip $(BR2_TARGET_UBOOT_CUSTOM_PATCH)),"")
        @mkdir -p $(U_BOOT_PATCH_DIR)
        cp -dpr $(BR2_TARGET_UBOOT_CUSTOM_PATCH) $(U_BOOT_PATCH_DIR)
        toolchain/patch-kernel.sh $(U_BOOT_DIR) $(U_BOOT_PATCH_DIR) \*.patch
+else
+       toolchain/patch-kernel.sh $(U_BOOT_DIR) target/u-boot/$(U_BOOT_VERSION) \
+               u-boot-$(U_BOOT_VERSION)-\*.patch \
+               u-boot-$(U_BOOT_VERSION)-\*.patch.$(ARCH)
 endif
        touch $@
#10 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

In reply to Peter Korsgaard

tor 2009-01-08 klockan 15:33 +0100 skrev Peter Korsgaard:

Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Markus" == Markus Heidelberg <[email protected]> writes:

Markus> The patches in target/u-boot/$(U_BOOT_VERSION)/ can conflict with custom Markus> board specific patches, so this option is necessary.

Quoted 7 lines

I would rather get rid of any patches that are not strictly needed. We should only have patches for criticial bugfixes or build issues.

The number of configuration options we have in target/u-boot nowadays is getty silly.

Ulf, let me know what you think.

I can see a few alternatives.
1) Get rid of the patches, and introduce the possibility to select a prepatched version with these patches

2) Keep the patches and prune them once
they are in mainstream.
I am only interested in maintaing one set of patches for one version, so as I mentioned before,
I submitted about half a dozen patches the last two days and it looks like they can enter the main tree, with maybe a little rework.

3) Do it the same way I did the Advanced Linux support Keep target/u-boot, but allow patches
to be taken from the target/device/<whatever> In this case, the patches would go into target/device/Atmel/arch-at91/u-boot-2009.01-rc1

The last option is appealing.

BR
Ulf Samuelsson

#11 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

Quoted 8 lines · Ulf Samuelsson

3) Do it the same way I did the Advanced Linux support Keep target/u-boot, but allow patches
to be taken from the target/device/<whatever> In this case, the patches would go into target/device/Atmel/arch-at91/u-boot-2009.01-rc1

The last option is appealing.

I like that approach. (Except the versioning of course :=)

But not sure how to make this work without too much hassle.

Thomas.

#12 lore raw

PATCH 1/1 u-boot: add config option to disable general patches

In reply to Ulf Samuelsson
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Ulf" == Ulf Samuelsson <[email protected]> writes:

Hi,

Ulf> I can see a few alternatives.
Ulf> 1) Get rid of the patches, and introduce the possibility
Ulf> to select a prepatched version with these patches

I take it that's already there with your BR2_U_BOOT_SITE addition?

Ulf> 2) Keep the patches and prune them once

 Ulf>    they are in mainstream.
 Ulf>    I am only interested in maintaing one set of patches
 Ulf>    for one version, so as I mentioned before,
 Ulf>    I submitted about half a dozen patches the last two days
 Ulf>    and it looks like they can enter the main tree,
 Ulf>    with maybe a little rework.

We'll see how they are received, We'll need to get rid of them sooner or later atleast.

Ulf> 3) Do it the same way I did the Advanced Linux support

 Ulf>    Keep target/u-boot, but allow patches
 Ulf>    to be taken from the target/device/<whatever>
 Ulf>    In this case, the patches would go into 
 Ulf>    target/device/Atmel/arch-at91/u-boot-2009.01-rc1 

Ulf> The last option is appealing.

Yes, or more generally - Provide more flexibility for the U-Boot/Linux source, see my earlier mail.

Signature · 1 line
Bye, Peter Korsgaard