[U-Boot] [PATCH v4 0/3] mtd, ubi, ubifs: resync with Linux-3.14
Heiko Schocher
hs at denx.de
Wed Jun 18 09:55:28 CEST 2014
Hello Scott,
Am 17.06.2014 17:54, schrieb Scott Wood:
> On Tue, 2014-06-17 at 09:15 +0200, Heiko Schocher wrote:
>> - Following Code in drivers/mtd/nand/nand_base.c nand_do_write_ops()
>> adapted for U-Boot:
>>
>> +#ifndef __UBOOT__
>> /* Reject writes, which are not page aligned */
>> if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {
>> +else
>> + /* Reject writes, which are not page aligned */
>> + if (NOTALIGNED(to)) {
>> +endif
>
> This sort of ifdeffing is a bad idea -- if a larger number of lines are
> ifdeffed, to avoid context conflicts, subsequent merges can silently put
> updates into the non-U-Boot section that are needed in the U-Boot
> section.
Yes, in general I agree, this is a bad idea. But I introduced it
here to get all differences between U-Boot and Linux marked with
the "_UBOOT_" define, so a future sync with linux should be easier ...
> Plus, it impairs readability, and in this case breaks text-editor
> brace-matching.
Yes, could be fixed, see below.
> Plus plus, that line has been that way in Linux since 2006 -- why is it
> being touched at all? Did you merge in changes from the previous mtd
> sync, or did you just copy the new files over and then try to fix
> resulting problems? (answered below)
Yes. Above Code is a good example. In Linux this piece of code looks like:
-------------
static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
struct mtd_oob_ops *ops)
[...]
ops->retlen = 0;
if (!writelen)
return 0;
/* Reject writes, which are not page aligned */
if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {
pr_notice("%s: attempt to write non page aligned data\n",
__func__);
return -EINVAL;
}
column = to & (mtd->writesize - 1);
-------------
in current U-Boot:
-------------
static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
struct mtd_oob_ops *ops)
[...]
ops->retlen = 0;
if (!writelen)
return 0;
column = to & (mtd->writesize - 1);
-------------
Why is this "if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {"
missing in U-Boot? Do we allow "not aligned writes" in U-Boot?
If so, we should have documented this, and best place for it,
I think, would be with a comment in the source code...
so if we delete this linux code in U-Boot, mark it with the
"_UBOOT_" define... and differences between linux and U-Boot are
documented and visible ...
I added for U-Boot the surely good "NOTALIGNED(to)" check, but not
the "NOTALIGNED(ops->len)" check, as it prevents such currently
working commands: "nand write xxx yyy ${filesize}" ... Maybe this
is the reason for missing this piece of linux code in U-Boot?
maybe I should add here a comment ... proposal:
-------------
#ifndef __UBOOT__
/* Reject writes, which are not page aligned */
if (NOTALIGNED(to) || NOTALIGNED(ops->len)) {
pr_notice("%s: attempt to write non page aligned data\n",
__func__);
return -EINVAL;
}
#else
/*
* Reject writes, which are not page aligned, but
* in U-Boot we accept "not page aligned data len" writes
* so commands for example "nand write xxx yyy ${filesize}"
* are working.
*/
if (NOTALIGNED(to)) {
pr_notice("%s: attempt to write non page aligned data\n",
__func__);
return -EINVAL;
}
#endif
-------------
With this approach, the difference between linux and u-boot is
documented, and text-editor brace-matching is not broken ...
>> as the original linux code leads in not working use of the env
>> var "filesize". For example a "nand write 80000000 80000 ${filesize}"
>> would not work with it ...
>>
>> - add CONFIG_MTD_NAND_VERIFY_WRITE from U-Boot code
>> (only compile tested)
>>
>> - Documented the config defines in README
>>
>> - kmalloc now uses memalign for allocating memory. This prevented
>> crashes when tested ubi/ubifs on imx6 board.
>>
>> - To produce this patch I did three steps:
>> - copied the linux source files to U-Boot tree -> commit this
>> - adapt license text in each file -> commit this
>> - make the code again compile clean and working -> commit this
>
> OK, so you did just copy stuff over. This method will lead to U-Boot
Yes.
> changes being reverted every time. You should look in the U-Boot
Yes and No. If they are documented with the "define _UBOOT_", they will
pop up after the copy from linux operation and you hopefully can
decide, if the U-Boot specific changes are longer necessary or not.
Above Code is a good example for that. Linux has this code
since 2006, why has U-Boot not this code?
> history to see what Linux version corresponded to the last mtd sync, and
> generate a diff relative to that.
I think, we should have the linux code as the reference, and on top
of this, we should add our U-Boot changes ... Thats the reason
for the git tree [1].
There we have the "simple copy from linux patch" [2]. It overwrites
the U-Boot specific changes, yes, but only in an intermediate step
in this git tree ... (Maybe I should add in the commit message, that
this is a simple copy from linux sources)
Then a U-Boot specifc patch [3], which introduces again hopefully
all U-Boot specific changes... so *all* U-Boot specific changs are
documented. At last a seperate licencse file patch [4] added which
changes the license text.
My hope was, that future linux mtd syncs could look like:
a) copy new linux source files to u-boot tree [1] (make a new branch
go back to commit [2] ... and copy new files from linux)
(commit this, to have all linux changes since last sync
with linux documented)
b) apply [3] and fix problems
All U-Boot changes documented
c) apply [4] and fix problems
d) add newly added U-Boot specific patches
(mark them with _UBOOT_ define if they are not yet marked with it)
(maybe squash them to step b ?)
-> new git tree ready
e) squash all patches into one patch and send it to mainline ...
so the mainline patch has only new changes with all U-Boot
specific changes.
Thats sounds easy ... and you have all linux and all U-Boot
changes visible and documented ... I must admit, I did not
tried your proposal to look into the linux changes since
the last mtd sync ... but that sounds the more tricky way
to get this sync, as the history can be long ... and shouldn;t
be the result the same?
bye,
Heiko
[1] http://git.denx.de/?p=u-boot/u-boot-testing.git;a=shortlog;h=refs/heads/update-mtd%2Bubi-linux-v3.14
[2] http://git.denx.de/?p=u-boot/u-boot-testing.git;a=commitdiff;h=2df3d72656e131d99f5be1dd10673477dfa9c171
[3] http://git.denx.de/?p=u-boot/u-boot-testing.git;a=commitdiff;h=9e571d0b0d7831cf0eea679396b9e8e39c488e01
[4] http://git.denx.de/?p=u-boot/u-boot-testing.git;a=commitdiff;h=65497750bfef88421cf5a1565fe82054cdf48395
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
More information about the U-Boot
mailing list