[U-Boot] [PATCH] i.MX6 nand bootupdate
Stefano Babic
sbabic at denx.de
Thu Jun 16 10:44:40 CEST 2016
Hi Sergey,
On 15/06/2016 21:42, Sergey Kubushyn wrote:
> On Wed, 15 Jun 2016, Stefano Babic wrote:
>
> Hi,
>
>> Hi Sergey,
>>
>>
>> On 14/06/2016 20:32, Sergey Kubushyn wrote:
>>> Here is the initial support for writing i.MX6 NAND U-Boot into NAND
>>> with all FCB and DBBT stuff as required.
>>>
>>
>> This is a very interesting feature missing in U-Boot. Up now we are
>> constrained to update the bootloader from user space with kobs-ng, and
>> this is really an improvement. Thanks for that.
>>
>> Anyway, as remarked by Peng, please rebase your patch on current tree,
>> else we cannot test it.
>
> I will when time permits, probably tomorrow. Now let's go over other
> stuff...
>
Right.
>>> Just build U-Boot for NAND as u-boot.imx, load it somehow in RAM and
>>> say "nand bootupdate [addr] [size]". It will create all necessary
>>> structures and write everything as needed.
>>
>> The whole phrase is misleading here. I guess this is the way you have to
>> put on your board, it is not a general case.
>
> The very (and only) purpose for this function is to put U-Boot with
> proper FCB/DBBT header on one's board so i.MX6 bootrom would be able to
> boot off of it.
Exactly - this is what it is supposed to do. Write this statement.
Building a u-boot.imx, loaing in RAM, etc, this is a specialized use
case. I could build SPL, downloading a u-boot.img via serial, and I get
the same result. Or use a JTAG debugger.
So what you are saying is a use case - correct, but it looks like it is
the rule. Drop it and write simply what the patch is doing, that is
adding FCB / DDBT.
> There is _NO_ general case.
Right, so do not try to explain - a reader can think _IS_ the rule.
> One can _NOT_ change the way
> i.MX6 (or whatever) boots up, it is in the ROM and it always reads FCB
> starting from sector 0 of NAND 0.
Right, but this has nothing to do how a user put the bootloader the
first time. It is enough you write that a new command is added, and the
syntax for the new command.
> BTW, if your particular NAND chip is
> not supported by that ROM you are out of luck -- it won't boot off of
> that chip.
We know that :-(
>
> Once again -- it is _NOT_ my board, it is how the i.MX6 boot code works.
Partially - I hope I have better explained.
>
>> I am expecting to have some parameters like in kobs-ng: I set the number
>> of copies with --search_exponent, how is it done here ?
>
> It makes no sense for limited purpose U-Boot command. You need at least
> 2 copies of FCB/DBBT.
Right.
> That kobs-ng puts FCB and corresponding DBBT in 2
> consecutive erase blocks (sectors) thus taking 2 sectors per FCB/DBBT
> instance. I put DBBT in the same sector with FCB (exactly like BareBox
> does) so I can squeeze 4 copies in the same 4 sectors kobs-ng takes for
> 2 copies.
It makes sense - thanks for clarification. Indeed, it will be nice if
you put exactly this sentences in a README file.
>
> Then, it is a _LIMITED_ use command so it makes no sense to do something
> with a long command line and its parsing code. If somebody needs more
> complex/sophisticated utility he can just put U-Boot with this simple
> command, boot into their favority OS and use a utility whatever complex
> or big he wants.
ok, fine with me - point is explained.
>
> That is what U-Boot is about. I do also miss the mkfs.ext4 or fsck or
> even just an option to delete or rename a file on ext4 but I do
> understand it is not all that easy and make workarounds to live with
> what I have.
...this is OT...
>
>>> No SPL required, just plain single stage U-Boot.
>>
>> Why ? This has nothing to do. We could write SPL into the NAND as first
>> bootloader, and SPL will load the u-boot.img unabhängig von FCB.
>
> "No SPL required" means exactly that :) It does _NOT_ mean "No SPL is
> possible" -- as a matter of fact neither that added command nor i.MX6
> bootrom even care what that binary payload is.
Exactly. IMHO the sentence can confuse and someone can understand that,
evenif SPL is not strictly forbidden, it is _better_ to habe just a
single stage. It is not, it has nothing to do.
As you have better explained now, it does not mean this. Drop the sentence.
> As long as FCB is valid
> and that binary has a proper header nobody cares what it is -- it is
> simply loaded and ran.
Exactly. This is what your patch is supposed to do. If someone uses it
for single stage u-boot, or with SPL, this is his decision. Please just
explain the new feature added by the patch, and let the rest outside.
>
>>> Supposed to write into
>>> first NAND partition, recommended size 4MiB.
>>
>> Where is this size coming ? This depends on the number of copies (can we
>> have configurable as for kobs-ng ?) and the size of U-Boot. If we write
>> SPL, we need less.
>
> It needs at least 4 sectors for 4 copies of FCB/DBBT
I guess the 4 copies are coming because RBL reads up to 4 copies, if no
FCB is found in the first sector. If we have just two copies, RBL stops
after the second one and we lose a redundancy feature.
Having 4 copies makes absolutely sense - I want just to understand it.
> (this is 2 copies
> in kbs-ng, nbare minimum) and at least 2 sectors for each copy of the
> actual binary if the latter fits in one sector (minimal guarantee we can
> write it if one of the 2 sectors is bad.)
ok, or more - I tend to say that this depends on the binary size, taking
into account the reserve.
My only remark is to write here a "fix" size, that can depend on the
target. We need a minimum number of sectors, and if NAND sector is 256KB
everything doubles. I just suggest to drop the "recommended size", and
replace that with number of sectors as you explained here. Then it is fine.
> In real life you need some
> more for NAND. Minimum/standard sector is 128K so you need at least 1MiB
> for a binary that fits in 128KiB (for bootrom you also need leading 1KiB
> padding and one trailing empty page so binary must fit in 125KiB.)
>
> If the SPL is that small, yes, one can save some space so SPL partition
> can be smaller.
Right, but my concern is just to avoid to confuse user with fix rules.
Anyone should check his project and understand how resources are used.
I will avoid to try to explain here all possible cases : let's remain
concentrate on what the patch provides, that means adding FCB / DBBT.
> However possible 3MiB saving on a NAND chip seem hardly
> worth a bother. I can make 2 commands instead of one, say ubootupdate
> and splupdate but that is hardly worth a bother.
No, why ? We have already explained that the RBL does not care about the
payload, if the image is correctly written.
> What if one wanted to
> put a non-SPL U-Boot on his NAND later once it's been partitioned for
> SPL (more on partition later, see below)?
I am not yet at the point - but IMHO any developer decides how the
bootloader looks for the board and if it is split into SPL and
u-boot.img or simply having a u-boot.imx. And if there should be that
case, it is responsibility of the board maintainer / single developer to
provide enough space for it. The tool should work in all cases.
>
>> Supposed to write into mtd0 works in most cases, yes. But it should be
>> better let it configurable as in kobs-ng.
>
> And the reason to do it is...? What are you going to do with a copy on
> mtd other than 0? Remember, it is not us who decide where to boot from,
> it is i.MX6 bootrom.
I am not sure, but I think that the order for devices in U-Boot can be
changed, meaning that nand0 could be the not booting device. I think I
had a project like that in the past.
Of course, hardware decides and boots always from the same nand. But the
order in software can be changed.
> It can NOT boot off of anything but nand0. And
> remember, that FCB page is not even readable with regular mtd read -- it
> will fail ECC...
We are talking about two different things. RBL always loads from the
same device. It is hardware, it is fixed.
But even if this is a rare case, we could have, in case of multiple nand
devices, a different order in U-Boot, depending which device is
initialized first.
>
> The only use case I could imagine would be 2 redundant NANDs with their
> CS lines swapped as needed by jumpers or whatever for recovery/dualboot
> purposes but you can use that very switching mechanism when writing
> U-Boot into that combination...
Understood what you mind, never heard about a project like this.
>
>> One reason is to show the user an easy path from kobs-ng to u-boot - and
>> of course, how to change from kobs-ng to this command should be
>> explained into the README.
>
> There is no path at all. And nothing can be easier than just issue one
> command -- there are no options to match/convert, no parameters, nothing
> at all. What path is needed?
ok - understood. Fine, we do not need any comparison.
>
> Furthermore once this command is in U-Boot 99.9999% of all users would
> not have to bother with it at all so they won't even know such a thing
> ever existed.
Who knows, users are very creative...:-D
>
>>> Creates 4 redundant copies
>>> of boot structures in first 4 sectors and 2 redundant copies of U-Boot
>>> in the remaining space.
>>>
>>> Tested on a Variscite VAR-SOM-SOLO based board with i.MX6DL SoC. Will
>>> probably post that board support files later, when config is cleaned up
>>> so people could have a working NAND boot DCD portion to be able actually
>>> use those braindead SOMs.
>>
>> ok - anyway, it looks like that Peng and me can test on other boards -
>> no worry about it.
>
> Sure.
>
>>> Signed-off-by: Sergey Kubushyn <ksi at koi8.net>
>>> ---
>>> diff --git a/arch/arm/include/asm/imx-common/mxs_nand.h
>>> b/arch/arm/include/asm/imx-common/mxs_nand.h
>>> new file mode 100644
>>> index 0000000..7826a9f
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/imx-common/mxs_nand.h
>>
>>> @@ -0,0 +1,37 @@
>>> +/*
>>> + * Copyright (C) 2015 PHYTEC Messtechnik GmbH,
>>> + * Author: Stefan Christ <s.christ at phytec.de>
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of
>>> + * the License, or (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>
>> Copyright is ok, but we have SPDX in U-Boot. Change the license header.
>
> That's an excerpt from original copyright. I can change it to SPDX, no
> biggie. Just wanted to include a verbatim copy of original copyright.
That's fine, thanks for that. Now we have seen where code is coming, and
according to the rule it is enough you restore the Copyright removing
the rest of the header.
>
>>
>>> +#ifndef __NAND_MXS_H
>>> +#define __NAND_MXS_H
>>> +
>>> +/*
>>> + * Functions are definied in drivers/mtd/nand/nand_mxs.c. They are
>>> used to
>>> + * calculate the ECC Strength, BadBlockMarkerByte and
>>> BadBlockMarkerStartBit
>>> + * which are placed into the FCB structure. The i.MX6 ROM needs these
>>> + * parameters to read the firmware from NAND.
>>> + *
>>> + * The parameters depends on the pagesize and oobsize of NAND chips and
>>> are
>>> + * different for each combination. To avoid placing hardcoded values in
>>> the bbu
>>> + * update handler code, the generic calculation from the driver code is
>>> used.
>>> + */
>>> +
>>> +uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>>> + uint32_t page_oob_size);
>>> +
>>> +uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd);
>>> +
>>> +uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd);
>>> +
>>> +#endif /* __NAND_MXS_H */
>>> diff --git a/cmd/nand.c b/cmd/nand.c
>>> index 583a18f..12e38e1 100644
>>> --- a/cmd/nand.c
>>> +++ b/cmd/nand.c
>>> @@ -38,6 +38,11 @@ int find_dev_and_part(const char *id, struct
>>> mtd_device **dev,
>>> u8 *part_num, struct part_info **part);
>>> #endif
>>>
>>> +#ifdef CONFIG_CMD_NAND_BOOTUPDATE
>>> +/* This comes from a separate file in drivers/mtd/nand */
>>> +int mxs_do_nand_bootupdate(ulong addr, size_t len);
>>> +#endif
>>> +
>>> static int nand_dump(struct mtd_info *mtd, ulong off, int only_oob,
>>> int repeat)
>>> {
>>> @@ -373,6 +378,9 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
>>> argc, char * const argv[])
>>> loff_t off, size, maxsize;
>>> char *cmd, *s;
>>> struct mtd_info *mtd;
>>> +#ifdef CONFIG_CMD_NAND_BOOTUPDATE
>>> + size_t cnt;
>>> +#endif
>>> #ifdef CONFIG_SYS_NAND_QUIET
>>> int quiet = CONFIG_SYS_NAND_QUIET;
>>> #else
>>> @@ -745,6 +753,48 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int
>>> argc, char * const argv[])
>>> }
>>> #endif
>>>
>>> +#ifdef CONFIG_CMD_NAND_BOOTUPDATE
>>> + if (strncmp(cmd, "bootupdate", 10) == 0) {
>>> +
>>> + if (argc < 3) {
>>> + /* All default values */
>>> + addr = getenv_ulong("fileaddr", 16, 0UL);
>>> + cnt = getenv_ulong("filesize", 16, 0UL);
>>> + }
>>> +
>>> + if (argc == 3) {
>>> + /* 'addr' only, file size from environment */
>>> + if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
>>> + addr = getenv_ulong("fileaddr", 16, 0UL);
>>> +
>>> + cnt = getenv_ulong("filesize", 16, 0UL);
>>> + }
>>> +
>>> + if (argc > 3) {
>>> + /* 'addr', 'size', and possibly more */
>>> + if ((addr = simple_strtoul(argv[2], NULL, 16)) == 0UL)
>>> + addr = getenv_ulong("fileaddr", 16, 0UL);
>>> +
>>> + if ((cnt = simple_strtoul(argv[3], NULL, 16)) == 0UL)
>>> + cnt = getenv_ulong("filesize", 16, 0UL);
>>> + }
>
>> Why do we stick with the environment ? Most U-Boot commands do not do
>> it. A user can always write a script as "bootupdate ${loadaddr}
>> ${filesize}" if he wants, the command itself should be unaware of it.
>
> That means you would have to _ALWAYS_ use the full form, with address
> and size.
On the other side, if a user do not add them and values are taken from
environment, the FCB / DBBT is maybe written where the user does not
want. That means it can corrupt the target.
If address / size are not taken from environment, the command simply
reports an error without doing anything.
> And the fact that address is always a hex but size shown after
> file load is in decimal while it is still hex for [almost] any command
> in U-Boot so it is very confusing and prone to user errors.
You do not understand. The issue is to avoid that code try to better
understand what happens in the developer's head, making assumptions that
could be wrong.
As I said, user can still use the variable, but he has to explicitely
set it. For people who wants your behavior, they add a script like:
setenv bootupd 'bootupdate ${loadaddr} ${filesize}'
and they use bootupd instead of bootupdate. The issue here is to avoid
to force in code a behavior. If you see in u-boot code, there are just a
couple of examples doing this, and most commands are unaware about the
environment, letting the user to decide what he wants.
>
> And what's wrong with using default environment variable that is
> _ALWAYS_ set after file load? It is not something a user can set or
> forget to in his environment, those are "system" variables.
Let user decide - see above.
>
>
>>> +
>>> +
>>> + if (addr == 0 || cnt == 0) {
>>> + puts("Invalid arguments to nand bootupdate!\n");
>>> + return 1;
>>> + }
>>> + + if (mxs_do_nand_bootupdate(addr, cnt)) {
>>
>>
>> Name is misleading. The function is not related to the NAND driver
>> (mxs_nand), but it is an implementation of the layout requested by NXP's
>> SOCs. A name related to BCB (Boot Control Block) as in manual is more
>> correct.
>>
>> So please changes all names from mxs_ in the code if they are not
>> strictly related to the driver, and let mxs_ name just for functions
>> exported by the driver.
>
> I can, no problems. But it is only useful for _SOME_ Freescale (the fact
> NXP bought them recently doesn't change a fact those are Freescale SOCs)
> with MXS NAND. It might happen this would apply to other devices in the
> future but for now it applies nothing but those SOCs with MXS NAND.
ok, as far as I know the FCB is used also by i.MX53. This is _NOT_ mxs,
and it uses a different driver, not the mxs. And we do not know the
future, that means with next generation of SOCs.
I repeat: the patch here has a well known purpose. It adds FCB / DBBT
block for SOCs supporting it. It is not important that the _CURRENT_ use
case is i.MX6. I tcan change. The tool uses some function from the NAND
driver, and if someone else in the future will add the future to a new
SOC, he has to provide the same set of functionalities in the NAND driver.
>
>>> + puts("NAND bootupdate failed!\n");
>>> + return 1;
>>> + }
>>> +
>>> + puts("NAND bootupdate successful\n");
>>> + return 0;
>>> + }
>>> +#endif
>>> +
>>> usage:
>>> return CMD_RET_USAGE;
>>> }
>>> @@ -766,6 +816,17 @@ static char nand_help_text[] =
>>> " 'addr', skipping bad blocks and dropping any pages at the
>>> end\n"
>>> " of eraseblocks that contain only 0xFF\n"
>>> #endif
>>> +#ifdef CONFIG_CMD_NAND_BOOTUPDATE
>>> + "nand bootupdate - [addr] [size]\n"
>>> + " write U-Boot into NAND in board/SoC specific manner creating
>>> all\n"
>>> + " required headers and other bits and pieces as required for
>>> the\n"
>>> + " system to be able to boot off of NAND. 'addr' is the
>>> address\n"
>>> + " where U-Boot image has been loaded at, 'size' is its size.\n"
>>> + " If any of 'addr'/'size' is missing it is taken from
>>> environment\n"
>>> + " for the last file loaded. U-Boot image must be of a proper
>>> type\n"
>>> + " for the target platform (only IMX image supported at the
>>> moment)\n"
>>> + " binary without U-Boot image headers (e.g. u-boot.imx file.)\n"
>>> +#endif
>>> "nand erase[.spread] [clean] off size - erase 'size' bytes "
>>> "from offset 'off'\n"
>>> " With '.spread', erase enough for given file size,
>>> otherwise,\n"
>>
>> A README file in doc/ is also useful.
>
> Sure. But not everything at once -- it is the first, initial submission
> that can be changed/ammended/betterized later on if needed. One has to
> start somewhere, right?
That is ok - I have written that a README is useful, and if you want to
add it, it is appreciated.
>
>>> diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
>>> index 837d397..36609e1 100644
>>> --- a/drivers/mtd/nand/Makefile
>>> +++ b/drivers/mtd/nand/Makefile
>>> @@ -66,6 +66,7 @@ obj-$(CONFIG_TEGRA_NAND) += tegra_nand.o
>>> obj-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
>>> obj-$(CONFIG_NAND_OMAP_ELM) += omap_elm.o
>>> obj-$(CONFIG_NAND_PLAT) += nand_plat.o
>>> +obj-$(CONFIG_CMD_NAND_BOOTUPDATE) += mxs_nand_bootupdate.o
>
>> It will be nice to set some dependencies between CONFIG_NAND_MXS and
>> CONFIG_CMD_NAND_BOOTUPDATE. CONFIG_CMD_NAND_BOOTUPDATE is not useful if
>> CONFIG_NAND_MXS is not set, right ?
>
> Yep. As well as e.g. UBI is not useful without RBTREE/LZO but you'll
> have to choose those yourself. And NAND_MXS also requires APHB_DMA and
> some other stuff. That, in turn, require some other stuff and it goes
> ad infinitum.
Sorry, my comment was not complete. I try to explain better.
You add with the patch a new command, but this will be set in the old
fashion way - without Kconfig.
If this is put in cmd/Kconfig, a dependency can be created. The was
chosen by U-Boot for the future is with Kconfig, that means new commands
must folow the new rule.
>
> BTW, the fact it is not useful without NAND_MXS is the reason why I
> used "mxs-" in source/functions file names. Do you still want me to
> change it?
Yes, I have explained before why. That just means that in Kconfig the
command has a "depend on NAND_MXS" now, in future who knows.
>
>>> else # minimal SPL drivers
>>>
>>> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
>>> index 7be1f86..afeec22 100644
>>> --- a/drivers/mtd/nand/mxs_nand.c
>>> +++ b/drivers/mtd/nand/mxs_nand.c
>>> @@ -26,6 +26,7 @@
>>> #include <asm/imx-common/regs-gpmi.h>
>>> #include <asm/arch/sys_proto.h>
>>> #include <asm/imx-common/dma.h>
>>> +#include <asm/imx-common/mxs_nand.h>
>>>
>>> #define MXS_NAND_DMA_DESCRIPTOR_COUNT 4
>>>
>>> @@ -145,7 +146,7 @@ static uint32_t mxs_nand_aux_status_offset(void)
>>> return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
>>> }
>>>
>>> -static inline uint32_t mxs_nand_get_ecc_strength(uint32_t
>>> page_data_size,
>>> +uint32_t mxs_nand_get_ecc_strength(uint32_t page_data_size,
>>> uint32_t page_oob_size)
>>> {
>>> int ecc_strength;
>>> @@ -221,14 +222,14 @@ static inline uint32_t
>>> mxs_nand_get_mark_offset(uint32_t page_data_size,
>>> return block_mark_bit_offset;
>>> }
>>>
>>> -static uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
>>> +uint32_t mxs_nand_mark_byte_offset(struct mtd_info *mtd)
>>> {
>>> uint32_t ecc_strength;
>>> ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
>>> mtd->oobsize);
>>> return mxs_nand_get_mark_offset(mtd->writesize, ecc_strength) >> 3;
>>> }
>>>
>>> -static uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
>>> +uint32_t mxs_nand_mark_bit_offset(struct mtd_info *mtd)
>>> {
>>> uint32_t ecc_strength;
>>> ecc_strength = mxs_nand_get_ecc_strength(mtd->writesize,
>>> mtd->oobsize);
>
>> Added Scott as MTD maintainer: it is clear to me why, but the driver
>> exports its functions via the struct nand_chip (see board_nand_init).
>>
>> Is it not better to extend the structure with the functions you want to
>> export ?
>
> It might be. I don't care how I access those functions but it is
> probably up to them to extend the structure...
>
> BTW it is _EXACTLY_ how it is done in barebox. They started with
> putting all that code into their source but then realized it is
> already there, just as a file scope in another file so they simply
> exported it to avoid code duplication.
>
> If MTD guys decided to add it to their structure I will happily take it
> from there. That would make it impossible to base it on a current master
> until such structure change is actually applied but there is no rush
> here; I can wait :)
Waiting from Scott's answer...
>
>>> diff --git a/drivers/mtd/nand/mxs_nand_bootupdate.c
>>> b/drivers/mtd/nand/mxs_nand_bootupdate.c
>>
>> I do not know if this file should be here or should be in imx-common. It
>> is relevant just for i.MX and not for other SOCs.
>
> I do not either :) I considered both places and the current one had been
> chosen with a coin toss :) If you want me to put it in imx-common I will
> do it, no problems. Just tell me to.
Agree. But there are also a competence area. If put in mtd/nand, Scott
has MTD maintainer has to take care of each patch regarding it, but this
feature is strictly related to some i.MXes. For this reason, I propose
to put it in imx-common.
>
>> In barebox it has a better name : imx-bbu-nand-fcb.c. It says much more
>> as mxs_nand_bootupdate.c.
>
> It it not. The "bbu" part stays for "BareBox Update" and we are not
> barebox, we are U-Boot. If you can come up with better file name please
> tell me what it is and I will rename it. I don't care what it is called.
ok, I express wrongly my thoughts. I mean: the file codes the FCB / DBBT
into the nand. Else a generic nand_boot_update, I suggest for
correctness something like imx-fcb-dbbt.c (or something in that
direction). I mean, just a way to understand that the code refers to FCB
/ DBBT. Choose then an appropriated name.
>
>>> new file mode 100644
>>> index 0000000..9442553
>>> --- /dev/null
>>> +++ b/drivers/mtd/nand/mxs_nand_bootupdate.c
>>
>>
>>> @@ -0,0 +1,556 @@
>>> +/*
>>> + * mxs_nand_bootupdate.c - write U-Boot to MXS NAND to make it bootable
>>> + *
>>> + * Copyright (c) 2016 Sergey Kubushyn <ksi at koi8.net>
>>> + *
>>> + * Most of the source shamelesly stolen from barebox.
>>> + *
>>> + * Here is the original copyright:
>>> + *
>>> + *=== Cut ===
>>> + * Copyright (C) 2014 Sascha Hauer, Pengutronix
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of
>>> + * the License, or (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation.
>>> + *=== Cut ===
>>> + */
>>
>> Let Copyright and add SPDX header.
>>
>> As you ported code from another Open Spurce project, it will be nice if
>> you write this into the commit message, writing also from which barebox
>> commit you took the code, exactly how we do when we port code from Linux.
>
> No problems, just wanted to keep a verbatim copy of original copyright...
That's fine - now it is clear, and you can switch to SPDX.
>
>>> +
>>> +
>>> +#include <common.h>
>>> +#include <linux/sizes.h>
>>> +#include <linux/mtd/mtd.h>
>>> +#include <linux/compat.h>
>>> +#include <command.h>
>>> +#include <console.h>
>>> +#include <malloc.h>
>>> +#include <asm/byteorder.h>
>>> +#include <jffs2/jffs2.h>
>>> +#include <nand.h>
>>> +#include <errno.h>
>>> +#include <asm/imx-common/mxs_nand.h>
>>> +#include <asm/imx-common/imximage.cfg>
>>> +
>>> +
>>> +#ifndef CONFIG_CMD_MTDPARTS
>>> +#error "CONFIG_CMD_MTDPARTS is not set, mtd partition support missing"
>>> +#endif
>>
>> I am missing because this is important. U-Boot works with addresses, and
>> not with MTDs. The whole nand utilities take an address as parameter.
>> MTD is optional, and can be used or not in nand erase/read/write.
>
> Sure but it makes life more difficult. The update needs to know what
> space it has to plan accordingly and calculate where to put things. That
> range is also used for DBBT scan -- it doesn't make sense to go beyond
> that partition (or region) when creating DBBT because it is of now other
> use, just for bootrom NAND boot.
It can be the opposite. We stated that 4 copies are written, that means
that the space is calculated and cannot be changed. The command can
output the total size used, and it is duty of the board developer to
adjust the size of MTDs if they are wrong.
This way means the user must allocate enough space without knowing
exactly how much is necessary.
>
> Then there is other reason for this. If one has NAND it would've been
> illogical to just put U-Boot there as Variscite guys do. The remaining
> space can host quite sizable FS on it. The only one currently supported
> is UBIFS and UBI requires a partition.
I do not get the point. Sure, it makes no sense to use raw NAND without
UBI. But u-boot is not in a UBI container.
It makes absolutely sense for safety to have _NO_ partition for u-boot.
The kernel does not find it, and the first MTD partition MTD0 starts
after u-boot. There is no *normal* way for user space to write into
u-boot space - even better as marking the partition as read-only.
> Once we have partition anyway it
> raises a question why not to use a partition instead of address range
> for U-Boot?
Because there are use cases without MTD partition for U-Boot.
>
>>> +
>>> +static const char *uboot_tgt = "nand0,0";
>>> +
>>> +/* partition handling routines */
>>> +int mtdparts_init(void);
>>> +int id_parse(const char *id, const char **ret_id, u8 *dev_type, u8
>>> *dev_num);
>>> +int find_dev_and_part(const char *id, struct mtd_device **dev,
>>> + u8 *part_num, struct part_info **part);
>>> +
>>> +struct dbbt_block {
>>> + uint32_t Checksum; /* reserved on i.MX6 */
>>> + uint32_t FingerPrint;
>>> + uint32_t Version;
>>> + uint32_t numberBB; /* reserved on i.MX6 */
>>> + uint32_t DBBTNumOfPages;
>>> +};
>>> +
>>> +struct fcb_block {
>>> + uint32_t Checksum; /* First fingerprint in first byte */
>>> + uint32_t FingerPrint; /* 2nd fingerprint at byte 4 */
>>> + uint32_t Version; /* 3rd fingerprint at byte 8 */
>>> + uint8_t DataSetup;
>>> + uint8_t DataHold;
>>> + uint8_t AddressSetup;
>>> + uint8_t DSAMPLE_TIME;
>>> + /* These are for application use only and not for ROM. */
>>> + uint8_t NandTimingState;
>>> + uint8_t REA;
>>> + uint8_t RLOH;
>>> + uint8_t RHOH;
>>> + uint32_t PageDataSize; /* 2048 for 2K pages, 4096 for 4K
>>> pages */
>>> + uint32_t TotalPageSize; /* 2112 for 2K pages, 4314 for 4K
>>> pages */
>>> + uint32_t SectorsPerBlock; /* Number of 2K sections per block */
>>> + uint32_t NumberOfNANDs; /* Total Number of NANDs - not used
>>> by ROM */
>>> + uint32_t TotalInternalDie; /* Number of separate chips in this
>>> NAND */
>>> + uint32_t CellType; /* MLC or SLC */
>>> + uint32_t EccBlockNEccType; /* Type of ECC, can be one of
>>> BCH-0-20 */
>>> + uint32_t EccBlock0Size; /* Number of bytes for Block0 -
>>> BCH */
>>> + uint32_t EccBlockNSize; /* Block size in bytes for all
>>> blocks other than Block0 - BCH */
>>> + uint32_t EccBlock0EccType; /* Ecc level for Block 0 - BCH */
>>> + uint32_t MetadataBytes; /* Metadata size - BCH */
>>> + uint32_t NumEccBlocksPerPage; /* Number of blocks per page for
>>> ROM use - BCH */
>>> + uint32_t EccBlockNEccLevelSDK; /* Type of ECC, can be one of
>>> BCH-0-20 */
>>> + uint32_t EccBlock0SizeSDK; /* Number of bytes for Block0 -
>>> BCH */
>>> + uint32_t EccBlockNSizeSDK; /* Block size in bytes for all blocks
>>> other than Block0 - BCH */
>>> + uint32_t EccBlock0EccLevelSDK; /* Ecc level for Block 0 - BCH */
>>> + uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for
>>> SDK use - BCH */
>>> + uint32_t MetadataBytesSDK; /* Metadata size - BCH */
>>> + uint32_t EraseThreshold; /* To set into BCH_MODE register */
>>> + uint32_t BootPatch; /* 0 for normal boot and 1 to load patch
>>> starting next to FCB */
>>> + uint32_t PatchSectors; /* Size of patch in sectors */
>>> + uint32_t Firmware1_startingPage;/* Firmware image starts on this
>>> sector */
>>> + uint32_t Firmware2_startingPage;/* Secondary FW Image starting
>>> Sector */
>>> + uint32_t PagesInFirmware1; /* Number of sectors in firmware
>>> image */
>>> + uint32_t PagesInFirmware2; /* Number of sector in secondary FW
>>> image */
>>> + uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt
>>> search area begins */
>>> + uint32_t BadBlockMarkerByte; /* Byte in page data that have
>>> manufacturer marked bad block marker, */
>>> + /* this will be swapped with metadata[0] to
>>> complete page data. */
>>> + uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8
>>> and 16 the bad block marker does not */
>>> + /* start at 0th bit of BadBlockMarkerByte. This
>>> field is used to get to */
>>> + /* the start bit of bad block marker byte with in
>>> BadBlockMarkerByte */
>>> + uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset
>>> for bad block marker on physical NAND page */
>>> + uint32_t BCHType;
>>> +
>>> + uint32_t TMTiming2_ReadLatency;
>>> + uint32_t TMTiming2_PreambleDelay;
>>> + uint32_t TMTiming2_CEDelay;
>>> + uint32_t TMTiming2_PostambleDelay;
>>> + uint32_t TMTiming2_CmdAddPause;
>>> + uint32_t TMTiming2_DataPause;
>>> + uint32_t TMSpeed;
>>> + uint32_t TMTiming1_BusyTimeout;
>>> +
>>> + uint32_t DISBBM; /* the flag to enable (1)/disable(0) bi swap */
>>> + uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of
>>> main area in spare area */
>>> +};
>>> +
>>> +struct fw_write_data {
>>> + int fw1_blk;
>>> + int fw2_blk;
>>> + int part_blks;
>>> + void *buf;
>>> + size_t len;
>>> +};
>>> +
>>> +#define BF_VAL(v, bf) (((v) & bf##_MASK) >> bf##_OFFSET)
>>> +#define GETBIT(v,n) (((v) >> (n)) & 0x1)
>>> +
>>> +
>>> +static uint8_t calculate_parity_13_8(uint8_t d)
>>> +{
>>> + uint8_t p = 0;
>>> +
>>> + p |= (GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 3) ^ GETBIT(d,
>>> 2)) << 0;
>>> + p |= (GETBIT(d, 7) ^ GETBIT(d, 5) ^ GETBIT(d, 4) ^ GETBIT(d, 2) ^
>>> GETBIT(d, 1)) << 1;
>>> + p |= (GETBIT(d, 7) ^ GETBIT(d, 6) ^ GETBIT(d, 5) ^ GETBIT(d, 1) ^
>>> GETBIT(d, 0)) << 2;
>>> + p |= (GETBIT(d, 7) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d,
>>> 0)) << 3;
>>> + p |= (GETBIT(d, 6) ^ GETBIT(d, 4) ^ GETBIT(d, 3) ^ GETBIT(d, 2) ^
>>> GETBIT(d, 1) ^ GETBIT(d, 0)) << 4;
>>> + return p;
>>> +}
>>> +
>>> +static void encode_hamming_13_8(void *_src, void *_ecc, size_t size)
>>> +{
>>> + int i;
>>> + uint8_t *src = _src;
>>> + uint8_t *ecc = _ecc;
>>> +
>>> + for (i = 0; i < size; i++)
>>> + ecc[i] = calculate_parity_13_8(src[i]);
>>> +}
>>> +
>>> +static uint32_t calc_chksum(void *buf, size_t size)
>>> +{
>>> + u32 chksum = 0;
>>> + u8 *bp = buf;
>>> + size_t i;
>>> +
>>> + for (i = 0; i < size; i++)
>>> + chksum += bp[i];
>>> +
>>> + return ~chksum;
>>> +}
>>
>> I understand that it is for debug only - but you have removed debug
>> functions from barebox (dump_fcb for example). Can they be useful here,
>> too ?
>
> They were useful for debugging, to show what we put into that FCB. Once
> we are sure it is working correctly there is nothing more to debug.
Fine if you do not want to add them back - anywyay, you know that
software is a life system. It is working now, but we do not know what
happens in future due to some other changes (in the MTD driver, maybe
?). Maybe it does not work anymore, and we need the debug back.
>
>>> +
>>> +
>>> +static ssize_t raw_write_page(struct mtd_info *mtd, void *buf, loff_t
>>> offset)
>>> +{
>>> + struct mtd_oob_ops ops;
>>> + ssize_t ret;
>>> +
>>> + ops.mode = MTD_OPS_RAW;
>>> + ops.ooboffs = 0;
>>> + ops.datbuf = buf;
>>> + ops.len = mtd->writesize;
>>> + ops.oobbuf = buf + mtd->writesize;
>>> + ops.ooblen = mtd->oobsize;
>>> + ret = mtd_write_oob(mtd, offset, &ops);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int fcb_create(struct fcb_block *fcb, struct mtd_info *mtd)
>>> +{
>>> + fcb->FingerPrint = 0x20424346;
>>> + fcb->Version = 0x01000000;
>>> + fcb->PageDataSize = mtd->writesize;
>>> + fcb->TotalPageSize = mtd->writesize + mtd->oobsize;
>>> + fcb->SectorsPerBlock = mtd->erasesize / mtd->writesize;
>>> +
>>> + /* Divide ECC strength by two and save the value into FCB
>>> structure. */
>>> + fcb->EccBlock0EccType =
>>> + mxs_nand_get_ecc_strength(mtd->writesize, mtd->oobsize) >> 1;
>>> +
>>> + fcb->EccBlockNEccType = fcb->EccBlock0EccType;
>>> +
>>> + /* Also hardcoded in kobs-ng */
>>> + fcb->EccBlock0Size = 0x00000200;
>>> + fcb->EccBlockNSize = 0x00000200;
>>> + fcb->DataSetup = 80;
>>> + fcb->DataHold = 60;
>>> + fcb->AddressSetup = 25;
>>> + fcb->DSAMPLE_TIME = 6;
>>> + fcb->MetadataBytes = 10;
>>> +
>>> + /* DBBT search area starts at second page on first block */
>>> + fcb->DBBTSearchAreaStartAddress = 1;
>>> +
>>> + fcb->BadBlockMarkerByte = mxs_nand_mark_byte_offset(mtd);
>>> + fcb->BadBlockMarkerStartBit = mxs_nand_mark_bit_offset(mtd);
>>> +
>>> + fcb->BBMarkerPhysicalOffset = mtd->writesize;
>>> +
>>> + fcb->NumEccBlocksPerPage = mtd->writesize / fcb->EccBlock0Size - 1;
>>> +
>>> + fcb->Checksum = calc_chksum((void *)fcb + 4, sizeof(*fcb) - 4);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Erase entire U-Boot partition */
>>> +static int mxs_nand_uboot_erase(struct mtd_info *mtd, struct part_info
>>> *part)
>>> +{
>>> + uint64_t offset = 0;
>>> + struct erase_info erase;
>>> + int len = part->size;
>>> + int ret;
>>> +
>>> + while (len > 0) {
>>> + pr_debug("erasing at 0x%08llx\n", offset);
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + pr_debug("erase skip block @ 0x%08llx\n", offset);
>>> + offset += mtd->erasesize;
>>> + continue;
>>> + }
>>> +
>>> + memset(&erase, 0, sizeof(erase));
>>> + erase.addr = offset;
>>> + erase.len = mtd->erasesize;
>>> +
>>> + ret = mtd_erase(mtd, &erase);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + offset += mtd->erasesize;
>>> + len -= mtd->erasesize;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Write the U-Boot proper (2 copies) to where it belongs. */
>>> +/* This is U-Boot binary image itself, no FCB/DBBT here yet. */
>>> +static int mxs_nand_uboot_write_fw(struct mtd_info *mtd, struct
>>> fw_write_data *fw)
>>> +{
>>> + uint64_t offset;
>>> + int ret;
>>> + int blk;
>>> + size_t dummy;
>>> + size_t bytes_left;
>>> + int chunk;
>>> + void *p;
>>> +
>>> + bytes_left = fw->len;
>>> + p = fw->buf;
>>> + blk = fw->fw1_blk;
>>> + offset = blk * mtd->erasesize;
>>> +
>>> + while (bytes_left > 0) {
>>> + chunk = min(bytes_left, mtd->erasesize);
>>> +
>>> + pr_debug("fw1: writing %p at 0x%08llx, left 0x%08x\n",
>>> + p, offset, bytes_left);
>>> +
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + pr_debug("fw1: write skip block @ 0x%08llx\n", offset);
>>> + offset += mtd->erasesize;
>>> + blk++;
>>> + continue;
>>> + }
>>> +
>>> + if (blk >= fw->fw2_blk)
>>> + return -ENOSPC;
>>> +
>>> + ret = mtd_write(mtd, offset, chunk, &dummy, p);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + offset += chunk;
>>> + bytes_left -= chunk;
>>> + p += chunk;
>>> + blk++;
>>> + }
>>> +
>>> + bytes_left = fw->len;
>>> + p = fw->buf;
>>> + blk = fw->fw2_blk;
>>> + offset = blk * mtd->erasesize;
>>> +
>>> + while (bytes_left > 0) {
>>> + chunk = min(bytes_left, mtd->erasesize);
>>> +
>>> + pr_debug("fw2: writing %p at 0x%08llx, left 0x%08x\n",
>>> + p, offset, bytes_left);
>>> +
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + pr_debug("fw2: write skip block @ 0x%08llx\n", offset);
>>> + offset += mtd->erasesize;
>>> + blk++;
>>> + continue;
>>> + }
>>> +
>>> + if (blk >= fw->part_blks)
>>> + return -ENOSPC;
>>> +
>>> + ret = mtd_write(mtd, offset, chunk, &dummy, p);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + offset += chunk;
>>> + bytes_left -= chunk;
>>> + p += chunk;
>>> + blk++;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int dbbt_data_create(struct mtd_info *mtd, void *buf, int
>>> num_blocks)
>>> +{
>>> + int n;
>>> + int n_bad_blocks = 0;
>>> + uint32_t *bb = buf + 0x8;
>>> + uint32_t *n_bad_blocksp = buf + 0x4;
>>> +
>>> + for (n = 0; n < num_blocks; n++) {
>>> + loff_t offset = n * mtd->erasesize;
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + n_bad_blocks++;
>>> + *bb = n;
>>> + bb++;
>>> + }
>>> + }
>>> +
>>> + *n_bad_blocksp = n_bad_blocks;
>>> +
>>> + return n_bad_blocks;
>>> +}
>>> +
>>> +/********************************************************************/
>>> +/* This is where it is all done. Takes pointer to a U-Boot image in */
>>> +/* RAM and image size, creates FCB/DBBT and writes everything where */
>>> +/* it belongs into NAND. Image must be an IMX image built for NAND. */
>>> +/********************************************************************/
>>
>> Multiline comment, please fix it.
>>
>>> +static int mxs_nand_uboot_update(const void *img, size_t len)
>
>> I would prefer you let the function names as in original code, so we
>> know where they are coming from (imx_bbu_nand_update here).
>
> As I already said "bbu" stands for "BareBox Update". Do we have to keep
> it in U-Boot?
>
> If you have a better idea for function mane please go ahead and tell
> what it is. I don't care what the name is; even xyz123456 would be OK.
See above
>
>>> +{
>>> + int i, ret;
>>> +
>>> + size_t dummy;
>>> + loff_t offset = 0;
>>> +
>>> + void *fcb_raw_page;
>>> + void *dbbt_page;
>>> + void *dbbt_data_page;
>>> + void *ecc;
>>> +
>>> + uint32_t num_blocks_fcb_dbbt;
>>> + uint32_t num_blocks_fw;
>>> +
>>> + struct mtd_info *mtd;
>>> + struct fcb_block *fcb;
>>> + struct dbbt_block *dbbt;
>>> +
>>> + struct mtd_device *dev;
>>> + struct part_info *part;
>>> + u8 pnum;
>>> +
>>> + struct fw_write_data fw;
>>> +
>>> + if ((mtdparts_init() == 0) &&
>>> + (find_dev_and_part(uboot_tgt, &dev, &pnum, &part) == 0)) {
>>> + if (dev->id->type != MTD_DEV_TYPE_NAND) {
>>> + puts("Not a NAND device\n");
>>> + return -ENODEV;
>>> + }
>>> + }
>>> +
>>> + nand_curr_device = dev->id->num;
>>> +
>>> +#ifdef CONFIG_SYS_NAND_SELECT_DEVICE
>>> + board_nand_select_device(nand_info[nand_curr_device].priv,
>>> nand_curr_device);
>>> +#endif
>>> +
>>> + /* Get a pointer to mtd_info for selected device */
>>> +
>>> + mtd = get_mtd_device_nm("nand0"); /* We always boot off of
>>> nand0 */
>
>> ...and if I have nand1 ?
>
> So keep it for yourself :) i.MX6 bootrom won't boot off of it so no
> reason to put a bootloader there.
>
>> As I said, U-Boot works with addresses
>
> And what?
>
>>> + + if (IS_ERR(mtd)) {
>>> + /* Should not happen */
>>> + puts("No nand0 device...\n");
>>> + return -ENODEV;
>>> + }
>>> + + put_mtd_device(mtd);
>>> +
>>> + /* Quick and dirty check if we have 2Mbytes of good blocks in
>>> nand0,0 */
>>> + /* Not sure if it is needed at all but won't hurt so here it
>>> is... */
>
>> Yes, it hurts.
>
> I can remove it, no problems.
>
>>> + + i = 0;
>>> + offset = 0; /* It is the first partition so it starts at
>>> block 0 */
>>> +
>>> + while (offset < part->size) {
>>> + if (!mtd_block_isbad(mtd, offset)) {
>>> + i += mtd->erasesize;
>>> + }
>>> + offset += mtd->erasesize;
>>> + }
>>> + + if (i < SZ_2M) {
>>> + puts("Partition too small for U-Boot!\n");
>>> + return -EINVAL;
>>> + }
>>
>> I try to understand your changes from barebox code - something are clear
>> due to the differences between the two bootloaders, some other ones not.
>>
>>> +
>>> + /* We will use 4 first blocks for FCB/DBBT copies. */
>>> + /* The rest of partition is split in half and used */
>>> + /* for two U-Boot copies. We don't care if those */
>>> + /* start on good or bad block - bad blocks will be */
>>> + /* skipped on write, ROM boot code will also skip */
>>> + /* bad blocks on bootup when loading U-Boot image. */
>
>> Multiline comment is wrong. I think the original comment in bareboc is
>> much more clear.
>
> OK, no problems, will steal that.
>
>>> +
>>> + fw.part_blks = part->size / mtd->erasesize;
>>> + num_blocks_fcb_dbbt = 4;
>>> + num_blocks_fw = (fw.part_blks - num_blocks_fcb_dbbt) / 2;
>>> + fw.fw1_blk = num_blocks_fcb_dbbt;
>>> + fw.fw2_blk = fw.fw1_blk + num_blocks_fw;
>>> +
>>> + /* OK, now create FCB structure for bootROM NAND boot */
>>> +
>>> + fcb_raw_page = kzalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL);
>>> +
>>> + fcb = fcb_raw_page + 12;
>>> + ecc = fcb_raw_page + 512 + 12;
>
>> ??
>
> !! :) It is how the FCB page is laid out and it is what makes it fail
> regular BCH ECC check. It is what i.MX6 bootrom expects.
>
> FCB page is read in the raw. And written too.
>
>>> +
>>> + dbbt_page = kzalloc(mtd->writesize, GFP_KERNEL);
>>> + dbbt_data_page = kzalloc(mtd->writesize, GFP_KERNEL);
>>> + dbbt = dbbt_page;
>>> +
>>> + /* Write one additional page to make the ROM happy. */
>>> + /* Maybe the PagesInFirmwarex fields are really the */
>>> + /* number of pages - 1. kobs-ng does the same. */
>>> + + fw.len = ALIGN(len + FLASH_OFFSET_STANDARD + mtd->writesize,
>>> mtd->writesize);
>>> + fw.buf = kzalloc(fw.len, GFP_KERNEL);
>>> + memcpy(fw.buf + FLASH_OFFSET_STANDARD, img, len);
>>> +
>>> + /* Erase entire partition */
>>> + ret = mxs_nand_uboot_erase(mtd, part);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* Now write 2 copies of the U-Boot proper to where they belong. */
>>> + /* Headers (FCB, DBBT) will be generated and written after that. */
>>> + ret = mxs_nand_uboot_write_fw(mtd, &fw);
>>> + if (ret < 0)
>>> + goto out;
>>> +
>>> + /* Create FCB, calculate ECC (we don't/can't use hardware ECC */
>>> + /* here so we do it ourselves and then write _RAW_ pages. */
>>> + + fcb->Firmware1_startingPage = fw.fw1_blk * mtd->erasesize /
>>> mtd->writesize;
>>> + fcb->Firmware2_startingPage = fw.fw2_blk * mtd->erasesize /
>>> mtd->writesize;
>>> + fcb->PagesInFirmware1 =
>>> + ALIGN(len + FLASH_OFFSET_STANDARD, mtd->writesize) /
>>> mtd->writesize;
>>> + fcb->PagesInFirmware2 = fcb->PagesInFirmware1;
>>> +
>>> + fcb_create(fcb, mtd);
>>> + encode_hamming_13_8(fcb, ecc, 512);
>>> +
>>> + /*
>>> + * Set the first and second byte of OOB data to 0xFF, not 0x00.
>>> These
>>> + * bytes are used as the Manufacturers Bad Block Marker (MBBM).
>>> Since
>>> + * the FCB is mostly written to the first page in a block, a
>>> scan for
>>> + * factory bad blocks will detect these blocks as bad, e.g. when
>>> + * function nand_scan_bbt() is executed to build a new bad block
>>> table.
>>> + * We will _NOT_ mark a bad block as good -- we skip the bad
>>> blocks.
>>> + */
>>> + memset(fcb_raw_page + mtd->writesize, 0xff, 2);
>>> +
>>> + /* Now create DBBT */
>>> + dbbt->Checksum = 0;
>>> + dbbt->FingerPrint = 0x54424244;
>>> + dbbt->Version = 0x01000000;
>>> +
>>> + if ((ret = dbbt_data_create(mtd, dbbt_data_page, fw.part_blks))
>>> < 0)
>>> + goto out;
>>> +
>>> + if (ret > 0)
>>> + dbbt->DBBTNumOfPages = 1;
>>> +
>>> + offset = 0;
>>> +
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + puts("Block 0 is bad, NAND unusable\n");
>>> + ret = -EIO;
>>> + goto out;
>>> + }
>>> +
>>> + /* Write FCB/DBBT to first 4 blocks. Skip bad blocks if any. */
>>> + /* Less than 4 copies will be written if there were BBs !!! */
>>> + for (i = 0; i < 4; i++) {
>>> +
>>> + if (mtd_block_isbad(mtd, offset)) {
>>> + pr_err("Block %d is bad, skipped\n", i);
>>> + continue;
>>> + }
>>> +
>>> +
>>> + ret = raw_write_page(mtd, fcb_raw_page, mtd->erasesize * i);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize,
>>> + mtd->writesize, &dummy, dbbt_page);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* DBBTNumOfPages == 0 if no bad blocks */
>>> + if (dbbt->DBBTNumOfPages > 0) {
>>> + ret = mtd_write(mtd, mtd->erasesize * i + mtd->writesize
>>> * 5,
>>> + mtd->writesize, &dummy, dbbt_data_page);
>>> + if (ret)
>>> + goto out;
>>> + }
>>> + }
>>> +
>>> +out:
>>> + kfree(dbbt_page);
>>> + kfree(dbbt_data_page);
>>> + kfree(fcb_raw_page);
>>> + kfree(fw.buf);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +
>>> +int mxs_do_nand_bootupdate(ulong addr, size_t len)
>>> +{
>>> + /* KSI: Unlock NAND first if it is locked... */
>>> +
>>> + return mxs_nand_uboot_update((const void *)addr, len);
>>> +}
>
> ---
> ******************************************************************
> * KSI at home KOI8 Net < > The impossible we do immediately. *
> * Las Vegas NV, USA < > Miracles require 24-hour notice. *
> ******************************************************************
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
More information about the U-Boot
mailing list