RE: [PATCH] block: support embedded device command line partition
> I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly > what we need. So needed that we proposed it in 2010: > http://marc.info/?l=linux-kernel=127425650923757=2 > http://marc.info/?l=linux-kernel=127599718024364=2 > > Is this patch inspired by Ulfs patch? (I can see the code is different, > but the cmdline argument is the same for example.) Some credit > could have been proper in that case. I have not searched other patch, English reading/writing is not an easy thing for me. Thank Ulf Hansson. He was the first one who thought of this method. > > Just asking: sometimes wheels do get reinvented. > > Yours, > Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, Aug 3, 2013 at 11:57 AM, Caizhiyong wrote: > From: Cai Zhiyong > > Read block device partition table from command line. > The partition used for fixed block device (eMMC) embedded device. > It is no MBR, save storage space. Bootloader can be easily accessed by > absolute address of data on the block device. > Users can easily change the partition. > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c" > About the partition verbose reference > "Documentation/block/cmdline-partition.txt" > > Signed-off-by: Cai Zhiyong I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly what we need. So needed that we proposed it in 2010: http://marc.info/?l=linux-kernel=127425650923757=2 http://marc.info/?l=linux-kernel=127599718024364=2 Is this patch inspired by Ulfs patch? (I can see the code is different, but the cmdline argument is the same for example.) Some credit could have been proper in that case. Just asking: sometimes wheels do get reinvented. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, Aug 3, 2013 at 11:57 AM, Caizhiyong caizhiy...@huawei.com wrote: From: Cai Zhiyong caizhiy...@huawei.com Read block device partition table from command line. The partition used for fixed block device (eMMC) embedded device. It is no MBR, save storage space. Bootloader can be easily accessed by absolute address of data on the block device. Users can easily change the partition. This code reference MTD partition, source drivers/mtd/cmdlinepart.c About the partition verbose reference Documentation/block/cmdline-partition.txt Signed-off-by: Cai Zhiyong caizhiy...@huawei.com I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly what we need. So needed that we proposed it in 2010: http://marc.info/?l=linux-kernelm=127425650923757w=2 http://marc.info/?l=linux-kernelm=127599718024364w=2 Is this patch inspired by Ulfs patch? (I can see the code is different, but the cmdline argument is the same for example.) Some credit could have been proper in that case. Just asking: sometimes wheels do get reinvented. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
I saw this patch appear in kernel v3.12-rc1, and it's nice since it's exactly what we need. So needed that we proposed it in 2010: http://marc.info/?l=linux-kernelm=127425650923757w=2 http://marc.info/?l=linux-kernelm=127599718024364w=2 Is this patch inspired by Ulfs patch? (I can see the code is different, but the cmdline argument is the same for example.) Some credit could have been proper in that case. I have not searched other patch, English reading/writing is not an easy thing for me. Thank Ulf Hansson. He was the first one who thought of this method. Just asking: sometimes wheels do get reinvented. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/19/2013 02:36 AM, Caizhiyong wrote: >> On 08/15/2013 08:54 PM, Caizhiyong wrote: > +blkdevparts=[;] > + := :[,] > + := [@](part-name) > + > + > +block device disk name, embedded device used fixed block device, > +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? >>> >>> Yes. Fixed is for single controller. >>> For multiple controllers, currently, there is not a simple way. >>> I tend to do something in the eMMC driver, such as initialize order, >>> but I have not tried. >> >> There have been proposals before to try and create a fixed naming for >> the controllers (or rather the block devices they generate...) but >> they've been rejected. I don't think we should rely on being able to do >> that. >> > + > + > +partition start address, in bytes. > + > +(part-name) > +partition name, kernel send uevent with "PARTNAME". application can > create > +a link to block device partition with the name "PARTNAME". > +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. >>> >>> If no need partname, your bootargs are mmcblk0:1G,1G,1G,... >> >> Well, you always need the offset too. I don't think there's any harm in >> forcing all fields to be specified in all cases; it makes the whole >> system much more regular and less error-prone. >> >> Alternatively, use a different separator between fields for a given >> partition, and between partitions, e.g.: >> >> size,offset,partname;size,offset,partname >> >> That way, you know that if you see a ; you're looking at a new >> partition, and hence the partname field need not always be specified. >> Although, if you want to specify a partname but not an offset you'd >> still need empty fields, so just requiring all fields to always be >> present still seems safest to me. > > I just follow MTD cmdline partition format.(reference > drivers/mtd/cmdlinepart.c) Ah OK, consistency with an existing format used for similar purposes probably does override any other concerns. > There are many pitfalls in using this partition format, the designer is more > consideration its ease of use, rather than safe. > There is an other conversation: https://lkml.org/lkml/2013/8/3/16 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
> On 08/15/2013 08:54 PM, Caizhiyong wrote: > >>> +blkdevparts=[;] > >>> + := :[,] > >>> + := [@](part-name) > >>> + > >>> + > >>> +block device disk name, embedded device used fixed block device, > >>> +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. > >> > >> The device-name isn't always fixed. > >> > >> For example, what if there are multiple SDHCI controllers, one hosting a > >> fixed eMMC device and the other an SD card? It's quite typical for the > >> eMMC's device name (which is likely what should be affected by this > >> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when > >> an SD card is present. Is there a more precise/stable way to define > >> which device the command-line option applies to? > > > > Yes. Fixed is for single controller. > > For multiple controllers, currently, there is not a simple way. > > I tend to do something in the eMMC driver, such as initialize order, > > but I have not tried. > > There have been proposals before to try and create a fixed naming for > the controllers (or rather the block devices they generate...) but > they've been rejected. I don't think we should rely on being able to do > that. > > >>> + > >>> + > >>> +partition start address, in bytes. > >>> + > >>> +(part-name) > >>> +partition name, kernel send uevent with "PARTNAME". application can > >>> create > >>> +a link to block device partition with the name "PARTNAME". > >>> +user space application can access partition by partition name. > >> > >> Do partitions usually have a PARTNAME attribute when probed through > >> normal mechanisms like MBR? If so, I guess this is fine. > >> > >> Perhaps we can just use , as the delimiter for all fields, rather than > >> special-casing part-name to use (), so: > >> > >> size,offset,partname,size,offset,partname,... > >> > >> The partname field could easily be empty if not needed. > > > > If no need partname, your bootargs are mmcblk0:1G,1G,1G,... > > Well, you always need the offset too. I don't think there's any harm in > forcing all fields to be specified in all cases; it makes the whole > system much more regular and less error-prone. > > Alternatively, use a different separator between fields for a given > partition, and between partitions, e.g.: > > size,offset,partname;size,offset,partname > > That way, you know that if you see a ; you're looking at a new > partition, and hence the partname field need not always be specified. > Although, if you want to specify a partname but not an offset you'd > still need empty fields, so just requiring all fields to always be > present still seems safest to me. I just follow MTD cmdline partition format.(reference drivers/mtd/cmdlinepart.c) There are many pitfalls in using this partition format, the designer is more consideration its ease of use, rather than safe. There is an other conversation: https://lkml.org/lkml/2013/8/3/16 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
On 08/15/2013 08:54 PM, Caizhiyong wrote: +blkdevparts=blkdev-def[;blkdev-def] + blkdev-def := blkdev-id:partdef[,partdef] +partdef := size[@offset](part-name) + +blkdev-id +block device disk name, embedded device used fixed block device, +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? Yes. Fixed is for single controller. For multiple controllers, currently, there is not a simple way. I tend to do something in the eMMC driver, such as initialize order, but I have not tried. There have been proposals before to try and create a fixed naming for the controllers (or rather the block devices they generate...) but they've been rejected. I don't think we should rely on being able to do that. + +offset +partition start address, in bytes. + +(part-name) +partition name, kernel send uevent with PARTNAME. application can create +a link to block device partition with the name PARTNAME. +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. If no need partname, your bootargs are mmcblk0:1G,1G,1G,... Well, you always need the offset too. I don't think there's any harm in forcing all fields to be specified in all cases; it makes the whole system much more regular and less error-prone. Alternatively, use a different separator between fields for a given partition, and between partitions, e.g.: size,offset,partname;size,offset,partname That way, you know that if you see a ; you're looking at a new partition, and hence the partname field need not always be specified. Although, if you want to specify a partname but not an offset you'd still need empty fields, so just requiring all fields to always be present still seems safest to me. I just follow MTD cmdline partition format.(reference drivers/mtd/cmdlinepart.c) There are many pitfalls in using this partition format, the designer is more consideration its ease of use, rather than safe. There is an other conversation: https://lkml.org/lkml/2013/8/3/16 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/19/2013 02:36 AM, Caizhiyong wrote: On 08/15/2013 08:54 PM, Caizhiyong wrote: +blkdevparts=blkdev-def[;blkdev-def] + blkdev-def := blkdev-id:partdef[,partdef] +partdef := size[@offset](part-name) + +blkdev-id +block device disk name, embedded device used fixed block device, +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? Yes. Fixed is for single controller. For multiple controllers, currently, there is not a simple way. I tend to do something in the eMMC driver, such as initialize order, but I have not tried. There have been proposals before to try and create a fixed naming for the controllers (or rather the block devices they generate...) but they've been rejected. I don't think we should rely on being able to do that. + +offset +partition start address, in bytes. + +(part-name) +partition name, kernel send uevent with PARTNAME. application can create +a link to block device partition with the name PARTNAME. +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. If no need partname, your bootargs are mmcblk0:1G,1G,1G,... Well, you always need the offset too. I don't think there's any harm in forcing all fields to be specified in all cases; it makes the whole system much more regular and less error-prone. Alternatively, use a different separator between fields for a given partition, and between partitions, e.g.: size,offset,partname;size,offset,partname That way, you know that if you see a ; you're looking at a new partition, and hence the partname field need not always be specified. Although, if you want to specify a partname but not an offset you'd still need empty fields, so just requiring all fields to always be present still seems safest to me. I just follow MTD cmdline partition format.(reference drivers/mtd/cmdlinepart.c) Ah OK, consistency with an existing format used for similar purposes probably does override any other concerns. There are many pitfalls in using this partition format, the designer is more consideration its ease of use, rather than safe. There is an other conversation: https://lkml.org/lkml/2013/8/3/16 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/15/2013 08:54 PM, Caizhiyong wrote: >>> +blkdevparts=[;] >>> + := :[,] >>> + := [@](part-name) >>> + >>> + >>> +block device disk name, embedded device used fixed block device, >>> +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. >> >> The device-name isn't always fixed. >> >> For example, what if there are multiple SDHCI controllers, one hosting a >> fixed eMMC device and the other an SD card? It's quite typical for the >> eMMC's device name (which is likely what should be affected by this >> feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when >> an SD card is present. Is there a more precise/stable way to define >> which device the command-line option applies to? > > Yes. Fixed is for single controller. > For multiple controllers, currently, there is not a simple way. > I tend to do something in the eMMC driver, such as initialize order, > but I have not tried. There have been proposals before to try and create a fixed naming for the controllers (or rather the block devices they generate...) but they've been rejected. I don't think we should rely on being able to do that. >>> + >>> + >>> +partition start address, in bytes. >>> + >>> +(part-name) >>> +partition name, kernel send uevent with "PARTNAME". application can >>> create >>> +a link to block device partition with the name "PARTNAME". >>> +user space application can access partition by partition name. >> >> Do partitions usually have a PARTNAME attribute when probed through >> normal mechanisms like MBR? If so, I guess this is fine. >> >> Perhaps we can just use , as the delimiter for all fields, rather than >> special-casing part-name to use (), so: >> >> size,offset,partname,size,offset,partname,... >> >> The partname field could easily be empty if not needed. > > If no need partname, your bootargs are mmcblk0:1G,1G,1G,... Well, you always need the offset too. I don't think there's any harm in forcing all fields to be specified in all cases; it makes the whole system much more regular and less error-prone. Alternatively, use a different separator between fields for a given partition, and between partitions, e.g.: size,offset,partname;size,offset,partname That way, you know that if you see a ; you're looking at a new partition, and hence the partname field need not always be specified. Although, if you want to specify a partname but not an offset you'd still need empty fields, so just requiring all fields to always be present still seems safest to me. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/15/2013 08:54 PM, Caizhiyong wrote: +blkdevparts=blkdev-def[;blkdev-def] + blkdev-def := blkdev-id:partdef[,partdef] +partdef := size[@offset](part-name) + +blkdev-id +block device disk name, embedded device used fixed block device, +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? Yes. Fixed is for single controller. For multiple controllers, currently, there is not a simple way. I tend to do something in the eMMC driver, such as initialize order, but I have not tried. There have been proposals before to try and create a fixed naming for the controllers (or rather the block devices they generate...) but they've been rejected. I don't think we should rely on being able to do that. + +offset +partition start address, in bytes. + +(part-name) +partition name, kernel send uevent with PARTNAME. application can create +a link to block device partition with the name PARTNAME. +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. If no need partname, your bootargs are mmcblk0:1G,1G,1G,... Well, you always need the offset too. I don't think there's any harm in forcing all fields to be specified in all cases; it makes the whole system much more regular and less error-prone. Alternatively, use a different separator between fields for a given partition, and between partitions, e.g.: size,offset,partname;size,offset,partname That way, you know that if you see a ; you're looking at a new partition, and hence the partname field need not always be specified. Although, if you want to specify a partname but not an offset you'd still need empty fields, so just requiring all fields to always be present still seems safest to me. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
> > +blkdevparts=[;] > > + := :[,] > > + := [@](part-name) > > + > > + > > +block device disk name, embedded device used fixed block device, > > +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. > > The device-name isn't always fixed. > > For example, what if there are multiple SDHCI controllers, one hosting a > fixed eMMC device and the other an SD card? It's quite typical for the > eMMC's device name (which is likely what should be affected by this > feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when > an SD card is present. Is there a more precise/stable way to define > which device the command-line option applies to? Yes. Fixed is for single controller. For multiple controllers, currently, there is not a simple way. I tend to do something in the eMMC driver, such as initialize order, but I have not tried. > > For the root= command-line option, we use UUID= or PARTUUID= to work > around this issue, but that won't work for naming the whole device. > > What about using the MMIO address of the controller hosting the device, > or something like that? > MMIO may be not a good idea, it is not intuitive. > > + > > +partition size, in bytes, such as: 512, 1m, 1G. > > s/m/M/? > > I assume M/G are MiB/GiB? > > Does it make sense to allow specifying the sizes in units of > sectors/blocks? If so, which specific block/sector size in the case of > things like NAND flash which IIRC have multiple types/levels of sectors; > perhaps this is a bad idea. I guess we could easily add this feature > later by introducing a new suffix, so no need to solve the issue now. > > > + > > + > > +partition start address, in bytes. > > + > > +(part-name) > > +partition name, kernel send uevent with "PARTNAME". application can > > create > > +a link to block device partition with the name "PARTNAME". > > +user space application can access partition by partition name. > > Do partitions usually have a PARTNAME attribute when probed through > normal mechanisms like MBR? If so, I guess this is fine. > > Perhaps we can just use , as the delimiter for all fields, rather than > special-casing part-name to use (), so: > > size,offset,partname,size,offset,partname,... > > The partname field could easily be empty if not needed. > If no need partname, your bootargs are mmcblk0:1G,1G,1G,... > > +Example: > > +eMMC disk name is "mmcblk0" and "mmcblk0boot0" > > + > > + bootargs: > > + > 'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)' > > + > > + dmesg: > > +mmcblk0: p1(data0) p2(data1) p3() > > +mmcblk0boot0: p1(boot) p2(kernel) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/03/2013 03:57 AM, Caizhiyong wrote: > From: Cai Zhiyong > > Read block device partition table from command line. > The partition used for fixed block device (eMMC) embedded device. > It is no MBR, save storage space. Bootloader can be easily accessed by > absolute address of data on the block device. > Users can easily change the partition. > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c" > About the partition verbose reference > "Documentation/block/cmdline-partition.txt" Interesting. NVIDIA Tegra devices running our downstream kernels have a "tegrapt" (for "Tegra Partition Table") command-line option for basically the same purpose. While I'd personally prefer we move our eMMC layout towards standardized GPT, the feature implemented by this patch may well provide an easy migration/co-existence path instead, so could be quite useful. > diff --git a/Documentation/block/cmdline-partition.txt > b/Documentation/block/cmdline-partition.txt > +Read block device partition table from command line. > +The partition used for fixed block device (eMMC) embedded device. > +It is no MBR, save storage space. Bootloader can be easily accessed > +by absolute address of data on the block device. > +Users can easily change the partition. > + > +The format for the command line is just like mtdparts: > + > +blkdevparts=[;] > + := :[,] > + := [@](part-name) > + > + > +block device disk name, embedded device used fixed block device, > +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? For the root= command-line option, we use UUID= or PARTUUID= to work around this issue, but that won't work for naming the whole device. What about using the MMIO address of the controller hosting the device, or something like that? > + > +partition size, in bytes, such as: 512, 1m, 1G. s/m/M/? I assume M/G are MiB/GiB? Does it make sense to allow specifying the sizes in units of sectors/blocks? If so, which specific block/sector size in the case of things like NAND flash which IIRC have multiple types/levels of sectors; perhaps this is a bad idea. I guess we could easily add this feature later by introducing a new suffix, so no need to solve the issue now. > + > + > +partition start address, in bytes. > + > +(part-name) > +partition name, kernel send uevent with "PARTNAME". application can > create > +a link to block device partition with the name "PARTNAME". > +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. > +Example: > +eMMC disk name is "mmcblk0" and "mmcblk0boot0" > + > + bootargs: > + > 'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)' > + > + dmesg: > +mmcblk0: p1(data0) p2(data1) p3() > +mmcblk0boot0: p1(boot) p2(kernel) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On 08/03/2013 03:57 AM, Caizhiyong wrote: From: Cai Zhiyong caizhiy...@huawei.com Read block device partition table from command line. The partition used for fixed block device (eMMC) embedded device. It is no MBR, save storage space. Bootloader can be easily accessed by absolute address of data on the block device. Users can easily change the partition. This code reference MTD partition, source drivers/mtd/cmdlinepart.c About the partition verbose reference Documentation/block/cmdline-partition.txt Interesting. NVIDIA Tegra devices running our downstream kernels have a tegrapt (for Tegra Partition Table) command-line option for basically the same purpose. While I'd personally prefer we move our eMMC layout towards standardized GPT, the feature implemented by this patch may well provide an easy migration/co-existence path instead, so could be quite useful. diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt +Read block device partition table from command line. +The partition used for fixed block device (eMMC) embedded device. +It is no MBR, save storage space. Bootloader can be easily accessed +by absolute address of data on the block device. +Users can easily change the partition. + +The format for the command line is just like mtdparts: + +blkdevparts=blkdev-def[;blkdev-def] + blkdev-def := blkdev-id:partdef[,partdef] +partdef := size[@offset](part-name) + +blkdev-id +block device disk name, embedded device used fixed block device, +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? For the root= command-line option, we use UUID= or PARTUUID= to work around this issue, but that won't work for naming the whole device. What about using the MMIO address of the controller hosting the device, or something like that? +size +partition size, in bytes, such as: 512, 1m, 1G. s/m/M/? I assume M/G are MiB/GiB? Does it make sense to allow specifying the sizes in units of sectors/blocks? If so, which specific block/sector size in the case of things like NAND flash which IIRC have multiple types/levels of sectors; perhaps this is a bad idea. I guess we could easily add this feature later by introducing a new suffix, so no need to solve the issue now. + +offset +partition start address, in bytes. + +(part-name) +partition name, kernel send uevent with PARTNAME. application can create +a link to block device partition with the name PARTNAME. +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. +Example: +eMMC disk name is mmcblk0 and mmcblk0boot0 + + bootargs: + 'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)' + + dmesg: +mmcblk0: p1(data0) p2(data1) p3() +mmcblk0boot0: p1(boot) p2(kernel) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
+blkdevparts=blkdev-def[;blkdev-def] + blkdev-def := blkdev-id:partdef[,partdef] +partdef := size[@offset](part-name) + +blkdev-id +block device disk name, embedded device used fixed block device, +it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. The device-name isn't always fixed. For example, what if there are multiple SDHCI controllers, one hosting a fixed eMMC device and the other an SD card? It's quite typical for the eMMC's device name (which is likely what should be affected by this feature) to be mmcblk0 when no SD card is present, yet be mmcblk1 when an SD card is present. Is there a more precise/stable way to define which device the command-line option applies to? Yes. Fixed is for single controller. For multiple controllers, currently, there is not a simple way. I tend to do something in the eMMC driver, such as initialize order, but I have not tried. For the root= command-line option, we use UUID= or PARTUUID= to work around this issue, but that won't work for naming the whole device. What about using the MMIO address of the controller hosting the device, or something like that? MMIO may be not a good idea, it is not intuitive. +size +partition size, in bytes, such as: 512, 1m, 1G. s/m/M/? I assume M/G are MiB/GiB? Does it make sense to allow specifying the sizes in units of sectors/blocks? If so, which specific block/sector size in the case of things like NAND flash which IIRC have multiple types/levels of sectors; perhaps this is a bad idea. I guess we could easily add this feature later by introducing a new suffix, so no need to solve the issue now. + +offset +partition start address, in bytes. + +(part-name) +partition name, kernel send uevent with PARTNAME. application can create +a link to block device partition with the name PARTNAME. +user space application can access partition by partition name. Do partitions usually have a PARTNAME attribute when probed through normal mechanisms like MBR? If so, I guess this is fine. Perhaps we can just use , as the delimiter for all fields, rather than special-casing part-name to use (), so: size,offset,partname,size,offset,partname,... The partname field could easily be empty if not needed. If no need partname, your bootargs are mmcblk0:1G,1G,1G,... +Example: +eMMC disk name is mmcblk0 and mmcblk0boot0 + + bootargs: + 'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)' + + dmesg: +mmcblk0: p1(data0) p2(data1) p3() +mmcblk0boot0: p1(boot) p2(kernel) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Tue, 6 Aug 2013 10:53:01 + Caizhiyong wrote: > > -Original Message- > > From: Andrew Morton [mailto:a...@linux-foundation.org] > > Sent: Tuesday, August 06, 2013 6:22 AM > > To: Caizhiyong > > Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; > > Jens Axboe; > > David Woodhouse; Marius Groeger > > Subject: Re: [PATCH] block: support embedded device command line partition > > > > On Sat, 3 Aug 2013 09:57:04 + Caizhiyong wrote: > > > > > From: Cai Zhiyong > > > > > > Read block device partition table from command line. > > > The partition used for fixed block device (eMMC) embedded device. > > > It is no MBR, save storage space. Bootloader can be easily accessed by > > > absolute address of data on the block device. > > > Users can easily change the partition. > > > > > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c" > > > About the partition verbose reference > > "Documentation/block/cmdline-partition.txt" > > > > > > > Seems OK to me. > > > > The obvious question is: can we consolidate this with > > drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have > > two parsers which do the same thing? > > I will move the parser to "lib/cmdline.c". > block/cmdline.c is an appropriate place for block-subsystem library code. But simply moving the file doesn't achieve anything. To prove that the code is indeed library-style code, we should actually *use* it in some client code. Such as MTD! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
> -Original Message- > From: Andrew Morton [mailto:a...@linux-foundation.org] > Sent: Tuesday, August 06, 2013 6:22 AM > To: Caizhiyong > Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; Jens > Axboe; > David Woodhouse; Marius Groeger > Subject: Re: [PATCH] block: support embedded device command line partition > > On Sat, 3 Aug 2013 09:57:04 + Caizhiyong wrote: > > > From: Cai Zhiyong > > > > Read block device partition table from command line. > > The partition used for fixed block device (eMMC) embedded device. > > It is no MBR, save storage space. Bootloader can be easily accessed by > > absolute address of data on the block device. > > Users can easily change the partition. > > > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c" > > About the partition verbose reference > "Documentation/block/cmdline-partition.txt" > > > > Seems OK to me. > > The obvious question is: can we consolidate this with > drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have > two parsers which do the same thing? I will move the parser to "lib/cmdline.c". Looking forward to your feedback. Best regards, Cai Zhiyong. http://www.huawei.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
-Original Message- From: Andrew Morton [mailto:a...@linux-foundation.org] Sent: Tuesday, August 06, 2013 6:22 AM To: Caizhiyong Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; Jens Axboe; David Woodhouse; Marius Groeger Subject: Re: [PATCH] block: support embedded device command line partition On Sat, 3 Aug 2013 09:57:04 + Caizhiyong caizhiy...@huawei.com wrote: From: Cai Zhiyong caizhiy...@huawei.com Read block device partition table from command line. The partition used for fixed block device (eMMC) embedded device. It is no MBR, save storage space. Bootloader can be easily accessed by absolute address of data on the block device. Users can easily change the partition. This code reference MTD partition, source drivers/mtd/cmdlinepart.c About the partition verbose reference Documentation/block/cmdline-partition.txt Seems OK to me. The obvious question is: can we consolidate this with drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have two parsers which do the same thing? I will move the parser to lib/cmdline.c. Looking forward to your feedback. Best regards, Cai Zhiyong. http://www.huawei.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Tue, 6 Aug 2013 10:53:01 + Caizhiyong caizhiy...@huawei.com wrote: -Original Message- From: Andrew Morton [mailto:a...@linux-foundation.org] Sent: Tuesday, August 06, 2013 6:22 AM To: Caizhiyong Cc: Karel Zak; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin; Jens Axboe; David Woodhouse; Marius Groeger Subject: Re: [PATCH] block: support embedded device command line partition On Sat, 3 Aug 2013 09:57:04 + Caizhiyong caizhiy...@huawei.com wrote: From: Cai Zhiyong caizhiy...@huawei.com Read block device partition table from command line. The partition used for fixed block device (eMMC) embedded device. It is no MBR, save storage space. Bootloader can be easily accessed by absolute address of data on the block device. Users can easily change the partition. This code reference MTD partition, source drivers/mtd/cmdlinepart.c About the partition verbose reference Documentation/block/cmdline-partition.txt Seems OK to me. The obvious question is: can we consolidate this with drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have two parsers which do the same thing? I will move the parser to lib/cmdline.c. block/cmdline.c is an appropriate place for block-subsystem library code. But simply moving the file doesn't achieve anything. To prove that the code is indeed library-style code, we should actually *use* it in some client code. Such as MTD! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, 3 Aug 2013 09:57:04 + Caizhiyong wrote: > From: Cai Zhiyong > > Read block device partition table from command line. > The partition used for fixed block device (eMMC) embedded device. > It is no MBR, save storage space. Bootloader can be easily accessed by > absolute address of data on the block device. > Users can easily change the partition. > > This code reference MTD partition, source "drivers/mtd/cmdlinepart.c" > About the partition verbose reference > "Documentation/block/cmdline-partition.txt" > Seems OK to me. The obvious question is: can we consolidate this with drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have two parsers which do the same thing? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, 3 Aug 2013 09:57:04 + Caizhiyong caizhiy...@huawei.com wrote: From: Cai Zhiyong caizhiy...@huawei.com Read block device partition table from command line. The partition used for fixed block device (eMMC) embedded device. It is no MBR, save storage space. Bootloader can be easily accessed by absolute address of data on the block device. Users can easily change the partition. This code reference MTD partition, source drivers/mtd/cmdlinepart.c About the partition verbose reference Documentation/block/cmdline-partition.txt Seems OK to me. The obvious question is: can we consolidate this with drivers/mtd/cmdlinepart.c in some fashion so the kernel doesn't have two parsers which do the same thing? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
> From: Karel Zak [mailto:k...@redhat.com]
> Sent: Wednesday, July 31, 2013 9:25 PM
> To: Caizhiyong
> Cc: Andrew Morton; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin
> Subject: Re: [PATCH] block: support embedded device command line partition
>
> On Sat, Jul 27, 2013 at 01:56:24PM +, Caizhiyong wrote:
> > +static int parse_partitions(struct parsed_partitions *state,
> > + struct cmdline_parts *parts)
> > +{
> > + int slot;
> > + uint64_t from = 0;
> > + uint64_t disk_size;
> > + char buf[BDEVNAME_SIZE];
> > + struct cmdline_subpart *subpart;
> > +
> > + bdevname(state->bdev, buf);
> > +
> > + while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
> > + parts = parts->next_parts;
> > +
> > + if (!parts)
> > + return 0;
> > +
> > + disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;
> > +
> > + for (slot = 1, subpart = parts->subpart;
> > +subpart && slot < state->limit;
> > +subpart = subpart->next_subpart, slot++) {
> > +
> > + unsigned label_max;
> > + struct partition_meta_info *info;
> > +
> > + if (subpart->from == OFFSET_CONTINUOUS)
> > + subpart->from = from;
> > + else
> > + from = subpart->from;
> > +
> > + if (from >= disk_size)
> > + break;
> > +
> > + if (subpart->size > (disk_size - from))
> > + subpart->size = disk_size - from;
> > +
> > + from += subpart->size;
> > +
> > + put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
> > + le64_to_cpu(subpart->size >> 9));
>
> Why le64_to_cpu()?
>
> I guess that memparse() does not return explicitly LE and it also
> seems that your code on another places uses ->size and ->from
> without any extra care.
Right. I will remove.
>
> Karel
>
> --
> Karel Zak
> http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, Jul 27, 2013 at 01:56:24PM +, Caizhiyong wrote:
> +static int parse_partitions(struct parsed_partitions *state,
> + struct cmdline_parts *parts)
> +{
> + int slot;
> + uint64_t from = 0;
> + uint64_t disk_size;
> + char buf[BDEVNAME_SIZE];
> + struct cmdline_subpart *subpart;
> +
> + bdevname(state->bdev, buf);
> +
> + while (parts && strncmp(buf, parts->name, BDEVNAME_SIZE))
> + parts = parts->next_parts;
> +
> + if (!parts)
> + return 0;
> +
> + disk_size = (uint64_t) get_capacity(state->bdev->bd_disk) << 9;
> +
> + for (slot = 1, subpart = parts->subpart;
> + subpart && slot < state->limit;
> + subpart = subpart->next_subpart, slot++) {
> +
> + unsigned label_max;
> + struct partition_meta_info *info;
> +
> + if (subpart->from == OFFSET_CONTINUOUS)
> + subpart->from = from;
> + else
> + from = subpart->from;
> +
> + if (from >= disk_size)
> + break;
> +
> + if (subpart->size > (disk_size - from))
> + subpart->size = disk_size - from;
> +
> + from += subpart->size;
> +
> + put_partition(state, slot, le64_to_cpu(subpart->from >> 9),
> + le64_to_cpu(subpart->size >> 9));
Why le64_to_cpu()?
I guess that memparse() does not return explicitly LE and it also
seems that your code on another places uses ->size and ->from
without any extra care.
Karel
--
Karel Zak
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, Jul 27, 2013 at 01:56:24PM +, Caizhiyong wrote:
+static int parse_partitions(struct parsed_partitions *state,
+ struct cmdline_parts *parts)
+{
+ int slot;
+ uint64_t from = 0;
+ uint64_t disk_size;
+ char buf[BDEVNAME_SIZE];
+ struct cmdline_subpart *subpart;
+
+ bdevname(state-bdev, buf);
+
+ while (parts strncmp(buf, parts-name, BDEVNAME_SIZE))
+ parts = parts-next_parts;
+
+ if (!parts)
+ return 0;
+
+ disk_size = (uint64_t) get_capacity(state-bdev-bd_disk) 9;
+
+ for (slot = 1, subpart = parts-subpart;
+ subpart slot state-limit;
+ subpart = subpart-next_subpart, slot++) {
+
+ unsigned label_max;
+ struct partition_meta_info *info;
+
+ if (subpart-from == OFFSET_CONTINUOUS)
+ subpart-from = from;
+ else
+ from = subpart-from;
+
+ if (from = disk_size)
+ break;
+
+ if (subpart-size (disk_size - from))
+ subpart-size = disk_size - from;
+
+ from += subpart-size;
+
+ put_partition(state, slot, le64_to_cpu(subpart-from 9),
+ le64_to_cpu(subpart-size 9));
Why le64_to_cpu()?
I guess that memparse() does not return explicitly LE and it also
seems that your code on another places uses -size and -from
without any extra care.
Karel
--
Karel Zak k...@redhat.com
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] block: support embedded device command line partition
From: Karel Zak [mailto:k...@redhat.com]
Sent: Wednesday, July 31, 2013 9:25 PM
To: Caizhiyong
Cc: Andrew Morton; linux-kernel@vger.kernel.org; Wanglin (Albert); Quyaxin
Subject: Re: [PATCH] block: support embedded device command line partition
On Sat, Jul 27, 2013 at 01:56:24PM +, Caizhiyong wrote:
+static int parse_partitions(struct parsed_partitions *state,
+ struct cmdline_parts *parts)
+{
+ int slot;
+ uint64_t from = 0;
+ uint64_t disk_size;
+ char buf[BDEVNAME_SIZE];
+ struct cmdline_subpart *subpart;
+
+ bdevname(state-bdev, buf);
+
+ while (parts strncmp(buf, parts-name, BDEVNAME_SIZE))
+ parts = parts-next_parts;
+
+ if (!parts)
+ return 0;
+
+ disk_size = (uint64_t) get_capacity(state-bdev-bd_disk) 9;
+
+ for (slot = 1, subpart = parts-subpart;
+subpart slot state-limit;
+subpart = subpart-next_subpart, slot++) {
+
+ unsigned label_max;
+ struct partition_meta_info *info;
+
+ if (subpart-from == OFFSET_CONTINUOUS)
+ subpart-from = from;
+ else
+ from = subpart-from;
+
+ if (from = disk_size)
+ break;
+
+ if (subpart-size (disk_size - from))
+ subpart-size = disk_size - from;
+
+ from += subpart-size;
+
+ put_partition(state, slot, le64_to_cpu(subpart-from 9),
+ le64_to_cpu(subpart-size 9));
Why le64_to_cpu()?
I guess that memparse() does not return explicitly LE and it also
seems that your code on another places uses -size and -from
without any extra care.
Right. I will remove.
Karel
--
Karel Zak k...@redhat.com
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: support embedded device command line partition
On Sat, 27 Jul 2013 13:56:24 + Caizhiyong wrote:
> From: Cai Zhiyong
>
> Read block device partition table from command line. This partition used for
> fixed block device (eMMC) embedded device.
> It no MBR, can save storage space. Bootloader can be easily accessed by
> absolute address of data on the block device. It support partition name,
> provides partition interface.
That seems a reasonable thing to be able to do.
> This code reference MTD partition, source "./drivers/mtd/cmdlinepart.c"
> The format for the command line is just like mtdparts.
>
> Examples: eMMC disk name is "mmcblk0"
> bootargs
> 'cmdlineparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
We should document this user interface properly. Is there
documentation under Documentation/ which can be referenced? If not,
something new should be created.
>
> ...
>
> +struct cmdline_parts {
> + char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
> + struct cmdline_subpart *subpart;
> + struct cmdline_parts *next_parts;
> +};
> +
> +static DEFINE_MUTEX(cmdline_string_mutex);
> +
> +static char cmdline_string[512] = { 0 }; static struct cmdline_parts
> +*cmdline_parts;
That's messed up.
> +static int parse_subpart(struct cmdline_subpart **subpart, char
> +*cmdline) {
Please convert all the function definitions to standard kernel style:
static int parse_subpart(struct cmdline_subpart **subpart, char *cmdline)
{
> + int ret = 0;
> + struct cmdline_subpart *new_subpart;
> +
> + (*subpart) = NULL;
> +
> + new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
> + if (!new_subpart)
> + return -ENOMEM;
> +
> + if ((*cmdline) == '-') {
> + new_subpart->size = SIZE_REMAINING;
> + cmdline++;
> + } else {
> + new_subpart->size = memparse(cmdline, );
> + if (new_subpart->size < PAGE_SIZE) {
> + pr_warn("cmdline partition size is invalid.");
> + ret = -EFAULT;
EFAULT is inappropriate here. EINVAL would be suitable.
> + goto fail;
> + }
> + }
> +
> + if ((*cmdline) == '@') {
> + cmdline++;
> + new_subpart->from = memparse(cmdline, );
> + } else {
> + new_subpart->from = OFFSET_CONTINUOUS;
> + }
> +
> + if ((*cmdline) == '(') {
> +
Remove this newline.
> + int length;
> + char *next = strchr(++cmdline, ')');
> +
> + if (!next) {
> + pr_warn("cmdline partition format is invalid.");
> + ret = -EFAULT;
EINVAL
> + goto fail;
> + }
> +
> + length = min((int)(next - cmdline),
> + (int)(sizeof(new_subpart->name) - 1));
OK, that's pretty ghastly.
Ideally, the types of the various variable should be compatible, so no
casting is needed.
If that is really truly not practical then use min_t rather than
open-coding the casts.
> + strncpy(new_subpart->name, cmdline, length);
> + new_subpart->name[length] = '\0';
> +
> + cmdline = ++next;
> + } else
> + new_subpart->name[0] = '\0';
> +
> + (*subpart) = new_subpart;
> + return 0;
> +fail:
> + kfree(new_subpart);
> + return ret;
> +}
> +
> +static void free_subpart(struct cmdline_parts *parts) {
> + struct cmdline_subpart *subpart;
> +
> + while (parts->subpart) {
> + subpart = parts->subpart;
> + parts->subpart = subpart->next_subpart;
> + kfree(subpart);
> + }
> +}
> +
> +static void free_parts(struct cmdline_parts **parts) {
> + struct cmdline_parts *next_parts;
> +
> + while ((*parts)) {
> + next_parts = (*parts)->next_parts;
> + free_subpart((*parts));
> + kfree((*parts));
> + (*parts) = next_parts;
> + }
> +}
> +
> +static int parse_parts(struct cmdline_parts **parts, const char
> +*cmdline) {
> + int ret = -EFAULT;
EINVAL?
> + char *next;
> + int length;
> + struct cmdline_subpart **next_subpart;
> + struct cmdline_parts *newparts;
> + char buf[BDEVNAME_SIZE + 32 + 4];
> +
> + (*parts) = NULL;
> +
> + newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
> + if (!newparts)
> + return -ENOMEM;
> +
> + next = strchr(cmdline, ':');
> + if (!next) {
> + pr_warn("cmdline partition has not block device.");
> + goto fail;
> + }
> +
> + length = min((int)(next - cmdline), (int)(sizeof(newparts->name) - 1));
Ditto.
> + strncpy(newparts->name, cmdline, length);
> + newparts->name[length] = '\0';
> +
> + next_subpart = >subpart;
> +
> + while (next && *(++next)) {
> +
Remove newline.
> + cmdline = next;
> + next = strchr(cmdline, ',');
> +
> + length = (!next) ?
Re: [PATCH] block: support embedded device command line partition
On Sat, 27 Jul 2013 13:56:24 + Caizhiyong caizhiy...@huawei.com wrote:
From: Cai Zhiyong caizhiy...@huawei.com
Read block device partition table from command line. This partition used for
fixed block device (eMMC) embedded device.
It no MBR, can save storage space. Bootloader can be easily accessed by
absolute address of data on the block device. It support partition name,
provides partition interface.
That seems a reasonable thing to be able to do.
This code reference MTD partition, source ./drivers/mtd/cmdlinepart.c
The format for the command line is just like mtdparts.
Examples: eMMC disk name is mmcblk0
bootargs
'cmdlineparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),-(kernel)'
We should document this user interface properly. Is there
documentation under Documentation/ which can be referenced? If not,
something new should be created.
...
+struct cmdline_parts {
+ char name[BDEVNAME_SIZE]; /* block device, such as 'mmcblk0' */
+ struct cmdline_subpart *subpart;
+ struct cmdline_parts *next_parts;
+};
+
+static DEFINE_MUTEX(cmdline_string_mutex);
+
+static char cmdline_string[512] = { 0 }; static struct cmdline_parts
+*cmdline_parts;
That's messed up.
+static int parse_subpart(struct cmdline_subpart **subpart, char
+*cmdline) {
Please convert all the function definitions to standard kernel style:
static int parse_subpart(struct cmdline_subpart **subpart, char *cmdline)
{
+ int ret = 0;
+ struct cmdline_subpart *new_subpart;
+
+ (*subpart) = NULL;
+
+ new_subpart = kzalloc(sizeof(struct cmdline_subpart), GFP_KERNEL);
+ if (!new_subpart)
+ return -ENOMEM;
+
+ if ((*cmdline) == '-') {
+ new_subpart-size = SIZE_REMAINING;
+ cmdline++;
+ } else {
+ new_subpart-size = memparse(cmdline, cmdline);
+ if (new_subpart-size PAGE_SIZE) {
+ pr_warn(cmdline partition size is invalid.);
+ ret = -EFAULT;
EFAULT is inappropriate here. EINVAL would be suitable.
+ goto fail;
+ }
+ }
+
+ if ((*cmdline) == '@') {
+ cmdline++;
+ new_subpart-from = memparse(cmdline, cmdline);
+ } else {
+ new_subpart-from = OFFSET_CONTINUOUS;
+ }
+
+ if ((*cmdline) == '(') {
+
Remove this newline.
+ int length;
+ char *next = strchr(++cmdline, ')');
+
+ if (!next) {
+ pr_warn(cmdline partition format is invalid.);
+ ret = -EFAULT;
EINVAL
+ goto fail;
+ }
+
+ length = min((int)(next - cmdline),
+ (int)(sizeof(new_subpart-name) - 1));
OK, that's pretty ghastly.
Ideally, the types of the various variable should be compatible, so no
casting is needed.
If that is really truly not practical then use min_t rather than
open-coding the casts.
+ strncpy(new_subpart-name, cmdline, length);
+ new_subpart-name[length] = '\0';
+
+ cmdline = ++next;
+ } else
+ new_subpart-name[0] = '\0';
+
+ (*subpart) = new_subpart;
+ return 0;
+fail:
+ kfree(new_subpart);
+ return ret;
+}
+
+static void free_subpart(struct cmdline_parts *parts) {
+ struct cmdline_subpart *subpart;
+
+ while (parts-subpart) {
+ subpart = parts-subpart;
+ parts-subpart = subpart-next_subpart;
+ kfree(subpart);
+ }
+}
+
+static void free_parts(struct cmdline_parts **parts) {
+ struct cmdline_parts *next_parts;
+
+ while ((*parts)) {
+ next_parts = (*parts)-next_parts;
+ free_subpart((*parts));
+ kfree((*parts));
+ (*parts) = next_parts;
+ }
+}
+
+static int parse_parts(struct cmdline_parts **parts, const char
+*cmdline) {
+ int ret = -EFAULT;
EINVAL?
+ char *next;
+ int length;
+ struct cmdline_subpart **next_subpart;
+ struct cmdline_parts *newparts;
+ char buf[BDEVNAME_SIZE + 32 + 4];
+
+ (*parts) = NULL;
+
+ newparts = kzalloc(sizeof(struct cmdline_parts), GFP_KERNEL);
+ if (!newparts)
+ return -ENOMEM;
+
+ next = strchr(cmdline, ':');
+ if (!next) {
+ pr_warn(cmdline partition has not block device.);
+ goto fail;
+ }
+
+ length = min((int)(next - cmdline), (int)(sizeof(newparts-name) - 1));
Ditto.
+ strncpy(newparts-name, cmdline, length);
+ newparts-name[length] = '\0';
+
+ next_subpart = newparts-subpart;
+
+ while (next *(++next)) {
+
Remove newline.
+ cmdline = next;
+ next = strchr(cmdline, ',');
+
+ length = (!next) ? (sizeof(buf) - 1) :
+ min((int)(next - cmdline), (int)(sizeof(buf) - 1));
Sort the
