[U-Boot] [PATCH 0/6] cmd: mtdparts: Add support for runtime generated defaults
Greetings,
this patch serie is a by-product of UBI SPL patchset, see http://lists.denx.de/pipermail/u-boot/2016-January/242962.html (and earlier) First three patches are just bugfixes. With fourth one, controversy starts, but here can be easily overcome by a config option for a boards wanting to use defaults by default :-) Core change is fifth patch, it is more likely a draft, documentation is missing, etc... As always, comments and suggestions even for completely different solution welcome.
ladis
A private buffer is used to read mtdparts variable from non-relocated environment. A pointer to that buffer is returned unconditionally, confusing later test for variable presence in the environment. Fix it by returning NULL when getenv_f fails.
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- cmd/mtdparts.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 44b2c3a..3a88a10 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -1720,11 +1720,13 @@ int mtdparts_init(void) * before the env is relocated, then we need to use our own stack * buffer. gd->env_buf will be too small. */ - if (gd->flags & GD_FLG_ENV_READY) { + if (gd->flags & GD_FLG_ENV_READY) parts = getenv("mtdparts"); - } else { - parts = tmp_parts; - getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN); + else { + if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1) + parts = tmp_parts; + else + parts = NULL; } current_partition = getenv("partition");
In case there is no mtdparts variable in relocated environment, NULL is assigned to p, which is later fed to strncpy. Also function parameter mtdparts is completely ignored, so use it in case mtdparts variable is not found in environment. This parameter is checked not to be NULL in caller.
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- cmd/mtdparts.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 3a88a10..995cb87 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -1524,7 +1524,7 @@ static int spread_partitions(void) */ static int parse_mtdparts(const char *const mtdparts) { - const char *p = mtdparts; + const char *p; struct mtd_device *dev; int err = 1; char tmp_parts[MTDPARTS_MAXLEN]; @@ -1538,20 +1538,25 @@ static int parse_mtdparts(const char *const mtdparts) }
/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */ - if (gd->flags & GD_FLG_ENV_READY) { + if (gd->flags & GD_FLG_ENV_READY) p = getenv("mtdparts"); - } else { - p = tmp_parts; - getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN); + else { + if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1) + p = tmp_parts; + else + p = NULL; }
+ if (!p) + p = mtdparts; + if (strncmp(p, "mtdparts=", 9) != 0) { printf("mtdparts variable doesn't start with 'mtdparts='\n"); return err; } p += 9;
- while (p && (*p != '\0')) { + while (*p != '\0') { err = 1; if ((device_parse(p, &p, &dev) != 0) || (!dev)) break; @@ -1569,12 +1574,10 @@ static int parse_mtdparts(const char *const mtdparts) list_add_tail(&dev->link, &devices); err = 0; } - if (err == 1) { + if (err == 1) device_delall(&devices); - return 1; - }
- return 0; + return err; }
/**
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- cmd/mtdparts.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 995cb87..7860ed9 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -1516,6 +1516,23 @@ static int spread_partitions(void) #endif /* CONFIG_CMD_MTDPARTS_SPREAD */
/** + * The mtdparts variable tends to be long. If we need to access it + * before the env is relocated, then we need to use our own stack + * buffer. gd->env_buf will be too small. + * + * @param buf temporary buffer pointer MTDPARTS_MAXLEN long + * @return mtdparts variable string, NULL if not found + */ +static const char *getenv_mtdparts(char *buf) +{ + if (gd->flags & GD_FLG_ENV_READY) + return getenv("mtdparts"); + if (getenv_f("mtdparts", buf, MTDPARTS_MAXLEN) != -1) + return buf; + return NULL; +} + +/** * Accept character string describing mtd partitions and call device_parse() * for each entry. Add created devices to the global devices list. * @@ -1538,15 +1555,7 @@ static int parse_mtdparts(const char *const mtdparts) }
/* re-read 'mtdparts' variable, mtd_devices_init may be updating env */ - if (gd->flags & GD_FLG_ENV_READY) - p = getenv("mtdparts"); - else { - if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1) - p = tmp_parts; - else - p = NULL; - } - + p = getenv_mtdparts(tmp_parts); if (!p) p = mtdparts;
@@ -1691,6 +1700,7 @@ static int parse_mtdids(const char *const ids) return 0; }
+ /** * Parse and initialize global mtdids mapping and create global * device/partition list. @@ -1718,19 +1728,7 @@ int mtdparts_init(void)
/* get variables */ ids = getenv("mtdids"); - /* - * The mtdparts variable tends to be long. If we need to access it - * before the env is relocated, then we need to use our own stack - * buffer. gd->env_buf will be too small. - */ - if (gd->flags & GD_FLG_ENV_READY) - parts = getenv("mtdparts"); - else { - if (getenv_f("mtdparts", tmp_parts, MTDPARTS_MAXLEN) != -1) - parts = tmp_parts; - else - parts = NULL; - } + parts = getenv_mtdparts(tmp_parts); current_partition = getenv("partition");
/* save it for later parsing, cannot rely on current partition pointer
Boards which are defining default mtdparts often need them early in boot process (to load environment from UBI volume, for example). This is currently solved by adding mtdparts and mtdids variable definitions also to default environment. With this change, default partitions are used by default unless explicitely deleted or redefined.
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- cmd/mtdparts.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 7860ed9..53074a1 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -142,6 +142,8 @@ static struct list_head devices; struct mtd_device *current_mtd_dev = NULL; u8 current_mtd_partnum = 0;
+u8 use_defaults; + static struct part_info* mtd_part_info(struct mtd_device *dev, unsigned int part_num);
/* command line only routines */ @@ -1723,6 +1725,7 @@ int mtdparts_init(void) memset(last_ids, 0, MTDIDS_MAXLEN); memset(last_parts, 0, MTDPARTS_MAXLEN); memset(last_partition, 0, PARTITION_MAXLEN); + use_defaults = 1; initialized = 1; }
@@ -1761,10 +1764,16 @@ int mtdparts_init(void) return 1; }
- /* do no try to use defaults when mtdparts variable is not defined, - * just check the length */ - if (!parts) - printf("mtdparts variable not set, see 'help mtdparts'\n"); + /* use defaults when mtdparts variable is not defined + * once mtdparts is saved environment, drop use_defaults flag */ + if (!parts) { + if (mtdparts_default && use_defaults) { + parts = mtdparts_default; + if (setenv("mtdparts", (char *)parts) == 0) + use_defaults = 0; + } else + printf("mtdparts variable not set, see 'help mtdparts'\n"); + }
if (parts && (strlen(parts) > MTDPARTS_MAXLEN - 1)) { printf("mtdparts too long (> %d)\n", MTDPARTS_MAXLEN); @@ -1936,9 +1945,10 @@ static int do_mtdparts(cmd_tbl_t *cmdtp, int flag, int argc, { if (argc == 2) { if (strcmp(argv[1], "default") == 0) { - setenv("mtdids", (char *)mtdids_default); - setenv("mtdparts", (char *)mtdparts_default); + setenv("mtdids", NULL); + setenv("mtdparts", NULL); setenv("partition", NULL); + use_defaults = 1;
mtdparts_init(); return 0;
Some CPUs contains boot ROM code capable reading first few blocks (where SPL resides) of NAND flash and executing it. It is wise to create separate partition here for SPL. As block size depends on NAND chip used, we could either use worst case (biggest) partition size or base its size on actual block size. This patch adds support for the latter option.
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- cmd/mtdparts.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index 53074a1..71c7acb 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -109,17 +109,20 @@ DECLARE_GLOBAL_DATA_PTR; #define MTD_WRITEABLE_CMD 1
/* default values for mtdids and mtdparts variables */ -#if defined(MTDIDS_DEFAULT) -static const char *const mtdids_default = MTDIDS_DEFAULT; -#else -static const char *const mtdids_default = NULL; +#if !defined(MTDIDS_DEFAULT) +#define MTDIDS_DEFAULT NULL #endif - -#if defined(MTDPARTS_DEFAULT) -static const char *const mtdparts_default = MTDPARTS_DEFAULT; +#if !defined(MTDPARTS_DEFAULT) +#define MTDPARTS_DEFAULT NULL +#endif +#if defined(CONFIG_SYS_MTDPARTS_DEFAULT_RUNTIME) +#define MTD_DEFAULTS_READONLY +extern void board_mtdparts_default(const char **mtdids, const char **mtdparts); #else -static const char *const mtdparts_default = NULL; +#define MTD_DEFAULTS_READONLY const #endif +static const char *MTD_DEFAULTS_READONLY mtdids_default = MTDIDS_DEFAULT; +static const char *MTD_DEFAULTS_READONLY mtdparts_default = MTDPARTS_DEFAULT;
/* copies of last seen 'mtdids', 'mtdparts' and 'partition' env variables */ #define MTDIDS_MAXLEN 128 @@ -1725,6 +1728,9 @@ int mtdparts_init(void) memset(last_ids, 0, MTDIDS_MAXLEN); memset(last_parts, 0, MTDPARTS_MAXLEN); memset(last_partition, 0, PARTITION_MAXLEN); +#if defined(CONFIG_SYS_MTDPARTS_DEFAULT_RUNTIME) + board_mtdparts_default(&mtdids_default, &mtdparts_default); +#endif use_defaults = 1; initialized = 1; }
Hello,
On 5 June 2016 at 19:43, Ladislav Michl ladis@linux-mips.org wrote:
Some CPUs contains boot ROM code capable reading first few blocks (where SPL resides) of NAND flash and executing it. It is wise to create separate partition here for SPL. As block size depends on NAND chip used, we could either use worst case (biggest) partition size or base its size on actual block size. This patch adds support for the latter option.
There is similar problem on sunxi.
Given this flash is non-removable and has many pins you are unlikely going to encounter its content on any system that was not loaded with u-boot.
Still Linux can only produce fixed size mtdparts. You can prepend perfectly sized mtdparts for u-boot but until Linux is taught to follow that with its own partitions without gap you still need to use the worst case scenario for the start of the Linux partitions.
On sunxi the range of supported block sizes and the size of the SPL+U-BOOT are not large so this is generally not worth the effort.
If support for this is attempted Linux should be probably taught to get mtdparts in pages and eraseblocks and then the default mtdparts can be in those.
If on the other hand you generate the fixed layout for Linux partitions on the fly and this patch changes how the Linux partitions are generated it can happen that the Linux partitions are at different place with different versions of u-boot giving potentially disastrous results.
Thanks
Miichal
On Sun, Jun 05, 2016 at 07:58:46PM +0200, Michal Suchanek wrote:
Hello,
On 5 June 2016 at 19:43, Ladislav Michl ladis@linux-mips.org wrote:
Some CPUs contains boot ROM code capable reading first few blocks (where SPL resides) of NAND flash and executing it. It is wise to create separate partition here for SPL. As block size depends on NAND chip used, we could either use worst case (biggest) partition size or base its size on actual block size. This patch adds support for the latter option.
There is similar problem on sunxi.
Given this flash is non-removable and has many pins you are unlikely going to encounter its content on any system that was not loaded with u-boot.
Still Linux can only produce fixed size mtdparts. You can prepend perfectly sized mtdparts for u-boot but until Linux is taught to follow that with its own partitions without gap you still need to use the worst case scenario for the start of the Linux partitions.
I didn't get 'fixed size mtdparts' part. Linux supports cmdline partition layout passing since the dawn of mtd subsystem (I used it more than a decade ago for netstar board). Finally that's a reason mtdparts implementation in U-Boot is done this way. Both U-Boot and Linux (can) see the same partition layout as it is passed either via kernel cmdline or device tree blob.
On sunxi the range of supported block sizes and the size of the SPL+U-BOOT are not large so this is generally not worth the effort.
If support for this is attempted Linux should be probably taught to get mtdparts in pages and eraseblocks and then the default mtdparts can be in those.
If on the other hand you generate the fixed layout for Linux partitions on the fly and this patch changes how the Linux partitions are generated it can happen that the Linux partitions are at different place with different versions of u-boot giving potentially disastrous results.
Unless I'm missing something, partition layout is passed to the kernel from the bootloader. So if kernel ends up with a different layout than a bootloader, there's a bug somewhere and that should be fixed.
Regards, ladis
On 5 June 2016 at 20:23, Ladislav Michl ladis@linux-mips.org wrote:
On Sun, Jun 05, 2016 at 07:58:46PM +0200, Michal Suchanek wrote:
Hello,
On 5 June 2016 at 19:43, Ladislav Michl ladis@linux-mips.org wrote:
Some CPUs contains boot ROM code capable reading first few blocks (where SPL resides) of NAND flash and executing it. It is wise to create separate partition here for SPL. As block size depends on NAND chip used, we could either use worst case (biggest) partition size or base its size on actual block size. This patch adds support for the latter option.
There is similar problem on sunxi.
Given this flash is non-removable and has many pins you are unlikely going to encounter its content on any system that was not loaded with u-boot.
Still Linux can only produce fixed size mtdparts. You can prepend perfectly sized mtdparts for u-boot but until Linux is taught to follow that with its own partitions without gap you still need to use the worst case scenario for the start of the Linux partitions.
I didn't get 'fixed size mtdparts' part. Linux supports cmdline partition layout passing since the dawn of mtd subsystem (I used it more
Sure.
It does not, however, support size in anything but some kind of fixed size blocks.
It cannot adjust the layout for different media page size or erase block size. It cannot specify that a partition follows after another or extends to the end of medium.
What you specify on the commandline is in no way related to the actual medium you are partitioning.
than a decade ago for netstar board). Finally that's a reason mtdparts implementation in U-Boot is done this way. Both U-Boot and Linux (can) see the same partition layout as it is passed either via kernel cmdline or device tree blob.
On sunxi the range of supported block sizes and the size of the SPL+U-BOOT are not large so this is generally not worth the effort.
If support for this is attempted Linux should be probably taught to get mtdparts in pages and eraseblocks and then the default mtdparts can be in those.
If on the other hand you generate the fixed layout for Linux partitions on the fly and this patch changes how the Linux partitions are generated it can happen that the Linux partitions are at different place with different versions of u-boot giving potentially disastrous results.
Unless I'm missing something, partition layout is passed to the kernel from the bootloader. So if kernel ends up with a different layout than a bootloader, there's a bug somewhere and that should be fixed.
That's not it. The problem is that if this patch changes the layout then building u-boot before this patch gives one layout and after this patch it gives another layout effectively changing the partitioning. All to save a few megabytes of a several gigiabyte medium. And if you say that people can always set the partitioning by hand then it completely defeats the purpose of fine-tuning the default in the first place.
If on the other hand Linux got support for sizing partitions in nand pages or eraseblocks and a patch changed the mtdparts layout to use the new units then either both u-boot and Linux support the units or parsing the partition fails. So it's safe and flexible and more general and probably even less work on u-boot's part.
Thanks
Michal
On Mon, Jun 06, 2016 at 09:08:47AM +0200, Michal Suchanek wrote:
On 5 June 2016 at 20:23, Ladislav Michl ladis@linux-mips.org wrote:
On Sun, Jun 05, 2016 at 07:58:46PM +0200, Michal Suchanek wrote:
There is similar problem on sunxi.
Given this flash is non-removable and has many pins you are unlikely going to encounter its content on any system that was not loaded with u-boot.
Still Linux can only produce fixed size mtdparts. You can prepend perfectly sized mtdparts for u-boot but until Linux is taught to follow that with its own partitions without gap you still need to use the worst case scenario for the start of the Linux partitions.
I didn't get 'fixed size mtdparts' part. Linux supports cmdline partition layout passing since the dawn of mtd subsystem (I used it more
Sure.
It does not, however, support size in anything but some kind of fixed size blocks.
It cannot adjust the layout for different media page size or erase block size. It cannot specify that a partition follows after another or extends to the end of medium.
That's not actually needed, as we know both page size and erase block size. Therefore it is easy to make layout respect those values, which was actual motivation for this patch. The later is simply not true, as you can certainly create a partition that extends to the end of medium: mtdparts=omap2-nand.0:512k(xloader),1920k(uboot),128k(uboot-env),10m(boot),-(rootfs) Above is something that I want to avoid and replace with: mtdparts=omap2-nand.0:<4*size_of_block>(SPL),-(UBI) (actually this is going to be translated to DTB before booting kernel) More free eraseblock for UBI, the better... Boot ROM reads from first (4) bare sector(s) and everything else is stored in UBI volumes. This is currently the only reliable way to make a use of MLC NAND.
What you specify on the commandline is in no way related to the actual medium you are partitioning.
What I'm specifying on cmdline (DTB) is what I'm going to get. I has always worked this way. Well, some drivers of even board support files had partitioning hardcoded, but we are trying to get rid of that.
[snip]
Unless I'm missing something, partition layout is passed to the kernel from the bootloader. So if kernel ends up with a different layout than a bootloader, there's a bug somewhere and that should be fixed.
That's not it. The problem is that if this patch changes the layout then building u-boot before this patch gives one layout and after this patch it gives another layout effectively changing the partitioning. All to save a few megabytes of a several gigiabyte medium. And if you say that people can always set the partitioning by hand then it completely defeats the purpose of fine-tuning the default in the first place.
U-Boot never prevented people from shooting their own leg, therefore there's no 'feature' preventing them to change default layout. They are still free to do whatever they want it they find it useful. Also I'm not proposing to change layout for exiting boards, just proving way to do it. igep00x0 boards had buggy NAND support since begining, so while fixing that, let's do it right way. Note that this feature is completely optional.
If on the other hand Linux got support for sizing partitions in nand pages or eraseblocks and a patch changed the mtdparts layout to use the new units then either both u-boot and Linux support the units or parsing the partition fails. So it's safe and flexible and more general and probably even less work on u-boot's part.
Okay, specifing size in therms of eraseblock would solve my problem and I have to admit I'm not aware such a thing exists. Any pointer to the patch introducing that change?
Thank you, ladis
On 6 June 2016 at 09:48, Ladislav Michl ladis@linux-mips.org wrote:
On Mon, Jun 06, 2016 at 09:08:47AM +0200, Michal Suchanek wrote:
On 5 June 2016 at 20:23, Ladislav Michl ladis@linux-mips.org wrote:
Unless I'm missing something, partition layout is passed to the kernel from the bootloader. So if kernel ends up with a different layout than a bootloader, there's a bug somewhere and that should be fixed.
That's not it. The problem is that if this patch changes the layout then building u-boot before this patch gives one layout and after this patch it gives another layout effectively changing the partitioning. All to save a few megabytes of a several gigiabyte medium. And if you say that people can always set the partitioning by hand then it completely defeats the purpose of fine-tuning the default in the first place.
U-Boot never prevented people from shooting their own leg, therefore there's no 'feature' preventing them to change default layout. They are still free to do whatever they want it they find it useful. Also I'm not proposing to change layout for exiting boards, just proving way to do it. igep00x0 boards had buggy NAND support since begining, so while fixing that, let's do it right way. Note that this feature is completely optional.
If on the other hand Linux got support for sizing partitions in nand pages or eraseblocks and a patch changed the mtdparts layout to use the new units then either both u-boot and Linux support the units or parsing the partition fails. So it's safe and flexible and more general and probably even less work on u-boot's part.
Okay, specifing size in therms of eraseblock would solve my problem and I have to admit I'm not aware such a thing exists. Any pointer to the patch introducing that change?
I am not aware of any. it's on the list of nice to have things I will probably not get to,
Anyway, It's imho the place to put this code so that Linux does not depend on u-boot for pre-chewing it's partition table to get the partitions right.
Thanks
Michal
On Mon, Jun 06, 2016 at 08:50:55PM +0200, Michal Suchanek wrote:
On 6 June 2016 at 09:48, Ladislav Michl ladis@linux-mips.org wrote:
[snip]
Okay, specifing size in therms of eraseblock would solve my problem and I have to admit I'm not aware such a thing exists. Any pointer to the patch introducing that change?
I am not aware of any. it's on the list of nice to have things I will probably not get to,
Anyway, It's imho the place to put this code so that Linux does not depend on u-boot for pre-chewing it's partition table to get the partitions right.
Linux already depends on U-Boot as U-Boot feeds Linux with MTD partitions. So this patch does not introduce any change in this regard. And whenever are paritions altered by user, saved to U-Boot environment or default layout provided according actual eraseblock size is just not important here. In the end both U-Boot and Linux are working with the same partition layout. This is the only thing that matters and this patch does not change that. It changes only this: instead of hardcoding default layout into string provided at compile time, there is now posibility to create that string runtime. That's all.
And now... Your recomended solution is to change both U-Boot and Linux to understand partition layout based on indexes and sizes expressed in eraseblocks. While size of SPL could be nicely evaluated in terms of eraseblocks, it is a bit more complicated with kernel partition, because 10 eraseblocks says nothing about partition size, therefore you cannot say whenever kernel will fit that space without knowledge of eraseblock size. So you just reverted the problem and both U-Boot and Linux would end with more code to be maintained for compatibility.
Best regards, ladis
On 6 June 2016 at 23:21, Ladislav Michl ladis@linux-mips.org wrote:
On Mon, Jun 06, 2016 at 08:50:55PM +0200, Michal Suchanek wrote:
On 6 June 2016 at 09:48, Ladislav Michl ladis@linux-mips.org wrote:
[snip]
Okay, specifing size in therms of eraseblock would solve my problem and I have to admit I'm not aware such a thing exists. Any pointer to the patch introducing that change?
I am not aware of any. it's on the list of nice to have things I will probably not get to,
Anyway, It's imho the place to put this code so that Linux does not depend on u-boot for pre-chewing it's partition table to get the partitions right.
Linux already depends on U-Boot as U-Boot feeds Linux with MTD partitions.
No. It depends on getting the partition layout on commandline or in DT. This can be something U-Boot generates, or something it passes on from its environment or something it does not touch at all.
So this patch does not introduce any change in this regard. And whenever are paritions altered by user, saved to U-Boot environment or default layout provided according actual eraseblock size is just not important here. In the end both U-Boot and Linux are working with the same partition layout. This is the only thing that matters and this patch does not change
But for the layout to be correct you must build current u-boot with this patch and not use compiled-in command line, devicetree, older u-boot or different bootloader.
that. It changes only this: instead of hardcoding default layout into string provided at compile time, there is now posibility to create that string runtime. That's all.
The only problem with that is that the code for generating the string is in u-boot and the string is needed in Linux so the logical thing is to move the code to Linux where it is needed.
And now... Your recomended solution is to change both U-Boot and Linux to understand partition layout based on indexes and sizes expressed in eraseblocks. While size of SPL could be nicely evaluated in terms of eraseblocks, it is a bit more complicated with kernel partition, because 10 eraseblocks says nothing about partition size, therefore you cannot say whenever kernel will fit that space without knowledge of eraseblock size.
That's why you need the possibility to start a partition at the end of preceding partition. That way you can have sizes of different partitions in different units.
It's interesting that your firmware uses erase block size for the boot partition. The sunxi firmware loads the bootloader from fixed number of pages.
So you just reverted the problem and both U-Boot and Linux would end with more code to be maintained for compatibility.
Yes, if you want U-Boot and Linux to talk the same units you need both implementing the code to handle them. Still U-Boot will only need to pass around a string with partition layout and parse it in the cases it is accessing the partitions.
Thanks
Michal
On Tue, Jun 07, 2016 at 09:43:20AM +0200, Michal Suchanek wrote:
On 6 June 2016 at 23:21, Ladislav Michl ladis@linux-mips.org wrote:
On Mon, Jun 06, 2016 at 08:50:55PM +0200, Michal Suchanek wrote: Linux already depends on U-Boot as U-Boot feeds Linux with MTD partitions.
No. It depends on getting the partition layout on commandline or in DT. This can be something U-Boot generates, or something it passes on from its environment or something it does not touch at all.
I really fail to understand how your wording conflicts with what I wrote above. Both cmdline and DT are provided by bootloader, so Linux depends on that. Unlike HDD for example where its layout is stored on media in the partition table.
So this patch does not introduce any change in this regard. And whenever are paritions altered by user, saved to U-Boot environment or default layout provided according actual eraseblock size is just not important here. In the end both U-Boot and Linux are working with the same partition layout. This is the only thing that matters and this patch does not change
But for the layout to be correct you must build current u-boot with this patch and not use compiled-in command line, devicetree, older u-boot or different bootloader.
No, layout is correct as it is for all boards I have there. But I was told boards with different NAND chips were produced. Also I have to change paritioning anyway, as original solution does not work in long term and devices are failing in field. I'm not proposing to break any existing setup. Your solution does the contrary as it requires both recent U-Boot and recent kernel. Also, that function is only helper one. If you decide to use static defaults, I'm fine with that. But with above patch, every user of that board will start with sane and optimal defaults. This is the sole point. Also I'm pretty sure previous patch which enables mtdparts to use defaults implicitely will not break anything, otherwise someone would notice bugs present in mtdparts code. (Perhaps it's time to read UBI SPL patch serie and related discusion for a reason I started to work on that)
that. It changes only this: instead of hardcoding default layout into string provided at compile time, there is now posibility to create that string runtime. That's all.
The only problem with that is that the code for generating the string is in u-boot and the string is needed in Linux so the logical thing is to move the code to Linux where it is needed.
No. The string is needed in both U-Boot _and_ Linux. I see no reason to change kernel as it is not byuig anything.
And now... Your recomended solution is to change both U-Boot and Linux to understand partition layout based on indexes and sizes expressed in eraseblocks. While size of SPL could be nicely evaluated in terms of eraseblocks, it is a bit more complicated with kernel partition, because 10 eraseblocks says nothing about partition size, therefore you cannot say whenever kernel will fit that space without knowledge of eraseblock size.
That's why you need the possibility to start a partition at the end of preceding partition. That way you can have sizes of different partitions in different units.
That posibility is already there for ages.
It's interesting that your firmware uses erase block size for the boot partition. The sunxi firmware loads the bootloader from fixed number of pages.
DM3730 tries to load from first eraseblock, if that fails, then second and finaly gives up at fourth (including).
So you just reverted the problem and both U-Boot and Linux would end with more code to be maintained for compatibility.
Yes, if you want U-Boot and Linux to talk the same units you need both implementing the code to handle them. Still U-Boot will only need to pass around a string with partition layout and parse it in the cases it is accessing the partitions.
And since U-Boot environment is usually stored in partition(s)... Also note that this change will break non-Linux systems and older Linux systems. And as you said, I'm doing that to save few megabytes on media, so now it seems to be too high price even to me.
Best regards, ladis
Just a sample implementation. Eventually become part of of UBI SPL patch series.
Signed-off-by: Ladislav Michl ladis@linux-mips.org --- board/isee/igep00x0/igep00x0.c | 16 ++++++++++++++++ include/configs/omap3_igep00x0.h | 1 + 2 files changed, 17 insertions(+)
diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c index 759daef..beaab79 100644 --- a/board/isee/igep00x0/igep00x0.c +++ b/board/isee/igep00x0/igep00x0.c @@ -18,6 +18,7 @@ #include <asm/arch/mux.h> #include <asm/arch/sys_proto.h> #include <asm/mach-types.h> +#include <linux/mtd/mtd.h> #include "igep00x0.h"
DECLARE_GLOBAL_DATA_PTR; @@ -184,6 +185,21 @@ int misc_init_r(void) return 0; }
+void board_mtdparts_default(const char **mtdids, const char **mtdparts) +{ + struct mtd_info *mtd = get_mtd_device(NULL, 0); + if (mtd) { + static char ids[24]; + static char parts[48]; + const char *linux_name = "omap2-nand"; + snprintf(ids, sizeof(ids), "%s=%s", mtd->name, linux_name); + snprintf(parts, sizeof(parts), "mtdparts=%s:%dk(SPL),-(UBI)", + linux_name, 4 * mtd->erasesize >> 10); + *mtdids = ids; + *mtdparts = parts; + } +} + /* * Routine: set_muxconf_regs * Description: Setting up the configuration Mux registers specific to the diff --git a/include/configs/omap3_igep00x0.h b/include/configs/omap3_igep00x0.h index 364d759..41782e3 100644 --- a/include/configs/omap3_igep00x0.h +++ b/include/configs/omap3_igep00x0.h @@ -146,6 +146,7 @@
#define CONFIG_RBTREE #define CONFIG_MTD_PARTITIONS +#define CONFIG_SYS_MTDPARTS_DEFAULT_RUNTIME
/* OneNAND boot config */ #ifdef CONFIG_BOOT_ONENAND
participants (2)
-
Ladislav Michl -
Michal Suchanek