Message ID | 20120321003435.GB17427@w500.lan |
---|---|
State | Superseded |
Headers | show |
Dear Luka Perkov, > Hi Marek, > > On Tue, Mar 20, 2012 at 07:48:05AM +0100, Marek Vasut wrote: > > > > > > > +#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board > > > > > > > lowlevel_init > > > > > > > > */ > > > > > > > > > > Are you sure you want to skip lowlevel init? It'll break cache > > > > > > setup etc. I believe. > > > > > > > > > > I will retest and send v4 once I get your feedback on other items. > > > > > > > > Ok, what's the result? From IRC I take it you must define this ... > > > > why? > > > > > > It generates error when building without it: > > > > > > /home/luka/uboot/arch/arm/cpu/arm926ejs/start.S:393: undefined > > > reference to `lowlevel_init' arm-openwrt-linux-ld: BFD (GNU Binutils) > > > 2.22 assertion fail elf32-arm.c:13830 > > > > Define it empty in your arch/arm/cpu/..../kirkwood.c and be done with it > > ;-) > > Yes, this seems to fix it: > > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c index fba5e01..ec2026c 100644 > --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > @@ -33,6 +33,8 @@ > > #define BUFLEN 16 > > +inline void lowlevel_init(void) {} > + > void reset_cpu(unsigned long ignored) > { > struct kwcpu_registers *cpureg = > diff --git a/include/configs/ib62x0.h b/include/configs/ib62x0.h > index 1c4778d..9808a04 100644 > --- a/include/configs/ib62x0.h > +++ b/include/configs/ib62x0.h > @@ -43,7 +43,6 @@ > #define CONFIG_KIRKWOOD /* SOC Family Name */ > #define CONFIG_KW88F6281 /* SOC Name */ > #define CONFIG_MACH_NAS6210 /* Machine type */ > -#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init */ > > /* > * Other required minimal configurations > > I'll clean this up and resend after we commit this... Commit what? Looking forward to V3 btw :) > > > > All other kirkwood targets I looked at define > > > CONFIG_SKIP_LOWLEVEL_INIT, including the ones mentioned above; here > > > are their configs for comparison: > > > > > > include/configs/dreamplug.h > > > include/configs/sheevaplug.h > > > include/configs/dockstar.h > > > > Why do you need to skip it? Does it hang or something? > > See above. I guess compile error also for other boards. Still you're missing cpu_init_crit in start.S, which might cause trouble. Now that you defined lowlevel_init(), you can as well remove this define SKIP... right? > > > > This is my proposal - I'll resend v4 and it should be ok to commit > > > > > > without fixes for: > > > 1) IB62x0_OE_LOW and IB62x0_OE_HIGH > > > 2) CONFIG_SKIP_LOWLEVEL_INIT > > > 3) ifdef indentation > > > > > > Because fixing the 1) and 2) is more than adding support for this new > > > board, and if it was in the same patch I would need to separate it. > > > That is a different issue. > > > > You can wait for Prafulla with #1 and #2, also for #2 check my comment. > > But we have two bugs going on for granted here at least and they're not > > your boards fault. On the other hand, it'd be cool if you could fix them > > prior to adding your board ;-) > > I'll resend v4 now and work on patches for this stuff later. > > > > I'll put on my TODO list, and work on this after commit: > > > * replace tabs with spaces in boards.config > > > * look at IB62x0_OE_LOW and IB62x0_OE_HIGH issue > > > * look at CONFIG_SKIP_LOWLEVEL_INIT issue > > For this one we have a patch now :) > > Thank you Marek. Thank you for your good work so far :) > > Bye, > Luka Best regards, Marek Vasut
> -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: 21 March 2012 12:52 > To: Luka Perkov > Cc: u-boot@lists.denx.de; dreagle@doukki.net; Wolfgang Denk; Prafulla > Wadaskar > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > Dear Luka Perkov, > > > Hi Marek, > > ...snip... > > > > This is my proposal - I'll resend v4 and it should be ok to > commit > > > > > > > > without fixes for: > > > > 1) IB62x0_OE_LOW and IB62x0_OE_HIGH > > > > 2) CONFIG_SKIP_LOWLEVEL_INIT > > > > 3) ifdef indentation > > > > > > > > Because fixing the 1) and 2) is more than adding support for > this new > > > > board, and if it was in the same patch I would need to separate > it. > > > > That is a different issue. > > > > > > You can wait for Prafulla with #1 and #2, also for #2 check my > comment. > > > But we have two bugs going on for granted here at least and > they're not > > > your boards fault. On the other hand, it'd be cool if you could > fix them > > > prior to adding your board ;-) Hi Luka #1: Defining these values as 0xffffffff, indicates that all GPIOs are configured high by default. so this configuration solely depends upon your board requirement. #2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since lowlevel_init is not needed on Kirkwood platforms. (ref: doc/README.kwbimage) Regards.. Prafulla . . .
Dear Prafulla Wadaskar, > > -----Original Message----- > > From: Marek Vasut [mailto:marex@denx.de] > > Sent: 21 March 2012 12:52 > > To: Luka Perkov > > Cc: u-boot@lists.denx.de; dreagle@doukki.net; Wolfgang Denk; Prafulla > > Wadaskar > > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > > > Dear Luka Perkov, > > > > > Hi Marek, > > ...snip... > > > > > > This is my proposal - I'll resend v4 and it should be ok to > > > > commit > > > > > > > without fixes for: > > > > > 1) IB62x0_OE_LOW and IB62x0_OE_HIGH > > > > > 2) CONFIG_SKIP_LOWLEVEL_INIT > > > > > 3) ifdef indentation > > > > > > > > > > Because fixing the 1) and 2) is more than adding support for > > > > this new > > > > > > > board, and if it was in the same patch I would need to separate > > > > it. > > > > > > > That is a different issue. > > > > > > > > You can wait for Prafulla with #1 and #2, also for #2 check my > > > > comment. > > > > > > But we have two bugs going on for granted here at least and > > > > they're not > > > > > > your boards fault. On the other hand, it'd be cool if you could > > > > fix them > > > > > > prior to adding your board ;-) > > Hi Luka > > #1: Defining these values as 0xffffffff, indicates that all GPIOs are > configured high by default. so this configuration solely depends upon your > board requirement. > > #2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since > lowlevel_init is not needed on Kirkwood platforms. (ref: > doc/README.kwbimage) Prafulla, you're then missing the fiddling with CPSR bits, which might be quite necessary. > > Regards.. > Prafulla . . . Best regards, Marek Vasut
> -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: 21 March 2012 15:33 > To: Prafulla Wadaskar > Cc: Luka Perkov; u-boot@lists.denx.de; dreagle@doukki.net; Wolfgang > Denk > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > Dear Prafulla Wadaskar, ...snip... > > > > Hi Luka > > > > #1: Defining these values as 0xffffffff, indicates that all GPIOs > are > > configured high by default. so this configuration solely depends > upon your > > board requirement. > > > > #2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since > > lowlevel_init is not needed on Kirkwood platforms. (ref: > > doc/README.kwbimage) > > Prafulla, you're then missing the fiddling with CPSR bits, which might > be quite > necessary. Hi Marek. May be, may you please explain these bits? Or any pointers? Can't these be addressed in kwimage.cfg? Regards.. Prafulla . . .
Dear Prafulla Wadaskar, > > -----Original Message----- > > From: Marek Vasut [mailto:marex@denx.de] > > Sent: 21 March 2012 15:33 > > To: Prafulla Wadaskar > > Cc: Luka Perkov; u-boot@lists.denx.de; dreagle@doukki.net; Wolfgang > > Denk > > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > > > Dear Prafulla Wadaskar, > > ...snip... > > > > Hi Luka > > > > > > #1: Defining these values as 0xffffffff, indicates that all GPIOs > > > > are > > > > > configured high by default. so this configuration solely depends > > > > upon your > > > > > board requirement. > > > > > > #2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since > > > lowlevel_init is not needed on Kirkwood platforms. (ref: > > > doc/README.kwbimage) > > > > Prafulla, you're then missing the fiddling with CPSR bits, which might > > be quite > > necessary. > > Hi Marek. > May be, may you please explain these bits? Or any pointers? Can't these be > addressed in kwimage.cfg? I have no idea, I'm no kirkwood expert. And about these bits, check start.S, what it does with them. > > Regards.. > Prafulla . . . Best regards, Marek Vasut
> -----Original Message----- > From: Marek Vasut [mailto:marex@denx.de] > Sent: 21 March 2012 16:27 > To: Prafulla Wadaskar > Cc: Luka Perkov; u-boot@lists.denx.de; dreagle@doukki.net; Wolfgang > Denk > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > Dear Prafulla Wadaskar, > > > > -----Original Message----- > > > From: Marek Vasut [mailto:marex@denx.de] > > > Sent: 21 March 2012 15:33 > > > To: Prafulla Wadaskar > > > Cc: Luka Perkov; u-boot@lists.denx.de; dreagle@doukki.net; > Wolfgang > > > Denk > > > Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0 > > > > > > Dear Prafulla Wadaskar, > > > > ...snip... > > > > > > Hi Luka > > > > > > > > #1: Defining these values as 0xffffffff, indicates that all > GPIOs > > > > > > are > > > > > > > configured high by default. so this configuration solely depends > > > > > > upon your > > > > > > > board requirement. > > > > > > > > #2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT > since > > > > lowlevel_init is not needed on Kirkwood platforms. (ref: > > > > doc/README.kwbimage) > > > > > > Prafulla, you're then missing the fiddling with CPSR bits, which > might > > > be quite > > > necessary. > > > > Hi Marek. > > May be, may you please explain these bits? Or any pointers? Can't > these be > > addressed in kwimage.cfg? > > I have no idea, I'm no kirkwood expert. And about these bits, check > start.S, > what it does with them. Hi Marek, I have checked arc/arm/cpu/arm926ejs/start.S, and I didn't find any issue, lowlevel_init will be called from cpu_init_crit which should be okay, there is cache init related code in it, if that can be a problem then it should be kept out. And on Kirkwood, internal bootloader does the job of lowlevel_init prior to start uboot execution. Regards.. Prafulla . . .
diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c index fba5e01..ec2026c 100644 --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c @@ -33,6 +33,8 @@ #define BUFLEN 16 +inline void lowlevel_init(void) {} + void reset_cpu(unsigned long ignored) { struct kwcpu_registers *cpureg = diff --git a/include/configs/ib62x0.h b/include/configs/ib62x0.h index 1c4778d..9808a04 100644 --- a/include/configs/ib62x0.h +++ b/include/configs/ib62x0.h @@ -43,7 +43,6 @@ #define CONFIG_KIRKWOOD /* SOC Family Name */ #define CONFIG_KW88F6281 /* SOC Name */ #define CONFIG_MACH_NAS6210 /* Machine type */ -#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init */ /* * Other required minimal configurations