[U-Boot] [PATCH 2/3] ARM: board: cm_fx6: fixup mtd partitions in the fdt

[Igor Grinberg <grinberg@compulab.co.il>]

Hi Christopher,

On 06/22/2016 10:21 PM, Christopher Spinrath wrote:
> Hi Igor,
> 
> On 06/22/2016 06:02 PM, Igor Grinberg wrote:
>> Hi Christopher,
>>
>> On 06/19/2016 06:44 PM, Christopher Spinrath wrote:
>>> The cm-fx6 module has an on-board st,m25p compatible spi flash chip
>>> used for u-boot (binary & environment). Overwrite the partitions in
>>> the device tree by the partition table provided in the mtdparts
>>> environment variable, if it is set.
>>>
>>> This allows to specify a kernel independent partitioning in the
>>> environment and provides a convient way for the user to adapt the
>>> partition table.
>>>
>>> Signed-off-by: Christopher Spinrath <christopher.spinrath@rwth-aachen.de>
>>> ---
>>>  board/compulab/cm_fx6/cm_fx6.c | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
>>> index 712057a..81a7ae2 100644
>>> --- a/board/compulab/cm_fx6/cm_fx6.c
>>> +++ b/board/compulab/cm_fx6/cm_fx6.c

[...]

>>> @@ -28,6 +29,7 @@
>>>  #include <asm/io.h>
>>>  #include <asm/gpio.h>
>>>  #include <dm/platform_data/serial_mxc.h>
>>> +#include <jffs2/load_kernel.h>
>>
>> Why is this needed?
>>
> The MTD_DEV_TYPE_NOR constant is defined in this header. I agree that it
> is a bit ugly but this header is used for the same purpose in other
> board files, too (e.g.board/pdm360ng/pdm360ng.c).

I see.
I don't feel right to request this in scope of these patches, but
if you can take care of that one (even in a follow up patch) - it would
be awesome.

[...]

>>> @@ -616,6 +626,10 @@ int ft_board_setup(void *blob, bd_t *bd)
>>>  				     NULL, 0, 1);
>>>  	}
>>>  
>>> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
>>> +	fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
>>> +#endif
>>
>> I really dislike the ifdeffery inside functions.
>> Care to introduce a stub for the !CONFIG_FDT_FIXUP_PARTITIONS case in
>> include/fdt_support.h for this one?
>>
> Sure. Is the header the correct place for this or should I add a #else
> case in the .c file?

IMO, the header file is better for stubbing things out.
It does not require you to add .c into compilation.
There are also already several examples inside this header.

-- 
Regards,
Igor.