[MTD] ofpart: Partitions at same address cannot have the same name

[MTD] ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Sometimes, an special partition is included in the device tree including all the
partitions. Like in:

partition@ff000000 {
	reg = < 0x000000 0x800000 >;
	label = "Root File System";
};
partition@ff800000 {
	reg = < 0x800000 0x1a0000 >;
	label = "Bitstream";
};
...
partitionAll@ff000000 {
	reg = < 0x000000 0x1000000 >;
	label = "Full FLASH";
};

Because two nodes of a device tree cannot have the same name, but all the 
partitions must be named "partition", this special partition is invalid.

This patch makes ofpart.c only check for the firt part of the name, and 
ignore the rest, allowing this special partition.


---

The extra partition is quite useful while formating the full firmware from linux

 drivers/mtd/ofpart.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 3e164f0..0af3b07 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -48,7 +48,8 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
 
 		/* check if this is a partition node */
 		partname = of_get_property(pp, "name", &len);
-		if (strcmp(partname, "partition") != 0) {
+		if (strncmp(partname, "partition", strlen("partition")-1)
+									!= 0) {
 			nr_parts--;
 			continue;
 		}
-- 
1.6.2.4

Re: [MTD] ofpart: Partitions at same address cannot have the same name

[Peter Korsgaard]

>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@uam.es> writes:

Hi,

 Ricardo> Sometimes, an special partition is included in the device
 Ricardo> tree including all the partitions. Like in:

 Ricardo>  drivers/mtd/ofpart.c |    3 ++-
 Ricardo>  1 files changed, 2 insertions(+), 1 deletions(-)

 Ricardo> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
 Ricardo> index 3e164f0..0af3b07 100644
 Ricardo> --- a/drivers/mtd/ofpart.c
 Ricardo> +++ b/drivers/mtd/ofpart.c
 Ricardo> @@ -48,7 +48,8 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
 
 Ricardo>  		/* check if this is a partition node */
 Ricardo>  		partname = of_get_property(pp, "name", &len);
 Ricardo> -		if (strcmp(partname, "partition") != 0) {
 Ricardo> +		if (strncmp(partname, "partition", strlen("partition")-1)

Why strlen() - 1 ?

-- 
Bye, Peter Korsgaard

Re: [MTD] ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Hello

You are right, remove the -1. I thought that strlen gives the #of
chars + 1 ('\0').


  Thanks



On Wed, Apr 22, 2009 at 11:24, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Ricardo" =3D=3D Ricardo Ribalda Delgado <ricardo.ribalda@uam.es> wr=
ites:
>
> Hi,
>
> =A0Ricardo> Sometimes, an special partition is included in the device
> =A0Ricardo> tree including all the partitions. Like in:
>
> =A0Ricardo> =A0drivers/mtd/ofpart.c | =A0 =A03 ++-
> =A0Ricardo> =A01 files changed, 2 insertions(+), 1 deletions(-)
>
> =A0Ricardo> diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
> =A0Ricardo> index 3e164f0..0af3b07 100644
> =A0Ricardo> --- a/drivers/mtd/ofpart.c
> =A0Ricardo> +++ b/drivers/mtd/ofpart.c
> =A0Ricardo> @@ -48,7 +48,8 @@ int __devinit of_mtd_parse_partitions(struc=
t device *dev,
>
> =A0Ricardo> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* check if this is a partition n=
ode */
> =A0Ricardo> =A0 =A0 =A0 =A0 =A0 =A0 =A0 partname =3D of_get_property(pp, =
"name", &len);
> =A0Ricardo> - =A0 =A0 =A0 =A0 =A0 =A0 if (strcmp(partname, "partition") !=
=3D 0) {
> =A0Ricardo> + =A0 =A0 =A0 =A0 =A0 =A0 if (strncmp(partname, "partition", =
strlen("partition")-1)
>
> Why strlen() - 1 ?
>
> --
> Bye, Peter Korsgaard
>



--=20
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/

ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Sometimes, an special partition is included in the device tree including all the
partitions. Like in:

partition@ff000000 {
       reg = < 0x000000 0x800000 >;
       label = "Root File System";
};
partition@ff800000 {
       reg = < 0x800000 0x1a0000 >;
       label = "Bitstream";
};
...
partitionAll@ff000000 {
       reg = < 0x000000 0x1000000 >;
       label = "Full FLASH";
};

Because two nodes of a device tree cannot have the same name, but all the
partitions must be named "partition", this special partition is invalid.

This patch makes ofpart.c only check for the firt part of the name, and
ignore the rest, allowing this special partition.


---

The extra partition is quite useful while formating the full firmware from linux

v2, removes the -1 <Peter Korsgaard>

 drivers/mtd/ofpart.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 3e164f0..0af3b07 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -48,7 +48,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
 
 		/* check if this is a partition node */
 		partname = of_get_property(pp, "name", &len);
-		if (strcmp(partname, "partition") != 0) {
+		if (strncmp(partname, "partition", strlen("partition") != 0) {
 			nr_parts--;
 			continue;
 		}
-- 
1.6.2.4

Re: ofpart: Partitions at same address cannot have the same name

[Benjamin Krill]

>--- a/drivers/mtd/ofpart.c
>+++ b/drivers/mtd/ofpart.c
>@@ -48,7 +48,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
> 
> 		/* check if this is a partition node */
> 		partname = of_get_property(pp, "name", &len);
>-		if (strcmp(partname, "partition") != 0) {
>+		if (strncmp(partname, "partition", strlen("partition") != 0) {

Hi Recardo,

I would suggest to do:

		if (strcmp(partname, "partition") <= 0) {

cheers
 ben

Re: ofpart: Partitions at same address cannot have the same name

[Scott Wood]

Benjamin Krill wrote:
>> --- a/drivers/mtd/ofpart.c
>> +++ b/drivers/mtd/ofpart.c
>> @@ -48,7 +48,7 @@ int __devinit of_mtd_parse_partitions(struct device *dev,
>>
>> 		/* check if this is a partition node */
>> 		partname = of_get_property(pp, "name", &len);
>> -		if (strcmp(partname, "partition") != 0) {
>> +		if (strncmp(partname, "partition", strlen("partition") != 0) {

Perhaps "compatible" should be used instead?

> Hi Recardo,
> 
> I would suggest to do:
> 
> 		if (strcmp(partname, "partition") <= 0) {

Check whether it sorts alphabetically before "partition"?  Why?

-Scott

Re: ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Hi Scott

> Perhaps "compatible" should be used instead?

What do you mean?

if (strcmp(partname, "partition") || strcmp(partname, "compatible") )

I can't see the advantages.


>
>> Hi Recardo,
>>
>> I would suggest to do:
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(partname, "partition") <=3D 0)=
 {
>
> Check whether it sorts alphabetically before "partition"? =A0Why?
>
> -Scott
>



--=20
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/

Re: ofpart: Partitions at same address cannot have the same name

[Scott Wood]

Ricardo Ribalda Delgado wrote:
> Hi Scott
> 
>> Perhaps "compatible" should be used instead?
> 
> What do you mean?
> 
> if (strcmp(partname, "partition") || strcmp(partname, "compatible") )
> 
> I can't see the advantages.

No, I mean:

foo {
	compatible = "partition";
	...
};

-Scott

Re: ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Hello Scott

It is definitively more elegant...

  Let me send tomorrow a patch

On Wed, Apr 22, 2009 at 20:11, Scott Wood <scottwood@freescale.com> wrote:
> Ricardo Ribalda Delgado wrote:
>>
>> Hi Scott
>>
>>> Perhaps "compatible" should be used instead?
>>
>> What do you mean?
>>
>> if (strcmp(partname, "partition") || strcmp(partname, "compatible") )
>>
>> I can't see the advantages.
>
> No, I mean:
>
> foo {
> =A0 =A0 =A0 =A0compatible =3D "partition";
> =A0 =A0 =A0 =A0...
> };
>
> -Scott
>



--=20
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/

Re: ofpart: Partitions at same address cannot have the same name

[Ricardo Ribalda Delgado]

Hello Benjamin

> Hi Recardo,
>
> I would suggest to do:
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(partname, "partition") <=3D 0) =
{

Anything alfabetically higher than partition (like "zzzzz" will pass
the test :S)

Regards



>
> cheers
> =A0ben
>
>



--=20
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/

Re: ofpart: Partitions at same address cannot have the same name

[Benjamin Krill]

* Ricardo Ribalda Delgado | 2009-04-22 19:59:08 [+0200]:
>>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (strcmp(partna=
me, "partition") <=3D 0) {
>
>Anything alfabetically higher than partition (like "zzzzz" will pass
>the test :S)

You are totally right!

cheers
 ben

Re: [MTD] ofpart: Partitions at same address cannot have the same name

[Benjamin Herrenschmidt]

On Wed, 2009-04-22 at 10:05 +0200, Ricardo Ribalda Delgado wrote:
> Sometimes, an special partition is included in the device tree including all the
> partitions. Like in:
> 
> partition@ff000000 {
> 	reg = < 0x000000 0x800000 >;
> 	label = "Root File System";
> };
> partition@ff800000 {
> 	reg = < 0x800000 0x1a0000 >;
> 	label = "Bitstream";
> };
> ...
> partitionAll@ff000000 {
> 	reg = < 0x000000 0x1000000 >;
> 	label = "Full FLASH";
> };
> 
> Because two nodes of a device tree cannot have the same name, but all the 
> partitions must be named "partition", this special partition is invalid.
> 
> This patch makes ofpart.c only check for the firt part of the name, and 
> ignore the rest, allowing this special partition.

I fail to see the point of this "special" partition in the first
place...

Things would make more sense if you had a full flash device
whose child nodes are the partitions.

Ben.

Re: [MTD] ofpart: Partitions at same address cannot have the same name

[David Woodhouse]

On Thu, 2009-04-30 at 04:19 +0100, Benjamin Herrenschmidt wrote:
> 
> I fail to see the point of this "special" partition in the first
> place...
> 
> Things would make more sense if you had a full flash device
> whose child nodes are the partitions.

That's the model I think I want to move to, and which I was toying with
in http://git.infradead.org/users/dwmw2/mtd-sysfs.git (I haven't done it
yet, but it's logically the next step after what I've already done).

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation