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

[Vignesh R]

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

>> 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.  

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/

> I'm also not super keen on anything that adds more to:
> 
>>  include/configs/k2e_evm.h            |  5 +++++
>>  include/configs/k2g_evm.h            |  3 +++
>>  include/configs/k2hk_evm.h           |  4 ++++
>>  include/configs/k2l_evm.h            |  4 ++++
>>  include/configs/ti_armv7_keystone2.h |  7 ++++++-
> 

The additions are mostly choosing the SPI instance to which nor flash is
connected which differs wrt k2g(SPI1) and other k2 boards(SPI0).

> Aside from #include <environment/ti/...h>.
> 
> [snip]
>> diff --git a/include/environment/ti/spi.h b/include/environment/ti/spi.h
>> new file mode 100644
>> index 000000000000..6fea9d61f071
>> --- /dev/null
>> +++ b/include/environment/ti/spi.h
>> @@ -0,0 +1,15 @@
>> +/*
>> + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com
>> + *
>> + * Environment variable definitions for SPI on TI boards.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#ifndef __TI_SPI_H
>> +#define __TI_SPI_H
>> +
>> +#define SPI0_MTD_PARTS "spi0.0:1m(u-boot-spl)ro,-(misc);\0"
>> +#define SPI1_MTD_PARTS "spi1.0:1m(u-boot-spl)ro,-(misc);\0"
> 
> OK, but if you look at all of the other SPI mtdparts on TI platforms
> this isn't enough and is very keystone specific.

Well, its mostly size specific. OMAP devices have bigger QSPI flashes a
K2* devices have just 16MB flash and hence, this layout. I will rename
it as KEYSTONE_SPIx_MTD_PART. But adding it here will enable partition
table to be used across k2 board config files.

> Also, curious, why aren't you making use of mtdids to give a more descriptive name here?

AFAIK, mtdids are supported for nand flash, parallel nor and dataflash
and not for SPI NOR devices supported under drivers/mtd/spi/spi_flash.c.
They are not modelled as MTD devices AFAIU


-- 
Regards
Vignesh