* [U-Boot] Breakage on arm/next
@ 2009-12-01 14:22 apgmoorthy
2009-12-01 14:39 ` Tom
2009-12-01 16:05 ` Alessandro Rubini
0 siblings, 2 replies; 14+ messages in thread
From: apgmoorthy @ 2009-12-01 14:22 UTC (permalink / raw)
To: u-boot
Hi Tom,
Amul is out-of-office for sometime.
Excuse us for the delay.
>>In rebasing arm/next against u-boot/next.
>>There is a general error with targets that use onenand.
>>This includes the targets
>>nk8815_onenand
>>omap3_evm
>>smdkc100
>>I believe the error is from
>>commit c758e947aa7d39a2be607ecdedd818ad300807b2
>>Author: Amul Kumar Saha <amul.saha@samsung.com>
>>Date: Wed Nov 4 10:38:46 2009 +0530
>> ENV Variable support for Flex-OneNAND
>> Define and use CONFIG_ENV_ADDR_FLEX and CONFIG_ENV_SIZE_FLEX
>> for storing environment variables.
The error comes since the Macros are defined in "configs/apollon.h".
And it works fine for Apollon.
I have tried with "make smdkc100_config" and did 'make all' and confirmed the error reported.
>>Please send a patch for this problem as soon as possible.
Moving these Macro definitions to "include/linux/mtd/onenand.h" looks more viable.
I can send across the patch. Please comment.
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-01 14:22 [U-Boot] Breakage on arm/next apgmoorthy
@ 2009-12-01 14:39 ` Tom
2009-12-17 15:53 ` Premi, Sanjeev
2009-12-01 16:05 ` Alessandro Rubini
1 sibling, 1 reply; 14+ messages in thread
From: Tom @ 2009-12-01 14:39 UTC (permalink / raw)
To: u-boot
apgmoorthy wrote:
> Hi Tom,
>
<snip>
>
> Moving these Macro definitions to "include/linux/mtd/onenand.h" looks more viable.
> I can send across the patch. Please comment.
>
Could the macros defined in apollo.h also be defined in the
other target board's config file ?
Thank you for looking into this
Tom
> With Regards
> Moorthy
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-01 14:22 [U-Boot] Breakage on arm/next apgmoorthy
2009-12-01 14:39 ` Tom
@ 2009-12-01 16:05 ` Alessandro Rubini
2009-12-02 12:13 ` apgmoorthy
1 sibling, 1 reply; 14+ messages in thread
From: Alessandro Rubini @ 2009-12-01 16:05 UTC (permalink / raw)
To: u-boot
> Could the macros defined in apollo.h also be defined in the
> other target board's config file ?
I don't think so (my board is one of the affected ones).
The macros are CONFIG_ENV_ADDR_FLEX and CONFIG_ENV_SIZE_FLEX . I just
don't have the flex device.
In the commit that introduced the problem, I see code like this:
env_addr = CONFIG_ENV_ADDR;
+ if (FLEXONENAND(this))
+ env_addr = CONFIG_ENV_ADDR_FLEX;
So why should CONFIG_ENV_ADDR_FLEX have a different name from CONFIG_ENV_ADDR ?
Same applies to CONFIG_ENV_SIZE_FLEX.
I think c758e947aa7d39a2be607ecdedd818ad300807b2 should be reverted
and done differently. If I got my reasoning right, the first hunk
should go and the next one:
instr.len = CONFIG_ENV_SIZE;
+ if (FLEXONENAND(this)) {
+ env_addr = CONFIG_ENV_ADDR_FLEX;
+ instr.len = CONFIG_ENV_SIZE_FLEX;
+ instr.len <<= onenand_mtd.eraseregions[0].numblocks == 1 ?
+ 1 : 0;
+ }
Shoul just become
+ if (FLEXONENAND(this))
+ instr.len <<= onenand_mtd.eraseregions[0].numblocks == 1 ? 1 : 0;
This has no adverse effect on other boards and handles the flex specifics,
withouth adding two unneeded macros.
/alessandro
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-01 16:05 ` Alessandro Rubini
@ 2009-12-02 12:13 ` apgmoorthy
2009-12-02 17:07 ` Scott Wood
0 siblings, 1 reply; 14+ messages in thread
From: apgmoorthy @ 2009-12-02 12:13 UTC (permalink / raw)
To: u-boot
Hi,
> -----Original Message-----
> From: rubini at gnudd.com [mailto:rubini at gnudd.com] On Behalf Of
> Alessandro Rubini
> Sent: Tuesday, December 01, 2009 9:36 PM
> To: Tom.Rix at windriver.com
> Cc: moorthy.apg at samsung.com; scottwood at freescale.com;
> u-boot at lists.denx.de; kyungmin.park at samsung.com
> Subject: Re: [U-Boot] Breakage on arm/next
>
> > Could the macros defined in apollo.h also be defined in the other
> > target board's config file ?
>
> I don't think so (my board is one of the affected ones).
>
> The macros are CONFIG_ENV_ADDR_FLEX and CONFIG_ENV_SIZE_FLEX
> . I just don't have the flex device.
I get u.
>
> In the commit that introduced the problem, I see code like this:
>
>
> env_addr = CONFIG_ENV_ADDR;
> + if (FLEXONENAND(this))
> + env_addr = CONFIG_ENV_ADDR_FLEX;
>
> So why should CONFIG_ENV_ADDR_FLEX have a different name from
> CONFIG_ENV_ADDR ?
> Same applies to CONFIG_ENV_SIZE_FLEX.
>
Erasesizes differ in Flex-OneNAND from OneNAND.
So Default Macros cannot be used.
> I think c758e947aa7d39a2be607ecdedd818ad300807b2 should be
> reverted and done differently. If I got my reasoning right,
> the first hunk should go and the next one:
>
> instr.len = CONFIG_ENV_SIZE;
> + if (FLEXONENAND(this)) {
> + env_addr = CONFIG_ENV_ADDR_FLEX;
> + instr.len = CONFIG_ENV_SIZE_FLEX;
> + instr.len <<=
> onenand_mtd.eraseregions[0].numblocks == 1 ?
> + 1 : 0;
> + }
>
>
> Shoul just become
>
> + if (FLEXONENAND(this))
> + instr.len <<=
> onenand_mtd.eraseregions[0].numblocks == 1
> + ? 1 : 0;
>
> This has no adverse effect on other boards and handles the
> flex specifics, withouth adding two unneeded macros.
>
The Erase Sizes of Flex-OneNAND are 256K for SLC and 512K for MLC regions.
And always 0th Block belongs to SLC region whereas, 1st Block can be of SLC or
MLC region in a Die.
So without these Macros Flex-OneNAND specifics cannot be handled.
I felt moving these macros "include/linux/mtd/onenand.h" will be ideal in this
scenario.
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-02 12:13 ` apgmoorthy
@ 2009-12-02 17:07 ` Scott Wood
2009-12-07 12:13 ` apgmoorthy
0 siblings, 1 reply; 14+ messages in thread
From: Scott Wood @ 2009-12-02 17:07 UTC (permalink / raw)
To: u-boot
apgmoorthy wrote:
> I felt moving these macros "include/linux/mtd/onenand.h" will be ideal in this
> scenario.
Are they going to be the same on all boards? We let the board determine
the environment location for other types of storage.
How about just using CONFIG_ENV_ADDR/CONFIG_ENV_SIZE? On boards that
must dynamically support multiple possibilities, define it as an
expression that returns the right thing.
-Scot
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-02 17:07 ` Scott Wood
@ 2009-12-07 12:13 ` apgmoorthy
2009-12-07 17:26 ` Scott Wood
0 siblings, 1 reply; 14+ messages in thread
From: apgmoorthy @ 2009-12-07 12:13 UTC (permalink / raw)
To: u-boot
Hi Scott,
> Are they going to be the same on all boards? We let the
> board determine the environment location for other types of storage.
>
OK
> How about just using CONFIG_ENV_ADDR/CONFIG_ENV_SIZE? On
> boards that must dynamically support multiple possibilities,
> define it as an expression that returns the right thing.
>
If the macros are not favoured to get consensus , let the code
use CONFIG_ENV_ADDR/CONFIG_ENV_SIZE and incase of Flex-OneNAND
increase it by one more fold.
something Like
Hunk 1:
env_addr = CONFIG_ENV_ADDR;
+ if (FLEXONENAND(this))
+ env_addr <<= 1;
Hunk 2:
+ if (FLEXONENAND(this)) {
+ env_addr <<= 1;
+ instr.len <<= onenand_mtd.eraseregions[0].numblocks == 1 ?
+ 2 : 1;
+ }
This should not break any other Board with OneNAND support. Please comment.
(Somehow I still feel Macros can be Cleaner way.)
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-07 12:13 ` apgmoorthy
@ 2009-12-07 17:26 ` Scott Wood
2009-12-10 4:15 ` apgmoorthy
0 siblings, 1 reply; 14+ messages in thread
From: Scott Wood @ 2009-12-07 17:26 UTC (permalink / raw)
To: u-boot
apgmoorthy wrote:
> Hi Scott,
>
>> Are they going to be the same on all boards? We let the
>> board determine the environment location for other types of storage.
>>
> OK
>
>> How about just using CONFIG_ENV_ADDR/CONFIG_ENV_SIZE? On
>> boards that must dynamically support multiple possibilities,
>> define it as an expression that returns the right thing.
>>
>
> If the macros are not favoured to get consensus , let the code
> use CONFIG_ENV_ADDR/CONFIG_ENV_SIZE and incase of Flex-OneNAND
> increase it by one more fold.
>
> something Like
>
> Hunk 1:
> env_addr = CONFIG_ENV_ADDR;
> + if (FLEXONENAND(this))
> + env_addr <<= 1;
>
> Hunk 2:
> + if (FLEXONENAND(this)) {
> + env_addr <<= 1;
> + instr.len <<= onenand_mtd.eraseregions[0].numblocks == 1 ?
> + 2 : 1;
> + }
>
> This should not break any other Board with OneNAND support. Please comment.
> (Somehow I still feel Macros can be Cleaner way.)
Why is the address automatically doubled on flex? I think this really
needs to be something board-specified.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-07 17:26 ` Scott Wood
@ 2009-12-10 4:15 ` apgmoorthy
2009-12-10 17:17 ` Scott Wood
0 siblings, 1 reply; 14+ messages in thread
From: apgmoorthy @ 2009-12-10 4:15 UTC (permalink / raw)
To: u-boot
Hi Scott,
> >
> > Hunk 1:
> > env_addr = CONFIG_ENV_ADDR;
> > + if (FLEXONENAND(this))
> > + env_addr <<= 1;
> >
> > Hunk 2:
> > + if (FLEXONENAND(this)) {
> > + env_addr <<= 1;
> > + instr.len <<=
> onenand_mtd.eraseregions[0].numblocks == 1 ?
> > + 2 : 1;
> > + }
> >
> > This should not break any other Board with OneNAND support.
> Please comment.
> > (Somehow I still feel Macros can be Cleaner way.)
>
> Why is the address automatically doubled on flex? I think
> this really needs to be something board-specified.
>
Please excuse me for the Delay.
Flex-OneNAND device's erasesize itself is double considered to OneNAND.
Like , In SLC region of Flex-OneNAND size is 256K and
in MLC region it is 512K. In case of OneNAND erasesize is 128K and
it is just SLC.
That's the reason size doubles for the Env in SLC and
for MLC increased by one more fold.
In reply to Alessandro's mail , I have given the same details.
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-10 4:15 ` apgmoorthy
@ 2009-12-10 17:17 ` Scott Wood
2009-12-21 11:00 ` apgmoorthy
0 siblings, 1 reply; 14+ messages in thread
From: Scott Wood @ 2009-12-10 17:17 UTC (permalink / raw)
To: u-boot
apgmoorthy wrote:
> Hi Scott,
>
>>> Hunk 1:
>>> env_addr = CONFIG_ENV_ADDR;
>>> + if (FLEXONENAND(this))
>>> + env_addr <<= 1;
>>>
>>> Hunk 2:
>>> + if (FLEXONENAND(this)) {
>>> + env_addr <<= 1;
>>> + instr.len <<=
>> onenand_mtd.eraseregions[0].numblocks == 1 ?
>>> + 2 : 1;
>>> + }
>>>
>>> This should not break any other Board with OneNAND support.
>> Please comment.
>>> (Somehow I still feel Macros can be Cleaner way.)
>> Why is the address automatically doubled on flex? I think
>> this really needs to be something board-specified.
>>
> Please excuse me for the Delay.
>
> Flex-OneNAND device's erasesize itself is double considered to OneNAND.
That doesn't mean that all data you're storing is double the size. A
board may want to keep the byte offset the same, and let the block
number change.
> Like , In SLC region of Flex-OneNAND size is 256K and
> in MLC region it is 512K. In case of OneNAND erasesize is 128K and
> it is just SLC.
Suppose I have a 256K U-Boot. I want CONFIG_ENV_ADDR to be 256K
regardless, which would be block 1 for flex SLC or block 2 for regular
OneNAND.
If I have a 512K U-Boot, then the byte offset would be the same for MLC
as well.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-01 14:39 ` Tom
@ 2009-12-17 15:53 ` Premi, Sanjeev
2009-12-18 11:01 ` apgmoorthy
0 siblings, 1 reply; 14+ messages in thread
From: Premi, Sanjeev @ 2009-12-17 15:53 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: u-boot-bounces at lists.denx.de
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Tom
> Sent: Tuesday, December 01, 2009 8:10 PM
> To: apgmoorthy
> Cc: 'Scott Wood'; u-boot at lists.denx.de; kyungmin.park at samsung.com
> Subject: Re: [U-Boot] Breakage on arm/next
>
> apgmoorthy wrote:
> > Hi Tom,
> >
>
> <snip>
>
> >
> > Moving these Macro definitions to
> "include/linux/mtd/onenand.h" looks more viable.
> > I can send across the patch. Please comment.
> >
>
> Could the macros defined in apollo.h also be defined in the
> other target board's config file ?
>
> Thank you for looking into this
> Tom
Is there any update on the fix/proposal?
I am trying to build for omap3_evm; but see the same problem.
My repo is currently at:
bb3bcfa : Merge branch 'next' of ../next
a200a7c : Update CHANGELOG; prepare Prepare v2009.11
Best regards,
Sanjeev
>
> > With Regards
> > Moorthy
> >
> >
> >
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-17 15:53 ` Premi, Sanjeev
@ 2009-12-18 11:01 ` apgmoorthy
0 siblings, 0 replies; 14+ messages in thread
From: apgmoorthy @ 2009-12-18 11:01 UTC (permalink / raw)
To: u-boot
Hi Sanjeev,
>
> Is there any update on the fix/proposal?
>
> I am trying to build for omap3_evm; but see the same problem.
> My repo is currently at:
> bb3bcfa : Merge branch 'next' of ../next
> a200a7c : Update CHANGELOG; prepare Prepare v2009.11
>
As Scott pointed out rightly, my previous suggestion missed
to see certain scenario.I will send the fix shortly.
Sorry for the Delay.
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-10 17:17 ` Scott Wood
@ 2009-12-21 11:00 ` apgmoorthy
2010-01-06 22:47 ` Scott Wood
0 siblings, 1 reply; 14+ messages in thread
From: apgmoorthy @ 2009-12-21 11:00 UTC (permalink / raw)
To: u-boot
Hi Scott,
> >>>
> >>> Hunk 2:
> >>> + if (FLEXONENAND(this)) {
> >>> + env_addr <<= 1;
> >>> + instr.len <<=
> >> onenand_mtd.eraseregions[0].numblocks == 1 ?
> >>> + 2 : 1;
> >>> + }
> >>>
> >>> This should not break any other Board with OneNAND support.
> >> Please comment.
> >>> (Somehow I still feel Macros can be Cleaner way.)
> >> Why is the address automatically doubled on flex? I think this
> >> really needs to be something board-specified.
> >>
> > Please excuse me for the Delay.
> >
> > Flex-OneNAND device's erasesize itself is double considered
> to OneNAND.
>
> That doesn't mean that all data you're storing is double the
> size. A board may want to keep the byte offset the same, and
> let the block number change.
>
> > Like , In SLC region of Flex-OneNAND size is 256K and in
> MLC region it
> > is 512K. In case of OneNAND erasesize is 128K and it is just SLC.
>
> Suppose I have a 256K U-Boot. I want CONFIG_ENV_ADDR to be
> 256K regardless, which would be block 1 for flex SLC or block
> 2 for regular OneNAND.
>
> If I have a 512K U-Boot, then the byte offset would be the
> same for MLC as well.
You are right , Sorry , I missed it. This is one of the scenarios which compelled us
to use seperate Flex-OneNAND Env Macros different from OneNANDs , when
suggesting a alternative somehow missed it.
Going back to the suggestion you have given couple of mails back in the same chain.
That there should be a macro which should return the ENV size for Flex.
In that case i just saw something like below should do.
Hunk 1:
env_addr += CONFIG_ENV_ADDR & (onenand_mtd.eraseregions[0].erasesize-1);
Hunk 2:
env_addr += CONFIG_ENV_ADDR & (onenand_mtd.eraseregions[0].erasesize-1);
instr.len = onenand_mtd.eraseregions[0].erasesize;
instr.len <<= onenand_mtd.eraseregions[0].numblocks == 1 ? 1 : 0;
Please comment.
With Regards
Moorthy
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
2009-12-21 11:00 ` apgmoorthy
@ 2010-01-06 22:47 ` Scott Wood
0 siblings, 0 replies; 14+ messages in thread
From: Scott Wood @ 2010-01-06 22:47 UTC (permalink / raw)
To: u-boot
On Mon, Dec 21, 2009 at 04:30:40PM +0530, apgmoorthy wrote:
> Hunk 1:
> env_addr += CONFIG_ENV_ADDR & (onenand_mtd.eraseregions[0].erasesize-1);
>
> Hunk 2:
> env_addr += CONFIG_ENV_ADDR & (onenand_mtd.eraseregions[0].erasesize-1);
I'd say it should be the board config file's responsibility to provide a
CONFIG_ENV_ADDR that is properly block-aligned, regardless of what sort of
flash it's using.
> instr.len = onenand_mtd.eraseregions[0].erasesize;
It's unlikely at these block sizes, but theoretically the environment could
span multiple erase blocks.
Again, the board config file should supply a suitable CONFIG_ENV_SIZE.
-Scott
^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] Breakage on arm/next
@ 2009-11-28 3:48 Tom
0 siblings, 0 replies; 14+ messages in thread
From: Tom @ 2009-11-28 3:48 UTC (permalink / raw)
To: u-boot
In rebasing arm/next against u-boot/next.
There is a general error with targets that use onenand.
This includes the targets
nk8815_onenand
omap3_evm
smdkc100
The error message is
env_onenand.c: In function 'env_relocate_spec':
env_onenand.c:70: error: 'CONFIG_ENV_ADDR_FLEX' undeclared (first use in this
function)
env_onenand.c:70: error: (Each undeclared identifier is reported only once
env_onenand.c:70: error: for each function it appears in.)
env_onenand.c: In function 'saveenv':
env_onenand.c:106: error: 'CONFIG_ENV_ADDR_FLEX' undeclared (first use in this
function)
env_onenand.c:107: error: 'CONFIG_ENV_SIZE_FLEX' undeclared (first use in this
function)
I believe the error is from
commit c758e947aa7d39a2be607ecdedd818ad300807b2
Author: Amul Kumar Saha <amul.saha@samsung.com>
Date: Wed Nov 4 10:38:46 2009 +0530
ENV Variable support for Flex-OneNAND
Define and use CONFIG_ENV_ADDR_FLEX and CONFIG_ENV_SIZE_FLEX
for storing environment variables.
Signed-off-by: Rohit Hagargundgi <h.rohit@samsung.com>
Signed-off-by: Amul Kumar Saha <amul.saha@samsung.com>
In general MAKEALL should be used to regression test your changes.
Please send a patch for this problem as soon as possible.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-01-06 22:47 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-01 14:22 [U-Boot] Breakage on arm/next apgmoorthy
2009-12-01 14:39 ` Tom
2009-12-17 15:53 ` Premi, Sanjeev
2009-12-18 11:01 ` apgmoorthy
2009-12-01 16:05 ` Alessandro Rubini
2009-12-02 12:13 ` apgmoorthy
2009-12-02 17:07 ` Scott Wood
2009-12-07 12:13 ` apgmoorthy
2009-12-07 17:26 ` Scott Wood
2009-12-10 4:15 ` apgmoorthy
2009-12-10 17:17 ` Scott Wood
2009-12-21 11:00 ` apgmoorthy
2010-01-06 22:47 ` Scott Wood
-- strict thread matches above, loose matches on Subject: below --
2009-11-28 3:48 Tom
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.