Skip to contentSkip to conversation

Fix SPL build for non-ARM targets

PATCH New Albert ARIBAUD 3 participants 8 messages Jan 8, 2013 – Jan 9, 2013

Subsystems (2)
Memory technology devices (MTD) NAND flash
lore
Share & tools
lore

Albert ARIBAUD

author
#0 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

Signed-off-by Albert ARIBAUD <[email protected]>
---
 drivers/mtd/nand/Makefile |    4 ++++
 1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +4 -0
index 2c3812c..c77c0c4 100644
@@ -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
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

Quoted 10 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>
---
 drivers/mtd/nand/Makefile |    4 ++++
 1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +0 -0
index 2c3812c..c77c0c4 100644

drivers/mtd/nand/Makefile COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o · line 87

Quoted 9 lines
drivers/mtd/nand/Makefile +4 -0
@@ -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!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130109/157fec9b/attachment.pgp>
#2 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

In reply to Albert ARIBAUD
Quoted 11 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>
---
 drivers/mtd/nand/Makefile |    4 ++++
 1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +0 -0
index 2c3812c..c77c0c4 100644

drivers/mtd/nand/Makefile COBJS-$(CONFIG_TEGRA_NAND) += tegra_nand.o · line 87

Quoted 9 lines
drivers/mtd/nand/Makefile +4 -0
@@ -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

#3 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

Quoted 15 lines · Scott Wood
Quoted 14 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>

---
drivers/mtd/nand/Makefile | 4 ++++

1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +0 -0
index 2c3812c..c77c0c4 100644
@@ -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

Quoted 10 lines
Quoted 6 lines
drivers/mtd/nand/Makefile +4 -0
+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.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130109/de2d49b0/attachment.pgp>
#4 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

Quoted 27 lines · Tom Rini
Quoted 15 lines · Scott Wood
Quoted 14 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>

---
drivers/mtd/nand/Makefile | 4 ++++

1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +0 -0
index 2c3812c..c77c0c4 100644
@@ -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

Quoted 10 lines
Quoted 6 lines
drivers/mtd/nand/Makefile +4 -0
+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?

Quoted 7 lines

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

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

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

#5 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

In reply to Tom Rini
Quoted 27 lines · Tom Rini
Quoted 15 lines · Scott Wood
Quoted 14 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>

---
drivers/mtd/nand/Makefile | 4 ++++

1 file changed, 4 insertions(+)
drivers/mtd/nand/Makefile +0 -0
index 2c3812c..c77c0c4 100644
@@ -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

Quoted 10 lines
Quoted 6 lines
drivers/mtd/nand/Makefile +4 -0
+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?

Quoted 7 lines

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

#6 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 30 lines · Scott Wood
Quoted 22 lines · Tom Rini
Quoted 13 lines · Scott Wood
Quoted 12 lines · Albert ARIBAUD
Signed-off-by Albert ARIBAUD <[email protected]>

--- drivers/mtd/nand/Makefile | 4 ++++ 1 file changed, 4
insertions(+)

(quoted diff) +0 -0

b/drivers/mtd/nand/Makefile index 2c3812c..c77c0c4 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile

(quoted diff) +0 -0
@@ -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

Quoted 7 lines
Quoted 2 lines

+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?

Quoted 6 lines

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.

Quoted 5 lines

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...

Quoted 3 lines

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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBAgAGBQJQ7e5pAAoJENk4IS6UOR1WQbYP+waymmRmbnoFI6j+1tbVwqAP M3ayJER63xo6kwp04cGlRJWyboqOS8IKoSJy3P6FDxyobCcC8SLmxYFcCuzoKxWD cwOLA5GxsY1cYY6JEBLY9Iy7DWh8P1YwMFvZSvSDdnh0NYJ/X4PCS4uO+JEkY4jf +kR01aylSshk11RpGzNB6T3rxgyBIyZPUsEzK1AUSJbV+R+2Opt7zhL1eUQyCRM0 18TySJEmmOBa0R0yMA510lRev0yhlCrw1WRYuXQB+F1cbNIF4G010fUO3W7QAxpe 1USrYdL0TFD65HfK/K08zGmLJO7DbOkOS7wbpVlQQTZKEul4mnyw4gkq/6n36Poz WDccGrAWEBYGARMcdNd/suNAjdpAFRpFFVKW88iKi1mZjRfT8Mm93CaWXY6TAi69 YxSmR91XlTuK5ZTEP4QZviIFvz2BBhuzsuglWnFLwCGGh/SARpKetkKAoFTX5n98 q0OBliai+KoadNy0kgkkx9iknZB5nZ7h5fKmqn035SZpOVbIhX/rfD4MSwBoWYFz ROEFofcFY6IMvrnriKcBcHBy2B97evZTY3rLA5g+9w+192xw3s3G9XniZ4SLve9Q yk1BPiZeu9v/IN2zCTD81f2hDQ+Ch7FNIZcbjv4yAVsiQnYvl1sRm8+wgyIT9kf6 AKXplnA0lkCk/vp35huN
=UTij
-----END PGP SIGNATURE-----

#7 lore raw

PATCH 1/1 Fix SPL build for non-ARM targets

Quoted 16 lines · Tom Rini

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 5 lines · Scott Wood

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...

Quoted 3 lines

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.

Quoted 6 lines

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