[U-Boot] [PATCH] Fix SPL build for non-ARM targets

Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers + +COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o + endif # drivers endif # nand

On Tue, Jan 08, 2013 at 11:57:20PM +0100, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers
+COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
endif # drivers endif # nand
Applied to u-boot/master, thanks!

On 01/08/2013 04:57:20 PM, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers
+COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
endif # drivers endif # nand
So, it looks like this is repairing breakage that came in through a manual merge resolution. Should such merge resolutions not be posted to the list for review? Or was it posted and I missed it?
-Scott

On Wed, Jan 09, 2013 at 01:53:21PM -0600, Scott Wood wrote:
On 01/08/2013 04:57:20 PM, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers
+COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
endif # drivers endif # nand
So, it looks like this is repairing breakage that came in through a manual merge resolution. Should such merge resolutions not be posted to the list for review? Or was it posted and I missed it?
None of the above. That powerpc was broken twice (once by this, and once by the arm head.S changes) was missed in my build testing. We don't have spelled out rules (that I'm aware of) for manual merges other than asking that someone check that X still works (in this case, am335x NAND). It did, but I didn't read the merge myself was the problem.

On 01/09/2013 03:38:22 PM, Tom Rini wrote:
On Wed, Jan 09, 2013 at 01:53:21PM -0600, Scott Wood wrote:
On 01/08/2013 04:57:20 PM, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers
+COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
endif # drivers endif # nand
So, it looks like this is repairing breakage that came in through a manual merge resolution. Should such merge resolutions not be posted to the list for review? Or was it posted and I missed it?
None of the above. That powerpc was broken twice (once by this, and once by the arm head.S changes) was missed in my build testing. We don't have spelled out rules (that I'm aware of) for manual merges other than asking that someone check that X still works (in this case, am335x NAND). It did, but I didn't read the merge myself was the problem.
The NAND Makefile breakage came from commit 79f38777947ac7685e2cef8bd977f954ab198c0e, which is a manual merge by Albert. Why should manual merges be exempt from the rule that all changes get posted to the list? What if next time it's a functional breakage rather than a broken build?
I tried repeating the merge between 96764df and 9bd5c1a and the only conflict marker was this:
ifdef CONFIG_SPL_BUILD <<<<<<< HEAD ifdef CONFIG_SPL_NAND_SIMPLE COBJS-y += nand_spl_simple.o endif COBJS-$(CONFIG_SPL_NAND_AM33XX_BCH) += am335x_spl_bch.o ifdef CONFIG_SPL_NAND_LOAD COBJS-y += nand_spl_load.o ||||||| merged common ancestors ifdef CONFIG_SPL_NAND_SIMPLE COBJS-y += nand_spl_simple.o endif ifdef CONFIG_SPL_NAND_LOAD COBJS-y += nand_spl_load.o =======
ifdef CONFIG_SPL_NAND_DRIVERS NORMAL_DRIVERS=y
> 96764df
endif
The fsl_elbc_spl.o part was still there, so it wasn't the automatic part of the merge that removed it.
If this was simply due to a bad patch in the ARM tree, which specific patch was it?
-Scott

On 01/09/2013 03:38:22 PM, Tom Rini wrote:
On Wed, Jan 09, 2013 at 01:53:21PM -0600, Scott Wood wrote:
On 01/08/2013 04:57:20 PM, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net
drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers
+COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
endif # drivers endif # nand
So, it looks like this is repairing breakage that came in through a manual merge resolution. Should such merge resolutions not be posted to the list for review? Or was it posted and I missed it?
None of the above. That powerpc was broken twice (once by this, and once by the arm head.S changes) was missed in my build testing. We don't have spelled out rules (that I'm aware of) for manual merges other than asking that someone check that X still works (in this case, am335x NAND). It did, but I didn't read the merge myself was the problem.
BTW, the conflicting patch was 5846b11e8810f0ecc15e78b383b7709b9b785580 ("am33xx_spl_bch: simple SPL nand loader for AM33XX"). It's a NAND patch, in drivers/mtd/nand specifically. I don't see my ACK on it, and it came in through the ti tree.
If we were having custodians sign-off patches as they apply them, you could tell from a glance that a patch is missing either Acked-by or Signed-off-by from a relevant maintainer.
-Scott

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 01/09/2013 05:06 PM, Scott Wood wrote:
On 01/09/2013 03:38:22 PM, Tom Rini wrote:
On Wed, Jan 09, 2013 at 01:53:21PM -0600, Scott Wood wrote:
On 01/08/2013 04:57:20 PM, Albert ARIBAUD wrote:
Signed-off-by: Albert ARIBAUD albert.u.boot@aribaud.net --- drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -79,6 +79,10 @@ COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
+else # minimal SPL drivers + +COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o + endif # drivers endif # nand
So, it looks like this is repairing breakage that came in through a manual merge resolution. Should such merge resolutions not be posted to the list for review? Or was it posted and I missed it?
None of the above. That powerpc was broken twice (once by this, and once by the arm head.S changes) was missed in my build testing. We don't have spelled out rules (that I'm aware of) for manual merges other than asking that someone check that X still works (in this case, am335x NAND). It did, but I didn't read the merge myself was the problem.
BTW, the conflicting patch was 5846b11e8810f0ecc15e78b383b7709b9b785580 ("am33xx_spl_bch: simple SPL nand loader for AM33XX"). It's a NAND patch, in drivers/mtd/nand specifically. I don't see my ACK on it, and it came in through the ti tree.
Putting on my u-boot-ti hat...
If we were having custodians sign-off patches as they apply them, you could tell from a glance that a patch is missing either Acked-by or Signed-off-by from a relevant maintainer.
Yes, the series was posted Oct 30, and was minor updates to an existing SoC driver (omap_gpmc), some code for new related parts of the SoC (the ELM code, for offloading bch math) and a new SPL shim because there was no other way to get the read correct. I merged it on or around Dec 10 and figured that since you hadn't spoken up in the intervening time, you didn't see anything worth commenting on.
- -- Tom

On 01/09/2013 04:25:46 PM, Tom Rini wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 01/09/2013 05:06 PM, Scott Wood wrote:
BTW, the conflicting patch was 5846b11e8810f0ecc15e78b383b7709b9b785580 ("am33xx_spl_bch: simple SPL nand loader for AM33XX"). It's a NAND patch, in drivers/mtd/nand specifically. I don't see my ACK on it, and it came in through the ti tree.
Putting on my u-boot-ti hat...
If we were having custodians sign-off patches as they apply them, you could tell from a glance that a patch is missing either Acked-by or Signed-off-by from a relevant maintainer.
Yes, the series was posted Oct 30, and was minor updates to an existing SoC driver (omap_gpmc), some code for new related parts of the SoC (the ELM code, for offloading bch math) and a new SPL shim because there was no other way to get the read correct. I merged it on or around Dec 10 and figured that since you hadn't spoken up in the intervening time, you didn't see anything worth commenting on.
I get a lot of e-mail. Some of it gets missed. If I haven't responded to something that directly touches drivers/mtd/nand within a reasonable time frame, please remind me rather than assume acquiscence.
Regardless, it's the manual merge that definitely needed review.
-Scott
participants (3)
-
Albert ARIBAUD
-
Scott Wood
-
Tom Rini