diff mbox

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

Message ID 84159f3220054bfa87ce9a72443afa1a@rwthex-s1-b.rwth-ad.de
State Changes Requested
Delegated to: Stefano Babic
Headers show

Commit Message

Christopher Spinrath June 19, 2016, 3:44 p.m. UTC
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(-)

Comments

Igor Grinberg June 22, 2016, 4:02 p.m. UTC | #1
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
> @@ -12,6 +12,7 @@
>  #include <dm.h>
>  #include <fsl_esdhc.h>
>  #include <miiphy.h>
> +#include <mtd_node.h>
>  #include <netdev.h>
>  #include <errno.h>
>  #include <usb.h>
> @@ -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?

>  #include "common.h"
>  #include "../common/eeprom.h"
>  #include "../common/common.h"
> @@ -581,6 +583,13 @@ int cm_fx6_setup_ecspi(void) { return 0; }
>  
>  #ifdef CONFIG_OF_BOARD_SETUP
>  #define USDHC3_PATH	"/soc/aips-bus@02100000/usdhc@02198000/"
> +
> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
> +struct node_info nodes[] = {
> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
> +};
> +#endif
> +
>  int ft_board_setup(void *blob, bd_t *bd)
>  {
>  	u32 baseboard_rev;
> @@ -589,6 +598,8 @@ int ft_board_setup(void *blob, bd_t *bd)
>  	char baseboard_name[16];
>  	int err;
>  
> +	fdt_shrink_to_minimum(blob); /* Make room for new properties */
> +
>  	/* MAC addr */
>  	if (eth_getenv_enetaddr("ethaddr", enetaddr)) {
>  		fdt_find_and_setprop(blob,
> @@ -607,7 +618,6 @@ int ft_board_setup(void *blob, bd_t *bd)
>  		return 0; /* Assume not an early revision SB-FX6m baseboard */
>  
>  	if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) {
> -		fdt_shrink_to_minimum(blob); /* Make room for new properties */
>  		nodeoffset = fdt_path_offset(blob, USDHC3_PATH);
>  		fdt_delprop(blob, nodeoffset, "cd-gpios");
>  		fdt_find_and_setprop(blob, USDHC3_PATH, "broken-cd",
> @@ -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?

> +
>  	return 0;
>  }
>  #endif
>
Igor Grinberg June 22, 2016, 4:17 p.m. UTC | #2
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

[...]

> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
> +struct node_info nodes[] = {
> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},

Nikita, is this enough for all flashes we assemble on cm-fx6?

> +};
> +#endif
> +

[...]
Christopher Spinrath June 22, 2016, 7:21 p.m. UTC | #3
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
>> @@ -12,6 +12,7 @@
>>  #include <dm.h>
>>  #include <fsl_esdhc.h>
>>  #include <miiphy.h>
>> +#include <mtd_node.h>
>>  #include <netdev.h>
>>  #include <errno.h>
>>  #include <usb.h>
>> @@ -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).

>>  #include "common.h"
>>  #include "../common/eeprom.h"
>>  #include "../common/common.h"
>> @@ -581,6 +583,13 @@ int cm_fx6_setup_ecspi(void) { return 0; }
>>  
>>  #ifdef CONFIG_OF_BOARD_SETUP
>>  #define USDHC3_PATH	"/soc/aips-bus@02100000/usdhc@02198000/"
>> +
>> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
>> +struct node_info nodes[] = {
>> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
>> +};
>> +#endif
>> +
>>  int ft_board_setup(void *blob, bd_t *bd)
>>  {
>>  	u32 baseboard_rev;
>> @@ -589,6 +598,8 @@ int ft_board_setup(void *blob, bd_t *bd)
>>  	char baseboard_name[16];
>>  	int err;
>>  
>> +	fdt_shrink_to_minimum(blob); /* Make room for new properties */
>> +
>>  	/* MAC addr */
>>  	if (eth_getenv_enetaddr("ethaddr", enetaddr)) {
>>  		fdt_find_and_setprop(blob,
>> @@ -607,7 +618,6 @@ int ft_board_setup(void *blob, bd_t *bd)
>>  		return 0; /* Assume not an early revision SB-FX6m baseboard */
>>  
>>  	if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) {
>> -		fdt_shrink_to_minimum(blob); /* Make room for new properties */
>>  		nodeoffset = fdt_path_offset(blob, USDHC3_PATH);
>>  		fdt_delprop(blob, nodeoffset, "cd-gpios");
>>  		fdt_find_and_setprop(blob, USDHC3_PATH, "broken-cd",
>> @@ -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?

Cheers,
Christopher

>> +
>>  	return 0;
>>  }
>>  #endif
>>
>
Igor Grinberg June 23, 2016, 8:56 a.m. UTC | #4
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.
Christopher Spinrath June 25, 2016, 3:03 p.m. UTC | #5
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@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
Nikita Kiryanov July 7, 2016, 8:53 a.m. UTC | #6
On Wed, Jun 22, 2016 at 07:17:53PM +0300, Igor Grinberg wrote:
> 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
> 
> [...]
> 
> > +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
> > +struct node_info nodes[] = {
> > +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
> 
> Nikita, is this enough for all flashes we assemble on cm-fx6?

Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are
supported by the m25p80.c driver. However, on the mainline branch
I don't see "m25p" in the list of device ids, and IIRC the request
is to favor "jedec,spi-nor" as compatible string over device specific
ones.


> 
> > +};
> > +#endif
> > +
> 
> [...]
> 
> -- 
> Regards,
> Igor.
>
Christopher Spinrath July 7, 2016, 1:30 p.m. UTC | #7
Hi Nikita,

On 07/07/2016 10:53 AM, Nikita Kiryanov wrote:
> On Wed, Jun 22, 2016 at 07:17:53PM +0300, Igor Grinberg wrote:
>> 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
>>
>> [...]
>>
>>> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
>>> +struct node_info nodes[] = {
>>> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
>>
>> Nikita, is this enough for all flashes we assemble on cm-fx6?
> 
> Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are
> supported by the m25p80.c driver. However, on the mainline branch
> I don't see "m25p" in the list of device ids, and IIRC the request
> is to favor "jedec,spi-nor" as compatible string over device specific
> ones.

Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list
(currently queued for inclusion in v4.8:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
).

I have chosen "st,m25p" here to cover both the mainline and CompuLab's
device trees (I have seen some where "jedec,spi-nor" is not in the
list). However, if you prefer I will switch to "jedec,spi-nor"
(excluding some device trees) in v2.

Thanks,
Christopher

> 
>>
>>> +};
>>> +#endif
>>> +
>>
>> [...]
>>
>> -- 
>> Regards,
>> Igor.
>>
Nikita Kiryanov July 10, 2016, 7:52 a.m. UTC | #8
Hi Christopher,

On Thu, Jul 07, 2016 at 03:30:25PM +0200, Christopher Spinrath wrote:
> Hi Nikita,
> 
> On 07/07/2016 10:53 AM, Nikita Kiryanov wrote:
> > On Wed, Jun 22, 2016 at 07:17:53PM +0300, Igor Grinberg wrote:
> >> 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
> >>
> >> [...]
> >>
> >>> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
> >>> +struct node_info nodes[] = {
> >>> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
> >>
> >> Nikita, is this enough for all flashes we assemble on cm-fx6?
> > 
> > Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are
> > supported by the m25p80.c driver. However, on the mainline branch
> > I don't see "m25p" in the list of device ids, and IIRC the request
> > is to favor "jedec,spi-nor" as compatible string over device specific
> > ones.
> 
> Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list
> (currently queued for inclusion in v4.8:
> https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
> ).
> 
> I have chosen "st,m25p" here to cover both the mainline and CompuLab's
> device trees (I have seen some where "jedec,spi-nor" is not in the
> list). However, if you prefer I will switch to "jedec,spi-nor"
> (excluding some device trees) in v2.

Does it have to be an "or" situation? m25p is necessary to serve older CM-FX6
kernels, but it is not supported in the mainline kernel, so the correct
course of actions seems to be to use both "st,m25p" and "jedec,spi-nor".
Christopher Spinrath July 11, 2016, 11:54 a.m. UTC | #9
Hi Nikita,

On 10.07.2016 09:52, Nikita Kiryanov wrote:
> Hi Christopher,
>
> On Thu, Jul 07, 2016 at 03:30:25PM +0200, Christopher Spinrath wrote:
>> Hi Nikita,
>>
>> On 07/07/2016 10:53 AM, Nikita Kiryanov wrote:
>>> On Wed, Jun 22, 2016 at 07:17:53PM +0300, Igor Grinberg wrote:
>>>> 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
>>>>
>>>> [...]
>>>>
>>>>> +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
>>>>> +struct node_info nodes[] = {
>>>>> +	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
>>>>
>>>> Nikita, is this enough for all flashes we assemble on cm-fx6?
>>>
>>> Yes, CM-FX6 is using M25PX16 and SST25VF016B, both of which are
>>> supported by the m25p80.c driver. However, on the mainline branch
>>> I don't see "m25p" in the list of device ids, and IIRC the request
>>> is to favor "jedec,spi-nor" as compatible string over device specific
>>> ones.
>>
>> Linux is going to use "st,m25p", "jedec,spi-nor" as compatible list
>> (currently queued for inclusion in v4.8:
>> https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/tree/arch/arm/boot/dts/imx6q-cm-fx6.dts?h=next/dt#n123
>> ).
>>
>> I have chosen "st,m25p" here to cover both the mainline and CompuLab's
>> device trees (I have seen some where "jedec,spi-nor" is not in the
>> list). However, if you prefer I will switch to "jedec,spi-nor"
>> (excluding some device trees) in v2.
>
> Does it have to be an "or" situation? m25p is necessary to serve older CM-FX6
> kernels, but it is not supported in the mainline kernel, so the correct
> course of actions seems to be to use both "st,m25p" and "jedec,spi-nor".

This has nothing to do with the kernel itself; the compatible is only 
used to find the correct dt node describing the flash chip and as I said 
in my previous email, the upstream device tree node is going to have the 
st,m25p compatible string.

On the other hand, m25p is not documented but it used to be in upstream 
device trees. Since it has been removed from most of them recently, I 
agree that having both compatibles is a good idea. From a quick look at 
the source code it should be possible. I will verify this for v2.

Thanks,
Christopher
diff mbox

Patch

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
@@ -12,6 +12,7 @@ 
 #include <dm.h>
 #include <fsl_esdhc.h>
 #include <miiphy.h>
+#include <mtd_node.h>
 #include <netdev.h>
 #include <errno.h>
 #include <usb.h>
@@ -28,6 +29,7 @@ 
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <dm/platform_data/serial_mxc.h>
+#include <jffs2/load_kernel.h>
 #include "common.h"
 #include "../common/eeprom.h"
 #include "../common/common.h"
@@ -581,6 +583,13 @@  int cm_fx6_setup_ecspi(void) { return 0; }
 
 #ifdef CONFIG_OF_BOARD_SETUP
 #define USDHC3_PATH	"/soc/aips-bus@02100000/usdhc@02198000/"
+
+#ifdef CONFIG_FDT_FIXUP_PARTITIONS
+struct node_info nodes[] = {
+	{ "st,m25p",	MTD_DEV_TYPE_NOR,	},
+};
+#endif
+
 int ft_board_setup(void *blob, bd_t *bd)
 {
 	u32 baseboard_rev;
@@ -589,6 +598,8 @@  int ft_board_setup(void *blob, bd_t *bd)
 	char baseboard_name[16];
 	int err;
 
+	fdt_shrink_to_minimum(blob); /* Make room for new properties */
+
 	/* MAC addr */
 	if (eth_getenv_enetaddr("ethaddr", enetaddr)) {
 		fdt_find_and_setprop(blob,
@@ -607,7 +618,6 @@  int ft_board_setup(void *blob, bd_t *bd)
 		return 0; /* Assume not an early revision SB-FX6m baseboard */
 
 	if (!strncmp("SB-FX6m", baseboard_name, 7) && baseboard_rev <= 120) {
-		fdt_shrink_to_minimum(blob); /* Make room for new properties */
 		nodeoffset = fdt_path_offset(blob, USDHC3_PATH);
 		fdt_delprop(blob, nodeoffset, "cd-gpios");
 		fdt_find_and_setprop(blob, USDHC3_PATH, "broken-cd",
@@ -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
+
 	return 0;
 }
 #endif