Re: [U-Boot] [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood

-----Original Message----- From: Prafulla Wadaskar Sent: Saturday, June 11, 2011 9:53 AM To: 'Holger Brunck'; u-boot@lists.denx.de Cc: Valentin Longchamp; Heiko Schocher Subject: RE: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
-----Original Message----- From: Holger Brunck [mailto:holger.brunck@keymile.com] Sent: Wednesday, June 08, 2011 5:13 PM To: u-boot@lists.denx.de Cc: Holger Brunck; Valentin Longchamp; Prafulla Wadaskar; Heiko
Schocher
Subject: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
suen3 and suen8 were in first HW version quite different, but now they are from a u-boot point of view similar. So these two boards can use the same header file. Other keymile boards differ only in the usage of the PCI interface. Therefore a target km_kirkwood_pci was introduced. All targets use the same header file.
Signed-off-by: Holger Brunck holger.brunck@keymile.com Signed-off-by: Valentin Longchamp valentin.longchamp@keymile.com cc: Prafulla Wadaskar prafulla@marvell.com cc: Heiko Schocher hs@denx.de
Changes for v2: - squashed together with 5/6 from previous serie, because the pci defines belongs logically to this patch - change typo in board maintainers name
MAINTAINERS | 6 ++- MAKEALL | 2 +- boards.cfg | 4 +- include/configs/km/km_arm.h | 1 - include/configs/{suen3.h => km_kirkwood.h} | 18 +++++++--- include/configs/mgcoge3un.h | 5 +++ include/configs/suen8.h | 50 -------------------
--
Applied to u-boot-marvell.git next branch Regards.. Prafulla . .
Hi Holger, Valentin
During this pull check I observed that u-boot.kwb image generation gives build errors for all Kirkwood based keymile boards.
Please kindly check on this and provide a fix.
Regards.. Prafulla . .

Hi Prafulla,
Sorry for the bad email formatting, using webmail from home. See my answer below.
-----Original Message----- From: Prafulla Wadaskar [mailto:prafulla@marvell.com] Sent: Sat 6/11/2011 6:50 AM To: Brunck, Holger; u-boot@lists.denx.de Cc: Longchamp, Valentin; Heiko Schocher Subject: RE: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
-----Original Message----- From: Prafulla Wadaskar Sent: Saturday, June 11, 2011 9:53 AM To: 'Holger Brunck'; u-boot@lists.denx.de Cc: Valentin Longchamp; Heiko Schocher Subject: RE: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
-----Original Message----- From: Holger Brunck [mailto:holger.brunck@keymile.com] Sent: Wednesday, June 08, 2011 5:13 PM To: u-boot@lists.denx.de Cc: Holger Brunck; Valentin Longchamp; Prafulla Wadaskar; Heiko
Schocher
Subject: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
suen3 and suen8 were in first HW version quite different, but now they are from a u-boot point of view similar. So these two boards can use the same header file. Other keymile boards differ only in the usage of the PCI interface. Therefore a target km_kirkwood_pci was introduced. All targets use the same header file.
Signed-off-by: Holger Brunck holger.brunck@keymile.com Signed-off-by: Valentin Longchamp valentin.longchamp@keymile.com cc: Prafulla Wadaskar prafulla@marvell.com cc: Heiko Schocher hs@denx.de
Changes for v2: - squashed together with 5/6 from previous serie, because the pci defines belongs logically to this patch - change typo in board maintainers name
MAINTAINERS | 6 ++- MAKEALL | 2 +- boards.cfg | 4 +- include/configs/km/km_arm.h | 1 - include/configs/{suen3.h => km_kirkwood.h} | 18 +++++++--- include/configs/mgcoge3un.h | 5 +++ include/configs/suen8.h | 50 -------------------
--
Applied to u-boot-marvell.git next branch Regards.. Prafulla . .
Hi Holger, Valentin
During this pull check I observed that u-boot.kwb image generation gives build errors for all Kirkwood based keymile boards.
Please kindly check on this and provide a fix.
I had noticed about this u-boot.kwb image generation earlier, but we don't use it internally yet. We would like to change this and use it (and thus fix this error).
Could you please answer my early question from this thread: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/99206
Here is my question again:
Now I have a question about the marvell boards build: I see in the Makefile that there is a rule about this kwb file, which is exactly what we do in our additionnal build script:
$(obj)u-boot.kwb: $(obj)u-boot.bin $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $
How do you use it (because I think it is not called by the default make command) ?
Best Regards
Valentin

Hi Valentin,
On Sun, Jun 12, 2011 at 02:09:24PM +0200, Longchamp, Valentin wrote:
Now I have a question about the marvell boards build: I see in the Makefile that there is a rule about this kwb file, which is exactly what we do in our additionnal build script:
$(obj)u-boot.kwb: $(obj)u-boot.bin $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $
How do you use it (because I think it is not called by the default make command) ?
For my part, I simply use:
make ${obj_path}/u-boot.kwb
Regards,
Simon

-----Original Message----- From: Longchamp, Valentin [mailto:Valentin.Longchamp@keymile.com] Sent: Sunday, June 12, 2011 5:39 PM To: Prafulla Wadaskar; Brunck, Holger; u-boot@lists.denx.de Cc: Heiko Schocher Subject: RE: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
Hi Prafulla,
Sorry for the bad email formatting, using webmail from home. See my answer below.
...snip...
Hi Holger, Valentin
During this pull check I observed that u-boot.kwb image generation gives build errors for all Kirkwood based keymile boards.
Please kindly check on this and provide a fix.
I had noticed about this u-boot.kwb image generation earlier, but we don't use it internally yet. We would like to change this and use it (and thus fix this error).
Could you please answer my early question from this thread: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/99206
Here is my question again:
Now I have a question about the marvell boards build: I see in the Makefile that there is a rule about this kwb file, which is exactly what we do in our additionnal build script:
You don't need to add additional build script for this. Just make sure in your config you have configured CONFIG_SYS_KWD_CONFIG properly. It also have default configurations.
$(obj)u-boot.kwb: $(obj)u-boot.bin $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $
How do you use it (because I think it is not called by the default make command) ?
Make u-boot.kwb CROSS_COMPILE=..., with this u-boot.kwb (i.e. Kirkwood boot image) target will be generated.
This is the standard way the other targets are build in u-boot.
That's it.
Regards.. Prafulla . .

Hi Valentin, Prafulla,
On 12/06/11 14:09, Longchamp, Valentin wrote:
During this pull check I observed that u-boot.kwb image generation gives build errors for all Kirkwood based keymile boards.
Please kindly check on this and provide a fix.
I had noticed about this u-boot.kwb image generation earlier, but we don't use it internally yet. We would like to change this and use it (and thus fix this error).
Could you please answer my early question from this thread: http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/99206
Here is my question again:
Now I have a question about the marvell boards build: I see in the Makefile that there is a rule about this kwb file, which is exactly what we do in our additionnal build script:
$(obj)u-boot.kwb: $(obj)u-boot.bin $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $
How do you use it (because I think it is not called by the default make command) ?
I simply execute "make -s km_kirkwood u-boot.kwb" and then I see the error too. Yes our problem was that we still use our litte make wrapper to build the kirkwood binary and therefore we didn't saw the error. A simple make km_kirkwood works fine.
The bug was introduced due to commit 010a958b (arm/km: remove CONFIG_SYS_KWD_CONFIG from keymile-common.h) there we try to use the already defined CONFIG_SYS_KWD_CONFIG in arch-kirkwood/config.h what is quite this was we should do, but we forgot to include arch-kirkwood/config.h... And because we only tested for build errors without u-boot.kwb it seems to be ok...
So we should add this include and remove some of the defines which are part of arch-kirkwood/config.h from our km_arm.h. But at one point we have to change arch-kirkwood/config.h. There is:
#ifdef CONFIG_CMD_I2C #ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI #define CONFIG_SYS_I2C_SLAVE 0x0
This wouldn't work for us because we have CONFIG_CMD_I2C enabled but we use SOFT_I2C and not HARD_I2C
I think it is correct if we change it to: #ifdef CONFIG_CMD_I2C +#ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI +#endif #define CONFIG_SYS_I2C_SLAVE 0x0
Prafulla if you don't object to this change I would send a patch on tuesday when I'm back at work to change this and to fix the build error.
Best regards Holger

-----Original Message----- From: Holger Brunck [mailto:holger.brunck@keymile.com] Sent: Monday, June 13, 2011 12:02 AM To: Longchamp, Valentin Cc: Prafulla Wadaskar; u-boot@lists.denx.de; Heiko Schocher Subject: Re: [PATCH v2 4/5] arm/km: replace suenx targets with km_kirkwood
Hi Valentin, Prafulla,
On 12/06/11 14:09, Longchamp, Valentin wrote:
During this pull check I observed that u-boot.kwb image generation
gives build errors for all Kirkwood based keymile boards.
Please kindly check on this and provide a fix.
I had noticed about this u-boot.kwb image generation earlier, but we
don't use it internally yet. We would like to change this and use it (and thus fix this error).
Could you please answer my early question from this thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/99206
Here is my question again:
Now I have a question about the marvell boards build: I see in the Makefile that there is a rule about this kwb file, which is exactly
what
we do in our additionnal build script:
$(obj)u-boot.kwb: $(obj)u-boot.bin $(obj)tools/mkimage -n $(CONFIG_SYS_KWD_CONFIG) -T kwbimage \ -a $(CONFIG_SYS_TEXT_BASE) -e $(CONFIG_SYS_TEXT_BASE) -d $< $
How do you use it (because I think it is not called by the default
make
command) ?
I simply execute "make -s km_kirkwood u-boot.kwb" and then I see the error too. Yes our problem was that we still use our litte make wrapper to build the kirkwood binary and therefore we didn't saw the error. A simple make km_kirkwood works fine.
The bug was introduced due to commit 010a958b (arm/km: remove CONFIG_SYS_KWD_CONFIG from keymile-common.h) there we try to use the already defined CONFIG_SYS_KWD_CONFIG in arch- kirkwood/config.h what is quite this was we should do, but we forgot to include arch- kirkwood/config.h... And because we only tested for build errors without u-boot.kwb it seems to be ok...
So we should add this include and remove some of the defines which are part of arch-kirkwood/config.h from our km_arm.h. But at one point we have to change arch- kirkwood/config.h. There is:
#ifdef CONFIG_CMD_I2C #ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI #define CONFIG_SYS_I2C_SLAVE 0x0
This wouldn't work for us because we have CONFIG_CMD_I2C enabled but we use SOFT_I2C and not HARD_I2C
I think it is correct if we change it to: #ifdef CONFIG_CMD_I2C +#ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI +#endif #define CONFIG_SYS_I2C_SLAVE 0x0
Prafulla if you don't object to this change I would send a patch on tuesday when I'm back at work to change this and to fix the build error.
Hi Holger Ack, Pls send the patch for the same. Also send a patch to resolve build error. May be those could be two standalone patches.
Regards.. Prafulla . .

Hi Prafulla,
On 06/13/2011 10:58 AM, Prafulla Wadaskar wrote:
So we should add this include and remove some of the defines which are part of arch-kirkwood/config.h from our km_arm.h. But at one point we have to change arch- kirkwood/config.h. There is:
#ifdef CONFIG_CMD_I2C #ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI #define CONFIG_SYS_I2C_SLAVE 0x0
This wouldn't work for us because we have CONFIG_CMD_I2C enabled but we use SOFT_I2C and not HARD_I2C
I think it is correct if we change it to: #ifdef CONFIG_CMD_I2C +#ifndef CONFIG_SOFT_I2C #define CONFIG_I2C_MVTWSI +#endif #define CONFIG_SYS_I2C_SLAVE 0x0
Prafulla if you don't object to this change I would send a patch on tuesday when I'm back at work to change this and to fix the build error.
Hi Holger Ack, Pls send the patch for the same. Also send a patch to resolve build error. May be those could be two standalone patches.
yes I prepare two patches. I fix this on top of current u-boot master and rebase the patch serie on top of it. Maybe you want to send this fixes upstream for inclusion in v2011.06...
Best regards Holger
participants (4)
-
Holger Brunck
-
Longchamp, Valentin
-
Prafulla Wadaskar
-
Simon Guinot