[U-Boot] [PATCH] ARM: keystone: Pass SPI MTD partition table via kernel command line

[Vignesh R]

On Wednesday 08 March 2017 08:11 PM, Tom Rini wrote:
> On Mon, Mar 06, 2017 at 03:16:57PM +0530, Vignesh R wrote:
>> Hi,
>>
>> On Saturday 04 March 2017 10:39 PM, Tom Rini wrote:
>>> On Sat, Mar 04, 2017 at 06:17:30PM +0530, Vignesh R wrote:
>>>
>>>> SPI U-Boot image for K2 boards have now exceeded 512K partition
>>>> allocated to it and no longer fit the partitions defined in kernel DTS
>>>> file. Therefore, pass an updated MTD partition table from U-Boot as
>>>> kernel command line arguments to avoid kernel from accidentally
>>>> modifying boot loader image that has overflowed to next user partition.
>>>
>>> So pretty much everyone is now hitting the problem of "U-Boot" gets huge
>>> because we're including N DTB files in the binary in order to pick the
>>> correct one.  We really need to start figuring out a real solution here
>>> that perhaps involves saying where a copy of the tree lives on hardware,
>>> and if not found falling back to a generic functional-enough tree.
>>
>> Hmm.. The DTBs need to be stored on boot media(spi flash here) anyways
>> and that will lead to altering SPI paritions
> 
> Yes.  This, FWIW, is exactly the use case at least some people describe
> as the point where the hardware is capable of shipping the device tree.
> 
>>>> To do is, introduce a common environment file for declaring SPI
>>>> partition so that each individual boards need not repeat the same.
>>>> Choose appropriate SPI bus from board config file and pass it as command
>>>> line argument to kernel.
>>>>
>>>> Signed-off-by: Vignesh R <vigneshr at ti.com>
>>>
>>> So, we want the mtdparts to be passed in via the device tree, not the
>>> command line directly.  
> 
> To be clear, I was saying that fdt_fixup_mtdparts() should be used
> (which sadly isn't a common hook today, but that also gets to the next
> big problem below) to pass in whatever the partition layout is.  I'm not
> a huge fan of dumping tons of data into the cmdline, but that's because
> I swear I've seen cases where that ends up being truncated.
> 

Fixing up partition table is not transparent to user IMO. Only user
aware of U-Boot magic fdt_fixup_mtdparts() can figure out how partitions
are being re organized.

Also, currently mtdparts and fdt_fixup_mtdparts() don't seem to work too
well for SPI NOR devices. SPI flash has to be probed (using "sf probe"
at U-Boot prompt) at least once for it to be registered as MTD device so
that, mtdparts starts recognizing NOR partitions (and thereby fixing mtd
partition before jumping to kernel). If fdt_fixup_mtdparts() and mtdids
are to be used without "sf probe" from u-boot, then a patch to make all
spi flash devices automatically probe from board file needs to be added
(similar to initr_nand()).

>> Passing mtdparts via DT creates DT backward compatibility issues and
>> does not scale well if partitions size/layout needs to altered in future
>> like this case. OTOH, cmdline args provide flexibility of not being
>> static and is mostly transparent (as partitions appear in cmdline). Few
>> recent discussions on lkml also point to use of non static way of
>> passing mtd partition table with cmdline as one of the options:
>> https://patchwork.kernel.org/patch/9499119/
>> https://patchwork.kernel.org/patch/9515551/
> 
> Adding in Tony since it seems like there's some confusion here perhaps.
> If you change the partition table of flash on a given device, you've
> changed the partition table on a given device, and users that had been
> using that previous table are SOL.  Given that we (generally) do not
> have an MBR or GPT or anything on the front of flash, it doesn't matter
> if we have a "flag day" type change at the device tree file itself or
> now the environment has changed and we're passing in a different set of
> arguments somewhere.  So if we're changing the partitioning of flash
> frequently, it doesn't matter where we're putting the definition of it,
> the problem is that we're doing it as it means we haven't put enough
> thought into the design to start with.  Thanks!
> 

Yes, I agree that initial DT layout of 512K may not have been good
design, but it would be good to have an agreeable way of fixing up MTD
partitions when there is overflow. So, is fdt_fixup_mtdparts() preferred
approach?


-- 
Regards
Vignesh