Message ID | 1240581715-25926-1-git-send-email-ricardo.ribalda@uam.es (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
>--- a/drivers/mtd/ofpart.c >+++ b/drivers/mtd/ofpart.c >@@ -48,7 +48,9 @@ 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 ((strcmp(partname, "partition") != 0) && >+ (of_device_is_compatible(pp, "partition") != 1)) >+ { > nr_parts--; > continue; > } If this is the way, how to go, you get my ack. Acked-by: Benjamin Krill <ben@codiert.org>
> 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"; > }; > ... > full@ff000000 { > compatible = "partition"; > reg = < 0x000000 0x1000000 >; > label = "Full FLASH"; > }; Your special "partition" isn't really a partition then, is it. Because of that, device nodes to represent your partitions doesn't work very well. You really want to use something else, a partition table on the flash itself for example. Or maybe the (platform? MTD?) code should create a Linux device for the "full" device. > Because two nodes of a device tree cannot have the same name, This isn't true. > but all the > partitions must be named "partition", Bad binding, no cookie for you! > - if (strcmp(partname, "partition") != 0) { > + if ((strcmp(partname, "partition") != 0) && > + (of_device_is_compatible(pp, "partition") != 1)) > + { You cannot claim a name as generic as "partition" for this. Pick something else if you really must do things this way. Segher
Hello Segher Thanks for your comments. > > Your special "partition" isn't really a partition then, is it. > Because of that, device nodes to represent your partitions doesn't > work very well. I think that they work pretty well. Unfortunately, since 4b08e149c0e02e97ec49c2a31d14a0d3a02f8074 all the partiton must be named "partition" > > You really want to use something else, a partition table on the > flash itself for example. Or maybe the (platform? MTD?) code > should create a Linux device for the "full" device. This wont be very flexible. With the device tree aproach we can define partitions not only for the full flash, but also for two partitions merged, a partition inside a partition.... > >> Because two nodes of a device tree cannot have the same name, > > This isn't true. Are you sure? This is what I get if I try to compile a device tree with two partitions with the same name starting at the same address. $ arch/powerpc/boot/dtc -O dtb -b 0 -p 1024 arch/powerpc/boot/dts/q5-avnet.dts -o /tmp/kk DTC: dts->dtb on file "arch/powerpc/boot/dts/q5-avnet.dts" ERROR (duplicate_node_names): Duplicate node name /plb@0/flash@ff000000/partition@ff000000 ERROR: Input tree has errors, aborting (use -f to force output) > >> but all the >> partitions must be named "partition", > > Bad binding, no cookie for you! Sorry, I dont understand want you want to say here. > You cannot claim a name as generic as "partition" for this. Pick > something else if you really must do things this way. You choose :) Best regards
>> Your special "partition" isn't really a partition then, is it. >> Because of that, device nodes to represent your partitions doesn't >> work very well. > > I think that they work pretty well. With "real" devices you don't normally have two devices sitting at the same address. Since you _do_ have that with your partitions, it doesn't fit the device node abstraction very well. > Unfortunately, since > 4b08e149c0e02e97ec49c2a31d14a0d3a02f8074 all the partiton must be > named "partition" Ah! After writing my original reply, I read the documentation, and this had me confused. 4b08e14 looks wrong to me. It also didn't update the documentation. >> You really want to use something else, a partition table on the >> flash itself for example. Or maybe the (platform? MTD?) code >> should create a Linux device for the "full" device. > > This wont be very flexible. With the device tree aproach we can define > partitions not only for the full flash, but also for two partitions > merged, a partition inside a partition.... Well, except you have problems doing just that :-P >>> Because two nodes of a device tree cannot have the same name, >> >> This isn't true. > > Are you sure? Yes. > This is what I get if I try to compile a device tree > with two partitions with the same name starting at the same address. > > $ arch/powerpc/boot/dtc -O dtb -b 0 -p 1024 > arch/powerpc/boot/dts/q5-avnet.dts -o /tmp/kk > DTC: dts->dtb on file "arch/powerpc/boot/dts/q5-avnet.dts" > ERROR (duplicate_node_names): Duplicate node name > /plb@0/flash@ff000000/partition@ff000000 > ERROR: Input tree has errors, aborting (use -f to force output) I think this error is trying to say both the names and the unit addresses are identical (and the latter exist), which isn't technically incorrect, but not very useful either. >>> but all the >>> partitions must be named "partition", >> >> Bad binding, no cookie for you! > > Sorry, I dont understand want you want to say here. I was trying to say that if the "partition" binding says the "name" of every partition node should be "partition", then it is a very bad binding. The binding does _not_ say that though, the code is wrong. >> You cannot claim a name as generic as "partition" for this. Pick >> something else if you really must do things this way. > > You choose :) Picking good names is hard work (more than 50% of all programming, heh). You choose :-) [Hint: something with a comma, maybe "linux,mtd,partition" or similar -- I don't really care. OTOH, it seems you won't need this at all]. Segher
Hello > > Ah! After writing my original reply, I read the documentation, > and this had me confused. > > 4b08e14 looks wrong to me. It also didn't update the documentation. So, what shall we do, revert 4b08e14 or apply this patch changing the name to "linux,mtd,partition" or similar.... ? Regards
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c index 3e164f0..59c1e4a 100644 --- a/drivers/mtd/ofpart.c +++ b/drivers/mtd/ofpart.c @@ -48,7 +48,9 @@ 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 ((strcmp(partname, "partition") != 0) && + (of_device_is_compatible(pp, "partition") != 1)) + { nr_parts--; continue; }