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

[Christopher Spinrath]

Hi Igor,

On 06/23/2016 10:56 AM, Igor Grinberg wrote:
> 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 at 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.
>

I prefer to defer it, since it touches another subsystem (jffs).

> [...]
>
>>>> @@ -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.
>
Ok.

Thanks,
Christopher