[PATCH u-boot v2016.07-aspeed-openbmc] Add MTD and UBI support to ast-g5

[anoo]

Thanks Joel, answers inline below.

On 2017-08-21 06:03, Joel Stanley wrote:
> On Wed, Aug 16, 2017 at 11:39 AM, Adriana Kobylak
> <anoo at linux.vnet.ibm.com> wrote:
>> Add MTD and UBI support to the default g5 include, conditioned to
>> having MTDPARTS_DEFAULT defined.
> 
> Can you explain what we're planning on using mtdparts for? As I
> understand it this is for specifying the mtd partitions on the command
> line.
> 
> Are you proposing to move away from the device tree description?

mtdparts is needed by u-boot to find the kernel in a ubi volume, for 
example
the u-boot command "ubi part" requires it.
Implementing the device tree in u-boot could be a future possibility but
the current version of u-boot in openbmc can't read from any other chips
besides the bmc, so we need to duplicate the device tree layout of the 
bmc
chip in u-boot for now.

> 
>> This allows platforms to enable this support if desired without
>> adding it by default since it increases the size of u-boot.
> 
> Do you have numbers? It's good to keep track in case we need to go
> back and save space in the future.

After you mentioned that you saw u-boot increasing by 122kB with an 
earlier
patch, I removed UBIFS since it wasn't needed, bringing the increase 
down
by 50kB. The new size is 302kB (increase of 72kB over current size).

> 
>> Signed-off-by: Adriana Kobylak <anoo at linux.vnet.ibm.com>
>> ---
>>  include/configs/ast-g5-ncsi.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/include/configs/ast-g5-ncsi.h 
>> b/include/configs/ast-g5-ncsi.h
>> index 12d6684..b126e6e 100644
>> --- a/include/configs/ast-g5-ncsi.h
>> +++ b/include/configs/ast-g5-ncsi.h
>> @@ -28,4 +28,14 @@
>> 
>>  #define CONFIG_HW_WATCHDOG
>> 
>> +/* MTD and UBI */
>> +#ifdef MTDPARTS_DEFAULT
>> +#define CONFIG_FLASH_CFI_MTD
>> +#define CONFIG_CMD_UBI
> 
> I thought Milton mentioned today that we didn't need to enable the UBI
> command in u-boot, but I may have misunderstood.

It is needed since the kernel is in ubi volume.

> 
>> +#define CONFIG_RBTREE
>> +#define CONFIG_MTD_DEVICE
>> +#define CONFIG_MTD_PARTITIONS
>> +#define CONFIG_CMD_MTDPARTS
>> +#endif
>> +
>>  #endif /* __AST_G5_NCSI_CONFIG_H */
>> --
>> 1.8.2.2
>>