RE: [PATCH] block: add command line partition parser
> -Original Message- > From: Brian Norris [mailto:computersforpe...@gmail.com] > Sent: Thursday, August 15, 2013 8:12 AM > To: Andrew Morton > Cc: Caizhiyong; Karel Zak; linux-...@lists.infradead.org; > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik > Ladkani; > Huang Shijie > Subject: Re: [PATCH] block: add command line partition parser > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton > wrote: > > On Tue, 13 Aug 2013 06:02:17 + Caizhiyong wrote: > > > >> move the command line parser to a separate module, and change it into > >> library-style code. > >> > >> reference: https://lkml.org/lkml/2013/8/6/550 > > The most recent patch is an addendum to this linked patch then? > > > Well OK. But to prove the library's usefulness and to generally clean > > up the kernel, someone needs to sign up to the task of converting > > drivers/mtd/cmdlinepart.c to use this code. > > > > I've been hopefully cc'ing various MTD people but am not being > > overwhelmed with waves of enthusiasm ;) > > "I've been" implies that you have done so prior to this email. And > "people" implies more than one person. I see that you CC'd David > Woodhouse over a week ago, but he's fairly silent these days on MTD > things. It's Artem or me who handle most of the day-to-day of MTD. And > this is the first time I've seen this! (BTW, please include > linux-...@lists.infradead.org for anything involving MTD.) > > This seems reasonable, and I'd be willing to work with this proposal. > > Caizhiyong, can you submit a clear single patch (or series of > patches), CC'd to linux-mtd at least? Then we can see about supporting > it in MTD. It doesn't look too difficult, but I need to check that it > faithfully mimics the capability we currently rely on. There have been > previous discussions on changing it, but this was rejected in favor of > allowing more flexibility. Here's part of one such conversation: > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html > > So I would recommend: > (1) consider carefully the implications of your command-line format > now, rather than later > (2) if you want MTD to use it, it needs to support the features we use now It is fully functional reference MTD, :-). > > Some particular cases to consider: overlapping partitions (how do > block devices handle overlapping partitions?), out-of-order > specification, zero sized partitions, mixed syntax (some specified > with an offset, some not), multiple '-' partitions. I think the 'offset' just is used to hide some MTD space. There are two way: 1) redefine the 'offset' as a gap between forward partition and next partition. 2) add code forbid command line partitions overlapping and out-of-order. I recommend 1), it seems to solve those problem(overlapping and out-of-order), but it will affect habit. > > Anyway, if you resend, we can review. > > Thanks, > Brian -- 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: add command line partition parser
> -Original Message- > From: Brian Norris [mailto:computersforpe...@gmail.com] > Sent: Thursday, August 15, 2013 1:00 PM > To: Caizhiyong > Cc: Andrew Morton; Karel Zak; linux-...@lists.infradead.org; > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik > Ladkani; > Huang Shijie > Subject: Re: [PATCH] block: add command line partition parser > > On Thu, Aug 15, 2013 at 03:38:47AM +, Caizhiyong wrote: > > > -Original Message- > > > From: Brian Norris [mailto:computersforpe...@gmail.com] > > > Sent: Thursday, August 15, 2013 8:12 AM > > > To: Andrew Morton > > > Cc: Caizhiyong; Karel Zak; linux-...@lists.infradead.org; > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik > Ladkani; > > > Huang Shijie > > > Subject: Re: [PATCH] block: add command line partition parser > > > > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton > > > wrote: > > > > On Tue, 13 Aug 2013 06:02:17 + Caizhiyong > > > > wrote: > > > > > > > >> move the command line parser to a separate module, and change it into > > > >> library-style code. > > > >> > > > >> reference: https://lkml.org/lkml/2013/8/6/550 > > > > > > The most recent patch is an addendum to this linked patch then? > > > > > > > Well OK. But to prove the library's usefulness and to generally clean > > > > up the kernel, someone needs to sign up to the task of converting > > > > drivers/mtd/cmdlinepart.c to use this code. > > > > > > > > I've been hopefully cc'ing various MTD people but am not being > > > > overwhelmed with waves of enthusiasm ;) > > > > > > "I've been" implies that you have done so prior to this email. And > > > "people" implies more than one person. I see that you CC'd David > > > Woodhouse over a week ago, but he's fairly silent these days on MTD > > > things. It's Artem or me who handle most of the day-to-day of MTD. And > > > this is the first time I've seen this! (BTW, please include > > > linux-...@lists.infradead.org for anything involving MTD.) > > > > > > This seems reasonable, and I'd be willing to work with this proposal. > > > > > > Caizhiyong, can you submit a clear single patch (or series of > > > patches), CC'd to linux-mtd at least? Then we can see about supporting > > > it in MTD. It doesn't look too difficult, but I need to check that it > > > faithfully mimics the capability we currently rely on. There have been > > > previous discussions on changing it, but this was rejected in favor of > > > allowing more flexibility. Here's part of one such conversation: > > > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html > > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html > > > > > > So I would recommend: > > > (1) consider carefully the implications of your command-line format > > > now, rather than later > > > (2) if you want MTD to use it, it needs to support the features we use now > > > > It is fully functional reference MTD, :-). > > I realize that. I just want to be clear that we have to reconcile (1) > and (2). IOW, if block device requirements stray too far from MTD > requirements, then we might as well drop the idea of integration now. > But if they agree, then we can move forward. > > > > Some particular cases to consider: overlapping partitions (how do > > > block devices handle overlapping partitions?), out-of-order > > > specification, zero sized partitions, mixed syntax (some specified > > > with an offset, some not), multiple '-' partitions. > > > > I think the 'offset' just is used to hide some MTD space. > > No, it specifies offset as a distance from the beginning of the flash, > so partitions can be numbered out of order. This is intentionally > utilized by some users, for example, to ensure that a particular > partition is always /dev/mtd0, even if it is not the first partition > physically. > > > There are two way: > > 1) redefine the 'offset' as a gap between forward partition and next > > partition. > > 2) add code forbid command line partitions overlapping and out-of-order. > > > > I recommend 1), it seems to solve th
RE: [PATCH] block: add command line partition parser
> -Original Message- > From: Brian Norris [mailto:computersforpe...@gmail.com] > Sent: Thursday, August 15, 2013 3:10 PM > To: Caizhiyong > Cc: Andrew Morton; Karel Zak; linux-...@lists.infradead.org; > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik > Ladkani; > Huang Shijie > Subject: Re: [PATCH] block: add command line partition parser > > On Thu, Aug 15, 2013 at 06:16:04AM +, Caizhiyong wrote: > > > -Original Message- > > > From: Brian Norris [mailto:computersforpe...@gmail.com] > > > Sent: Thursday, August 15, 2013 1:00 PM > > > To: Caizhiyong > > > Cc: Andrew Morton; Karel Zak; linux-...@lists.infradead.org; > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; Shmulik > Ladkani; > > > Huang Shijie > > > Subject: Re: [PATCH] block: add command line partition parser > > > > > > On Thu, Aug 15, 2013 at 03:38:47AM +, Caizhiyong wrote: > > > > > -Original Message- > > > > > From: Brian Norris [mailto:computersforpe...@gmail.com] > > > > > Sent: Thursday, August 15, 2013 8:12 AM > > > > > To: Andrew Morton > > > > > Cc: Caizhiyong; Karel Zak; linux-...@lists.infradead.org; > > > > > linux-kernel@vger.kernel.org; Wanglin (Albert); Artem Bityutskiy; > > > > > Shmulik > > > Ladkani; > > > > > Huang Shijie > > > > > Subject: Re: [PATCH] block: add command line partition parser > > > > > > > > > > On Wed, Aug 14, 2013 at 3:57 PM, Andrew Morton > > > > > wrote: > > > > > > On Tue, 13 Aug 2013 06:02:17 + Caizhiyong > > > > > > > wrote: > > > > > > > > > > > >> move the command line parser to a separate module, and change it > > > > > >> into > > > > > >> library-style code. > > > > > >> > > > > > >> reference: https://lkml.org/lkml/2013/8/6/550 > > > > > > > > > > The most recent patch is an addendum to this linked patch then? > > > > > > > > > > > Well OK. But to prove the library's usefulness and to generally > > > > > > clean > > > > > > up the kernel, someone needs to sign up to the task of converting > > > > > > drivers/mtd/cmdlinepart.c to use this code. > > > > > > > > > > > > I've been hopefully cc'ing various MTD people but am not being > > > > > > overwhelmed with waves of enthusiasm ;) > > > > > > > > > > "I've been" implies that you have done so prior to this email. And > > > > > "people" implies more than one person. I see that you CC'd David > > > > > Woodhouse over a week ago, but he's fairly silent these days on MTD > > > > > things. It's Artem or me who handle most of the day-to-day of MTD. And > > > > > this is the first time I've seen this! (BTW, please include > > > > > linux-...@lists.infradead.org for anything involving MTD.) > > > > > > > > > > This seems reasonable, and I'd be willing to work with this proposal. > > > > > > > > > > Caizhiyong, can you submit a clear single patch (or series of > > > > > patches), CC'd to linux-mtd at least? Then we can see about supporting > > > > > it in MTD. It doesn't look too difficult, but I need to check that it > > > > > faithfully mimics the capability we currently rely on. There have been > > > > > previous discussions on changing it, but this was rejected in favor of > > > > > allowing more flexibility. Here's part of one such conversation: > > > > > > > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-August/043599.html > > > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-September/043825.html > > > > > http://lists.infradead.org/pipermail/linux-mtd/2012-December/045322.html > > > > > > > > > > So I would recommend: > > > > > (1) consider carefully the implications of your command-line format > > > > > now, rather than later > > > > > (2) if you want MTD to use it, it needs to support the features we > > > > > use now > > > > > > > > It is fully functional reference MTD, :-). > > > > > >
[PATCH] mtd: cmdlinepart: use cmdline partition parser lib
From: Cai Zhiyong MTD cmdline partition use cmdline partition parser lib reference: https://lkml.org/lkml/2013/8/6/550 Signed-off-by: Cai ZhiYong --- Documentation/block/cmdline-partition.txt | 93 +--- block/cmdline-parser.c| 6 +- drivers/mtd/Kconfig | 1 + drivers/mtd/cmdlinepart.c | 360 -- 4 files changed, 101 insertions(+), 359 deletions(-) diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt index 651863d..adc5f7a 100644 --- a/Documentation/block/cmdline-partition.txt +++ b/Documentation/block/cmdline-partition.txt @@ -1,40 +1,59 @@ Embedded device command line partition -= - -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=[;] +Authors: Cai Zhiyong + +The format for the command line is as follows, +it is used by MTD device (reference drivers/mtd/cmdlinepart.c) and +block device (reference block/partitions/cmdline.c) + + +For MTD device: + mtdparts=[; := :[,] + := [@][][ro][lk] +:= unique name used in mapping driver/device (mtd->name) + := standard linux memsize OR "-" to denote all remaining space + size is automatically truncated at end of device + if specified or trucated size is 0 the part is skipped +:= standard linux memsize + if omitted the part will immediately follow the previous part + or 0 if the first part + := '(' NAME ')' + NAME will appear in /proc/mtd + + and can be specified such that the parts are out of order + in physical memory and may even overlap. + + The parts are assigned MTD numbers in the order they are specified in the + command line regardless of their order in physical memory. + + Examples: + + 1 NOR Flash, with 1 single writable partition: + edb7312-nor:- + + 1 NOR Flash with 2 partitions, 1 NAND with one + edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) + + +For block device: + blkdevparts=[;] := :[,] - := [@](part-name) - - -block device disk name, embedded device used fixed block device, -it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. - - -partition size, in bytes, such as: 512, 1m, 1G. - - -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. - -Example: -eMMC disk name is "mmcblk0" and "mmcblk0boot0" - - bootargs: + := [@][] + + block device disk name, embedded device used fixed block device, + it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. + := standard linux memsize OR "-" to denote all remaining space + size is automatically truncated at end of device + if specified or trucated size is 0 the part is skipped +:= standard linux memsize + if omitted the part will immediately follow the previous part + or 0 if the first part + := '(' NAME ')' +partition name, kernel send uevent with "PARTNAME". application could +create a link to block device partition with the name "PARTNAME". +user space application can access partition by partition name. + + 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) - diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c index 18fb435..7ad2bc2 100644 --- a/block/cmdline-parser.c +++ b/block/cmdline-parser.c @@ -4,8 +4,6 @@ * Written by Cai Zhiyong * */ -#include -#include #include static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) @@ -158,6 +156,7 @@ void cmdline_parts_free(struct cmdline_parts **parts) *parts = next_parts; } } +EXPORT_SYMBOL(cmdline_parts_free); int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline) { @@ -205,6 +204,7 @@ fail: cmdline_parts_free(parts); goto done; } +EXPORT_SYMBOL(cmdline_parts_parse); struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts, const char *bdev) @@ -213,6 +213,7 @@ struc
[PATCH] mtd: cmdlinepart: use cmdline partition parser lib
From: Cai Zhiyong MTD cmdline partition use cmdline partition parser lib reference: https://lkml.org/lkml/2013/8/6/550 Signed-off-by: Cai ZhiYong --- Documentation/block/cmdline-partition.txt | 93 +--- block/cmdline-parser.c| 16 +- drivers/mtd/Kconfig | 1 + drivers/mtd/cmdlinepart.c | 361 -- include/linux/cmdline-parser.h| 8 +- 5 files changed, 111 insertions(+), 368 deletions(-) diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt index 651863d..adc5f7a 100644 --- a/Documentation/block/cmdline-partition.txt +++ b/Documentation/block/cmdline-partition.txt @@ -1,40 +1,59 @@ Embedded device command line partition -= - -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=[;] +Authors: Cai Zhiyong + +The format for the command line is as follows, +it is used by MTD device (reference drivers/mtd/cmdlinepart.c) and +block device (reference block/partitions/cmdline.c) + + +For MTD device: + mtdparts=[; := :[,] + := [@][][ro][lk] +:= unique name used in mapping driver/device (mtd->name) + := standard linux memsize OR "-" to denote all remaining space + size is automatically truncated at end of device + if specified or trucated size is 0 the part is skipped +:= standard linux memsize + if omitted the part will immediately follow the previous part + or 0 if the first part + := '(' NAME ')' + NAME will appear in /proc/mtd + + and can be specified such that the parts are out of order + in physical memory and may even overlap. + + The parts are assigned MTD numbers in the order they are specified in the + command line regardless of their order in physical memory. + + Examples: + + 1 NOR Flash, with 1 single writable partition: + edb7312-nor:- + + 1 NOR Flash with 2 partitions, 1 NAND with one + edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) + + +For block device: + blkdevparts=[;] := :[,] - := [@](part-name) - - -block device disk name, embedded device used fixed block device, -it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. - - -partition size, in bytes, such as: 512, 1m, 1G. - - -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. - -Example: -eMMC disk name is "mmcblk0" and "mmcblk0boot0" - - bootargs: + := [@][] + + block device disk name, embedded device used fixed block device, + it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. + := standard linux memsize OR "-" to denote all remaining space + size is automatically truncated at end of device + if specified or trucated size is 0 the part is skipped +:= standard linux memsize + if omitted the part will immediately follow the previous part + or 0 if the first part + := '(' NAME ')' +partition name, kernel send uevent with "PARTNAME". application could +create a link to block device partition with the name "PARTNAME". +user space application can access partition by partition name. + + 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) - diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c index 18fb435..8c09db3 100644 --- a/block/cmdline-parser.c +++ b/block/cmdline-parser.c @@ -4,8 +4,6 @@ * Written by Cai Zhiyong * */ -#include -#include #include static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) @@ -158,6 +156,7 @@ void cmdline_parts_free(struct cmdline_parts **parts) *parts = next_parts; } } +EXPORT_SYMBOL(cmdline_parts_free); int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline) { @@ -205,6 +204,7 @@ fail: cmdline_parts_free(parts); goto done; } +EXPORT_SYMBOL(cmdline_parts_parse); struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts,
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/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/
patch "[SCSI] Fix out of spec CD-ROM problem with media change" will cause usb disk resume fail.
>[SCSI] Fix out of spec CD-ROM problem with media change > >Some CD-ROMs fail to report a media change correctly. The specific >one for this patch simply fails to respond to commands, then gives a >UNIT ATTENTION after being reset which returns ASC/ASCQ 28/00. This >is out of spec behaviour, but add a check in the eat CC/UA on reset >path to catch this case so the CD-ROM will function somewhat properly. > >[jejb: fixed up white space and accepted without signoff] >Signed-off-by: James Bottomley > >diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c >index a4b9cdb..dc6131e 100644 >--- a/drivers/scsi/scsi_error.c >+++ b/drivers/scsi/scsi_error.c >@@ -293,8 +293,16 @@ static int scsi_check_sense(struct scsi_cmnd *scmd) > * so that we can deal with it there. > */ >if (scmd->device->expecting_cc_ua) { >- scmd->device->expecting_cc_ua = 0; >- return NEEDS_RETRY; >+ /* >+* Because some device does not queue unit >+* attentions correctly, we carefully check >+* additional sense code and qualifier so as >+* not to squash media change unit attention. >+*/ >+ if (sshdr.asc != 0x28 || sshdr.ascq != 0x00) { >+ scmd->device->expecting_cc_ua = 0; >+ return NEEDS_RETRY; >+ } >} Consider this scene: 1. Insert usb disk. 2. System suspend, usb controller power off. 3. System resume, usb controller power on, reset the usb disk. When read/write usb disk, it will pass up "media changed" status to scsi layer. this time need NEEDS_RETRY cmd,but this patch filter NEEDS_RETRY cmd, block layer report "IO ERRROR". 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 2/2] mtd: cmdlinepart: use cmdline partition parser lib
>> For further information, see "https://lkml.org/lkml/2013/8/6/550"; > > Thanks for doing this. Could we please get some acked-by's or, > preferably, tested-by's from the MTD people? Acked-by: Ezequiel Garcia Acked-by: Andrew Morton -- 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 2/2] mtd: cmdlinepart: use cmdline partition parser lib
> Nobody has had time to test this on MTD, it seems, and as such, I > strongly recommend you do not force it through -mm. We are perfectly > capable of merging it through the MTD tree if it ever gets proper > vetting by people in MTD (not just on block devices), and I am well > aware of this patch set's existence. > > However, the patch really provides little to no benefit to the MTD > subsystem at the moment, at the added cost of reviewing the small > differences in parsing. For some reason, Cai decided to make small > differences in the parser between drivers/mtd/cmdlinepart.c and > block/cmdline-parser.c, and it is not obvious that these differences > retain the same parsing. For instance, according to my code > read-through, the block device parser seems to accept multiple > partitions to be specified with "-" (meaning "fill the remaining > device"). MTD already has code that rejects such a parser string > outright. The next '-' partition be ignore at function "cmdline_parts_set", and the client will get a right result. Is there any other worry about '-' I don't know ? > > So, I'd recommend one of the following: > (1) We (MTD users) make more time to properly test this patch and post > relevant results (i.e., tested partition strings) or > (2) Somebody (Cai?) spend time to actually make block/cmdline-parser.c > fully equivalent to the parser in drivers/mtd/cmdlinepart.c. That > means it should be trivial to compare the two and say "yes, these are > equivalent". That is currently not the case, AFAICT. I understand your worry about, we use cmdlinepart many years. I will spend time to make block/cmdline-parser.c fully equivalent to the parser in drivers/mtd/cmdlinepart.c. > > Without one of those two, I will give my NAK. > > Brian -- 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 2/2] mtd: cmdlinepart: use cmdline partition parser lib
> Such results was never posted and unless we see those, I think > I'd rather NACK this patch instead. I like the cleanup, but only > if it's guaranteed to _not_ brake things, specially when dealing > with a kernel parameter. Do you have some test case or test standard for me perform. > -- > Ezequiel García, Free Electrons > Embedded Linux, Kernel and Android Engineering > http://free-electrons.com
[PATCH] regmap: Fix compile warning with value uninitialized
From: Cai Zhiyong Date: Sat, 9 Nov 2013 14:14:36 +0800 Subject: [PATCH] regmap: Fix compile warning with value uninitialized Fix compile warning with value uninitialized: drivers/base/regmap/regmap.c:2173: warning: 'ret' may be used uninitialized in this function Signed-off-by: Cai Zhiyong --- drivers/base/regmap/regmap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 9c021d9..4fac073 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2170,7 +2170,8 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs, int num_regs) { struct reg_default *p; - int i, ret; + int i; + int ret = 0; bool bypass; map->lock(map->lock_arg); -- 1.8.1.5 -- 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/
[PATCH 2/2] mtd: cmdlinepart: support master name is not set when parser partition
From: Cai Zhiyong Date: Sat, 9 Nov 2013 17:54:12 +0800 Subject: [PATCH 2/2] mtd: cmdlinepart: support master name is not set when parser partition -Fix compile warning with value and function undeclared. this reported by and Randy Dunlap -support master name is not set when parser partition. this feature is supported in the old parser. Signed-off-by: Cai Zhiyong --- drivers/mtd/Kconfig | 1 + drivers/mtd/cmdlinepart.c | 10 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index daf544a..f8f45e0 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -76,6 +76,7 @@ endif # MTD_REDBOOT_PARTS config MTD_CMDLINE_PARTS tristate "Command line partition table parsing" select BLK_CMDLINE_PARSER + depends on BLOCK depends on MTD ---help--- Allow generic configuration of the MTD partition tables via the kernel diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c index ba934a4..2074164 100644 --- a/drivers/mtd/cmdlinepart.c +++ b/drivers/mtd/cmdlinepart.c @@ -54,6 +54,7 @@ */ #include +#include #include #include #include @@ -108,7 +109,12 @@ static int parse_cmdline_partitions(struct mtd_info *master, if (!mtd_cmdline_parts) return 0; - parts = cmdline_parts_find(mtd_cmdline_parts, master->name); + /* If master->name is not set, return the first device partition. */ + if (!master->name) + parts = mtd_cmdline_parts->next_parts; + else + parts = cmdline_parts_find(mtd_cmdline_parts, master->name); + if (!parts) return 0; @@ -143,5 +149,5 @@ MODULE_PARM_DESC(mtdparts, "Partitioning specification"); module_param(mtdparts, charp, 0); MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Marius Groeger "); +MODULE_AUTHOR("Cai Zhiyong "); MODULE_DESCRIPTION("Command line configuration of MTD partitions"); -- 1.8.1.5 -- 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/
[PATCH] block: cmdline-parser: perfect cmdline format checking
From: Cai Zhiyong Date: Sat, 9 Nov 2013 19:27:38 +0800 Subject: [PATCH] block: cmdline-parser: perfect cmdline format checking -Fix compile warning with value and function undeclared. this reported by and Randy Dunlap -perfect cmdline format checking, make the error information clear for understand, make this lib fully equivalent to the old parser in drivers/mtd/cmdlinepart.c Signed-off-by: Cai Zhiyong --- block/cmdline-parser.c | 86 +++--- include/linux/cmdline-parser.h | 1 + 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c index 9dbc67e..39b6cf4 100644 --- a/block/cmdline-parser.c +++ b/block/cmdline-parser.c @@ -6,10 +6,15 @@ */ #include #include +#include + +#define PARSER "cmdline-parser: " static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) { int ret = 0; + int lastpart = 0; + char *partorg = partdef; struct cmdline_subpart *new_subpart; *subpart = NULL; @@ -21,10 +26,12 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) if (*partdef == '-') { new_subpart->size = (sector_t)(~0ULL); partdef++; + lastpart = 1; } else { new_subpart->size = (sector_t)memparse(partdef, &partdef); if (new_subpart->size < (sector_t)PAGE_SIZE) { - pr_warn("cmdline partition size is invalid."); + pr_warn(PARSER "partition '%s' size '0x%llx' too small.", + partorg, new_subpart->size); ret = -EINVAL; goto fail; } @@ -42,13 +49,19 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) char *next = strchr(++partdef, ')'); if (!next) { - pr_warn("cmdline partition format is invalid."); + pr_warn(PARSER "partition '%s' has no closing ')' " + "found in partition name.", partorg); ret = -EINVAL; goto fail; } - length = min_t(int, next - partdef, - sizeof(new_subpart->name) - 1); + length = (int)(next - partdef); + if (length > sizeof(new_subpart->name) - 1) { + pr_warn(PARSER "partition '%s' partition name too long," + " truncating.", partorg); + length = sizeof(new_subpart->name) - 1; + } + strncpy(new_subpart->name, partdef, length); new_subpart->name[length] = '\0'; @@ -68,8 +81,15 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) partdef += 2; } + if (*partdef) { + pr_warn(PARSER "partition '%s' has bad character '%c' after" + " partition.", partorg, *partdef); + ret = -EINVAL; + goto fail; + } + *subpart = new_subpart; - return 0; + return lastpart; fail: kfree(new_subpart); return ret; @@ -86,14 +106,13 @@ static void free_subpart(struct cmdline_parts *parts) } } -static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) +static int parse_parts(struct cmdline_parts **parts, char *bdevdef) { int ret = -EINVAL; char *next; int length; struct cmdline_subpart **next_subpart; struct cmdline_parts *newparts; - char buf[BDEVNAME_SIZE + 32 + 4]; *parts = NULL; @@ -102,14 +121,21 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) return -ENOMEM; next = strchr(bdevdef, ':'); - if (!next) { - pr_warn("cmdline partition has no block device."); + if (!next || next == bdevdef) { + pr_warn(PARSER "partition '%s' has no block device.", bdevdef); goto fail; } - length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1); + length = (int)(next - bdevdef); + if (length > sizeof(newparts->name) - 1) { + pr_warn(PARSER "partition '%s' device name too long, " + "truncating.", bdevdef); + length = sizeof(newparts->name) - 1; + } + strncpy(newparts->name, bdevdef, length); newparts->name[length] = '\0'; + newparts->nr_subparts = 0; next_subpart = &newparts->subpart; @@ -117,23 +143,26 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) while (next && *(++next)) { bdevdef = next; next = strchr(bdevdef, ','); + if (next) +
RE: [PATCH 2/2] mtd: cmdlinepart: support master name is not set when parser partition
> On Sat, Nov 09, 2013 at 11:43:06AM +0000, Caizhiyong wrote: > > From: Cai Zhiyong > > Date: Sat, 9 Nov 2013 17:54:12 +0800 > > > > @@ -143,5 +149,5 @@ MODULE_PARM_DESC(mtdparts, "Partitioning > > specification"); > > module_param(mtdparts, charp, 0); > > > > MODULE_LICENSE("GPL"); > > -MODULE_AUTHOR("Marius Groeger "); > > +MODULE_AUTHOR("Cai Zhiyong "); > I tested this patch with several cases, it is ok. Thank you for test. > > But is it neccessary to change the author line? I rewrite this module, there may be some bugs or other problems, if I keep the original author, I am afraid that someone care about this. > > thanks > Huang Shijie -- 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/
[PATCH] regmap: Fix 'ret' would return an uninitialized value
From: Cai Zhiyong Date: Mon, 11 Nov 2013 19:26:14 +0800 Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value - Fix 'ret' would return an uninitialized value. - Add a warning avoid invalid 'num_regs' value passed in. When the num_regs parameter is zero and krealloc doesn't fail, then the code would return an uninitialized value. However, calling this function with num_regs == 0, would be a waste as it essentially does nothing. Signed-off-by: Cai Zhiyong --- drivers/base/regmap/regmap.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 9c021d9..7b5c28a 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2170,9 +2170,15 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs, int num_regs) { struct reg_default *p; - int i, ret; + int i; + int ret = 0; bool bypass; + if (num_regs <= 0) { + WARN_ONCE(1, "Call regmap_register_patch with num_regs <= 0."); + return 0; + } + map->lock(map->lock_arg); bypass = map->cache_bypass; -- 1.8.1.5 -- 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/
[PATCH] block: cmdline-parser: perfect cmdline format checking
From: Cai Zhiyong Date: Tue, 12 Nov 2013 15:50:42 +0800 Subject: [PATCH] block: cmdline-parser: perfect cmdline format checking -Fix compile warning with value and function undeclared. this reported by and Randy Dunlap -perfect cmdline format checking, make the error information clear for understand, make this lib fully equivalent to the old parser in drivers/mtd/cmdlinepart.c I tested each branch of this patch change, it is ok. Signed-off-by: Cai Zhiyong --- block/cmdline-parser.c | 83 +++--- include/linux/cmdline-parser.h | 1 + 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c index 9dbc67e..4833936 100644 --- a/block/cmdline-parser.c +++ b/block/cmdline-parser.c @@ -4,12 +4,17 @@ * Written by Cai Zhiyong * */ +#define pr_fmt(fmt) "cmdline-parser: " fmt + #include #include +#include static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) { int ret = 0; + int lastpart = 0; + char *partorg = partdef; struct cmdline_subpart *new_subpart; *subpart = NULL; @@ -21,10 +26,12 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) if (*partdef == '-') { new_subpart->size = (sector_t)(~0ULL); partdef++; + lastpart = 1; } else { new_subpart->size = (sector_t)memparse(partdef, &partdef); if (new_subpart->size < (sector_t)PAGE_SIZE) { - pr_warn("cmdline partition size is invalid."); + pr_warn("partition '%s' size '0x%llx' too small.\n", + partorg, new_subpart->size); ret = -EINVAL; goto fail; } @@ -42,13 +49,19 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) char *next = strchr(++partdef, ')'); if (!next) { - pr_warn("cmdline partition format is invalid."); + pr_warn("partition '%s' has no closing ')' found in partition name.\n", + partorg); ret = -EINVAL; goto fail; } - length = min_t(int, next - partdef, - sizeof(new_subpart->name) - 1); + length = (int)(next - partdef); + if (length > sizeof(new_subpart->name) - 1) { + pr_warn("partition '%s' partition name too long, truncating.\n", + partorg); + length = sizeof(new_subpart->name) - 1; + } + strncpy(new_subpart->name, partdef, length); new_subpart->name[length] = '\0'; @@ -68,8 +81,15 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) partdef += 2; } + if (*partdef) { + pr_warn("partition '%s' has bad character '%c' after partition.\n", + partorg, *partdef); + ret = -EINVAL; + goto fail; + } + *subpart = new_subpart; - return 0; + return lastpart; fail: kfree(new_subpart); return ret; @@ -86,14 +106,13 @@ static void free_subpart(struct cmdline_parts *parts) } } -static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) +static int parse_parts(struct cmdline_parts **parts, char *bdevdef) { int ret = -EINVAL; char *next; int length; struct cmdline_subpart **next_subpart; struct cmdline_parts *newparts; - char buf[BDEVNAME_SIZE + 32 + 4]; *parts = NULL; @@ -102,14 +121,21 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) return -ENOMEM; next = strchr(bdevdef, ':'); - if (!next) { - pr_warn("cmdline partition has no block device."); + if (!next || next == bdevdef) { + pr_warn("partition '%s' has no block device.\n", bdevdef); goto fail; } - length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1); + length = (int)(next - bdevdef); + if (length > sizeof(newparts->name) - 1) { + pr_warn("partition '%s' device name too long, truncating.\n", + bdevdef); + length = sizeof(newparts->name) - 1; + } + strncpy(newparts->name, bdevdef, length); newparts->name[length] = '\0'; + newparts->nr_subparts = 0; next_subpart = &newparts->subpart; @@ -117,23 +143,25 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef) while (next && *(++next)) { bdevdef = next; next = strchr(bdevdef, ','); +
[PATCH] regmap: Fix 'ret' would return an uninitialized value
From: Cai Zhiyong Date: Mon, 11 Nov 2013 19:26:14 +0800 Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value This patch give a warning when calling regmap_register_patch with parameter num_regs <= 0. When the num_regs parameter is zero and krealloc doesn't fail, then the code would return an uninitialized value. However, calling this function with num_regs == 0, would be a waste as it essentially does nothing. Signed-off-by: Cai Zhiyong --- drivers/base/regmap/regmap.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 9c021d9..7b5c28a 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2170,9 +2170,12 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs, int num_regs) { struct reg_default *p; int i, ret; bool bypass; + if (WARN_ONCE(num_regs <= 0)) + return 0; + map->lock(map->lock_arg); bypass = map->cache_bypass; -- 1.8.1.5 -- 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/
[PATCH] regmap: Fix 'ret' would return an uninitialized value
From: Cai Zhiyong Date: Mon, 18 Nov 2013 20:21:49 +0800 Subject: [PATCH] regmap: Fix 'ret' would return an uninitialized value This patch give a warning when calling regmap_register_patch with parameter num_regs <= 0. When the num_regs parameter is zero and krealloc doesn't fail, then the code would return an uninitialized value. However, calling this function with num_regs == 0, would be a waste as it essentially does nothing. Signed-off-by: Cai Zhiyong --- drivers/base/regmap/regmap.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 9c021d9..9a36ac1 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -2173,6 +2173,10 @@ int regmap_register_patch(struct regmap *map, const struct reg_default *regs, int i, ret; bool bypass; + if (WARN_ONCE(num_regs <= 0, "invalid registers number (%d)\n", + num_regs)) + return 0; + map->lock(map->lock_arg); bypass = map->cache_bypass; -- 1.8.1.5 -- 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-kernel&m=127425650923757&w=2 > http://marc.info/?l=linux-kernel&m=127599718024364&w=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 -next] block: fix error return code in parse_parts()
> From: Wei Yongjun [mailto:weiyj...@gmail.com] > Sent: Friday, August 23, 2013 10:48 AM > To: ax...@kernel.dk; a...@linux-foundation.org; Caizhiyong; k...@redhat.com; > m...@sysgo.de; dw...@infradead.org; computersforpe...@gmail.com; > dedek...@infradead.org > Cc: yongjun_...@trendmicro.com.cn; linux-kernel@vger.kernel.org > Subject: [PATCH -next] block: fix error return code in parse_parts() > > From: Wei Yongjun > > Fix to return -EINVAL in the parts parse error handling case instead > of 0(may overwrite to 0 by parse_subpart()), as done elsewhere in this > function. > > Signed-off-by: Wei Yongjun > --- > block/cmdline-parser.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c > index 18fb435..cc2637f 100644 > --- a/block/cmdline-parser.c > +++ b/block/cmdline-parser.c > @@ -135,6 +135,7 @@ static int parse_parts(struct cmdline_parts **parts, > const char > *bdevdef) > > if (!newparts->subpart) { > pr_warn("cmdline partition has no valid partition."); > + ret = -EINVAL; Seems OK to me. > goto fail; > } > -- 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 -next] block: fix error return code in parse_parts()
> -Original Message- > From: Wei Yongjun [mailto:weiyj...@gmail.com] > Sent: Friday, August 23, 2013 10:48 AM > To: ax...@kernel.dk; a...@linux-foundation.org; Caizhiyong; k...@redhat.com; > m...@sysgo.de; dw...@infradead.org; computersforpe...@gmail.com; > dedek...@infradead.org > Cc: yongjun_...@trendmicro.com.cn; linux-kernel@vger.kernel.org > Subject: [PATCH -next] block: fix error return code in parse_parts() > > From: Wei Yongjun > > Fix to return -EINVAL in the parts parse error handling case instead > of 0(may overwrite to 0 by parse_subpart()), as done elsewhere in this > function. > > Signed-off-by: Wei Yongjun > --- > block/cmdline-parser.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c > index 18fb435..cc2637f 100644 > --- a/block/cmdline-parser.c > +++ b/block/cmdline-parser.c > @@ -135,6 +135,7 @@ static int parse_parts(struct cmdline_parts **parts, > const char > *bdevdef) > > if (!newparts->subpart) { > pr_warn("cmdline partition has no valid partition."); > + ret = -EINVAL; Seems OK to me. > goto fail; > } > -- 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/
[PATCH] block: add command line partition parser
From: Cai Zhiyong move the command line parser to a separate module, and change it into library-style code. reference: https://lkml.org/lkml/2013/8/6/550 Signed-off-by: Cai Zhiyong --- block/Kconfig | 6 + block/Makefile | 1 + block/cmdline-parser.c | 249 + block/partitions/Kconfig | 1 + block/partitions/cmdline.c | 311 ++--- include/linux/cmdline-parser.h | 43 ++ 6 files changed, 342 insertions(+), 269 deletions(-) create mode 100644 block/cmdline-parser.c create mode 100644 include/linux/cmdline-parser.h diff --git a/block/Kconfig b/block/Kconfig index a7e40a7..7f38e40 100644 --- a/block/Kconfig +++ b/block/Kconfig @@ -99,6 +99,12 @@ config BLK_DEV_THROTTLING See Documentation/cgroups/blkio-controller.txt for more information. +config CMDLINE_PARSER + bool "Block device command line partition parser" + default n + ---help--- + Parsing command line, get the partitions information. + menu "Partition Types" source "block/partitions/Kconfig" diff --git a/block/Makefile b/block/Makefile index 39b76ba..4fa4be5 100644 --- a/block/Makefile +++ b/block/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o obj-$(CONFIG_BLK_DEV_INTEGRITY)+= blk-integrity.o +obj-$(CONFIG_CMDLINE_PARSER) += cmdline-parser.o diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c new file mode 100644 index 000..18fb435 --- /dev/null +++ b/block/cmdline-parser.c @@ -0,0 +1,249 @@ +/* + * Parse command line, get partition information + * + * Written by Cai Zhiyong + * + */ +#include +#include +#include + +static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) +{ + 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 (*partdef == '-') { + new_subpart->size = (sector_t)(~0ULL); + partdef++; + } else { + new_subpart->size = (sector_t)memparse(partdef, &partdef); + if (new_subpart->size < (sector_t)PAGE_SIZE) { + pr_warn("cmdline partition size is invalid."); + ret = -EINVAL; + goto fail; + } + } + + if (*partdef == '@') { + partdef++; + new_subpart->from = (sector_t)memparse(partdef, &partdef); + } else { + new_subpart->from = (sector_t)(~0ULL); + } + + if (*partdef == '(') { + int length; + char *next = strchr(++partdef, ')'); + + if (!next) { + pr_warn("cmdline partition format is invalid."); + ret = -EINVAL; + goto fail; + } + + length = min_t(int, next - partdef, + sizeof(new_subpart->name) - 1); + strncpy(new_subpart->name, partdef, length); + new_subpart->name[length] = '\0'; + + partdef = ++next; + } else + new_subpart->name[0] = '\0'; + + new_subpart->flags = 0; + + if (!strncmp(partdef, "ro", 2)) { + new_subpart->flags |= PF_RDONLY; + partdef += 2; + } + + if (!strncmp(partdef, "lk", 2)) { + new_subpart->flags |= PF_POWERUP_LOCK; + partdef += 2; + } + + *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 int parse_parts(struct cmdline_parts **parts, const char *bdevdef) +{ + int ret = -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(bdevdef, ':'); + if (!next) { + pr_warn("cmdline partition has no block device."); + goto fail; + } + + length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1); + strncpy(newparts->name, bdevdef, length); + newparts->name[length] = '\0'; + newparts->nr_subparts = 0; + + next_subpart = &newparts->subpart; + + while (next && *(++next)) { + bdevdef = next; + nex
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/
[PATCH] block: support embedded device command line partition
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. 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)' Signed-off-by: Cai Zhiyong --- block/partitions/Kconfig | 6 + block/partitions/Makefile | 1 + block/partitions/check.c | 4 + block/partitions/cmdline.c | 346 + block/partitions/cmdline.h | 2 + 5 files changed, 359 insertions(+) create mode 100644 block/partitions/cmdline.c create mode 100644 block/partitions/cmdline.h diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 4cebb2f..2ebf251 100644 --- a/block/partitions/Kconfig +++ b/block/partitions/Kconfig @@ -260,3 +260,9 @@ config SYSV68_PARTITION partition table format used by Motorola Delta machines (using sysv68). Otherwise, say N. + +config CMDLINE_PARTITION + bool "Command line partition support" if PARTITION_ADVANCED + help + Say Y here if you would read the partitions table from bootargs. + The format for the command line is just like mtdparts. diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 2be4d7b..4e9d584 100644 --- a/block/partitions/Makefile +++ b/block/partitions/Makefile @@ -19,3 +19,4 @@ obj-$(CONFIG_IBM_PARTITION) += ibm.o obj-$(CONFIG_EFI_PARTITION) += efi.o obj-$(CONFIG_KARMA_PARTITION) += karma.o obj-$(CONFIG_SYSV68_PARTITION) += sysv68.o +obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o diff --git a/block/partitions/check.c b/block/partitions/check.c index 19ba207..9ac1df7 100644 --- a/block/partitions/check.c +++ b/block/partitions/check.c @@ -34,6 +34,7 @@ #include "efi.h" #include "karma.h" #include "sysv68.h" +#include "cmdline.h" int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/ @@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed_partitions *) = { adfspart_check_ADFS, #endif +#ifdef CONFIG_CMDLINE_PARTITION + cmdline_partition, +#endif #ifdef CONFIG_EFI_PARTITION efi_partition, /* this must come before msdos */ #endif diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c new file mode 100644 index 000..05c7e17 --- /dev/null +++ b/block/partitions/cmdline.c @@ -0,0 +1,346 @@ +/* + * Copyright (C) 2013 HUAWEI + * Author: Cai Zhiyong + * + * Read block device partition table from command line. + * Design the partition for eMMC device. + * 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)' + * + */ + +#include +#include +#include + +#include "check.h" +#include "cmdline.h" + +#define SIZE_REMAINING ULLONG_MAX +#define OFFSET_CONTINUOUS ULLONG_MAX + +struct cmdline_subpart { + char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */ + uint64_t from; + uint64_t size; + struct cmdline_subpart *next_subpart; +}; + +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; + +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; + goto fail; + } + } + + if ((*cmdline) == '@') { + cmdline++; + new_subpart->from = memparse(cmdline, &cmdline); + } else { + new_subpart->from = OFFSET_CONTINUOUS; + } + + if ((*cmdline) == '(') { + + int length; + char *next = strchr(++cmdline, ')'); + + if (!next) { + pr_warn("
[PATCH] block: support embedded device command line partition
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 --- Documentation/block/cmdline-partition.txt | 40 block/partitions/Kconfig | 6 + block/partitions/Makefile | 1 + block/partitions/check.c | 4 + block/partitions/cmdline.c| 326 ++ block/partitions/cmdline.h| 2 + 6 files changed, 379 insertions(+) create mode 100644 Documentation/block/cmdline-partition.txt create mode 100644 block/partitions/cmdline.c create mode 100644 block/partitions/cmdline.h diff --git a/Documentation/block/cmdline-partition.txt b/Documentation/block/cmdline-partition.txt new file mode 100644 index 000..651863d --- /dev/null +++ b/Documentation/block/cmdline-partition.txt @@ -0,0 +1,40 @@ +Embedded device command line partition += + +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. + + +partition size, in bytes, such as: 512, 1m, 1G. + + +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. + +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) + diff --git a/block/partitions/Kconfig b/block/partitions/Kconfig index 4cebb2f..2ebf251 100644 --- a/block/partitions/Kconfig +++ b/block/partitions/Kconfig @@ -260,3 +260,9 @@ config SYSV68_PARTITION partition table format used by Motorola Delta machines (using sysv68). Otherwise, say N. + +config CMDLINE_PARTITION + bool "Command line partition support" if PARTITION_ADVANCED + help + Say Y here if you would read the partitions table from bootargs. + The format for the command line is just like mtdparts. diff --git a/block/partitions/Makefile b/block/partitions/Makefile index 2be4d7b..4e9d584 100644 --- a/block/partitions/Makefile +++ b/block/partitions/Makefile @@ -19,3 +19,4 @@ obj-$(CONFIG_IBM_PARTITION) += ibm.o obj-$(CONFIG_EFI_PARTITION) += efi.o obj-$(CONFIG_KARMA_PARTITION) += karma.o obj-$(CONFIG_SYSV68_PARTITION) += sysv68.o +obj-$(CONFIG_CMDLINE_PARTITION) += cmdline.o diff --git a/block/partitions/check.c b/block/partitions/check.c index 19ba207..9ac1df7 100644 --- a/block/partitions/check.c +++ b/block/partitions/check.c @@ -34,6 +34,7 @@ #include "efi.h" #include "karma.h" #include "sysv68.h" +#include "cmdline.h" int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/ @@ -65,6 +66,9 @@ static int (*check_part[])(struct parsed_partitions *) = { adfspart_check_ADFS, #endif +#ifdef CONFIG_CMDLINE_PARTITION + cmdline_partition, +#endif #ifdef CONFIG_EFI_PARTITION efi_partition, /* this must come before msdos */ #endif diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c new file mode 100644 index 000..dc69483 --- /dev/null +++ b/block/partitions/cmdline.c @@ -0,0 +1,326 @@ +/* + * Copyright (C) 2013 HUAWEI + * Author: 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" + * The format for the command line is just like mtdparts. + * + * Verbose config please reference "Documentation/block/cmdline-partition.txt" + * + */ + +#include +#include +#include + +#include "check.h" +#include "cmdline.h" + +struct cmdline_subpart { + char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */ +
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] mtd: nand: assign mtd->name in find_full_id_nand
> -Original Message- > From: Brian Norris [mailto:computersforpe...@gmail.com] > Sent: Sunday, January 12, 2014 4:10 AM > To: Huang Shijie; Caizhiyong > Cc: linux-...@lists.infradead.org; Wanglin (Albert); David Woodhouse; > linux-kernel@vger.kernel.org; Artem Bityutskiy > Subject: Re: [PATCH] mtd: nand: assign mtd->name in find_full_id_nand > > Hi, > > On Thu, Dec 26, 2013 at 10:19:39AM +0800, Huang Shijie wrote: > > On Thu, Dec 26, 2013 at 01:51:58AM +, Caizhiyong wrote: > > > From: Cai Zhiyong > > > Date: Wed, 25 Dec 2013 21:19:21 +0800 > > > Subject: [PATCH] mtd: nand: assign mtd->name in find_full_id_nand > > > > > > This patch assigned the type->name to mtd->name when mtd->name is > > > NULL in function "find_full_id_nand". > > > mtd->name is NULL may cause some problem. > > > > > > Signed-off-by: Cai Zhiyong > > Acked-by: Huang Shijie > > Thanks, the patch looks good. Pushed to l2-mtd.git. > > Wouldn't this trigger an exception when reading > /sys/class/mtd/mtdX/name? If so, should this be marked for stable? View the source code, if mtd->name is NULL, cat "/sys/class/mtd/mtdX/name" will get string "(null)". static ssize_t mtd_name_show(struct device *dev, struct device_attribute *attr, char *buf) { struct mtd_info *mtd = dev_get_drvdata(dev); return snprintf(buf, PAGE_SIZE, "%s\n", mtd->name); } -- 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/
[PATCH 1/2] block: remove unrelated header files and export symbol
From: CaiZhiyong Subject: block: remove unrelated header files and export symbol This patch fix up the following items: - remove unrelated header files. - export interface function. - modify function cmdline_parts_parse return value, this will make it more friendly for the caller. Signed-off-by: CaiZhiyong --- block/cmdline-parser.c | 18 +++--- include/linux/cmdline-parser.h | 8 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c index cc2637f..9dbc67e 100644 --- a/block/cmdline-parser.c +++ b/block/cmdline-parser.c @@ -4,8 +4,7 @@ * Written by Cai Zhiyong * */ -#include -#include +#include #include static int parse_subpart(struct cmdline_subpart **subpart, char *partdef) @@ -159,6 +158,7 @@ void cmdline_parts_free(struct cmdline_parts **parts) *parts = next_parts; } } +EXPORT_SYMBOL(cmdline_parts_free); int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline) { @@ -206,6 +206,7 @@ fail: cmdline_parts_free(parts); goto done; } +EXPORT_SYMBOL(cmdline_parts_parse); struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts, const char *bdev) @@ -214,17 +215,17 @@ struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts, parts = parts->next_parts; return parts; } +EXPORT_SYMBOL(cmdline_parts_find); /* * add_part() *0 success. *1 can not add so many partitions. */ -void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size, - int slot, - int (*add_part)(int, struct cmdline_subpart *, void *), - void *param) - +int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size, + int slot, + int (*add_part)(int, struct cmdline_subpart *, void *), + void *param) { sector_t from = 0; struct cmdline_subpart *subpart; @@ -247,4 +248,7 @@ void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size, if (add_part(slot, subpart, param)) break; } + + return slot; } +EXPORT_SYMBOL(cmdline_parts_set); diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h index 98e892e..43b65a9 100644 --- a/include/linux/cmdline-parser.h +++ b/include/linux/cmdline-parser.h @@ -35,9 +35,9 @@ int cmdline_parts_parse(struct cmdline_parts **parts, const char *cmdline); struct cmdline_parts *cmdline_parts_find(struct cmdline_parts *parts, const char *bdev); -void cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size, - int slot, - int (*add_part)(int, struct cmdline_subpart *, void *), - void *param); +int cmdline_parts_set(struct cmdline_parts *parts, sector_t disk_size, + int slot, + int (*add_part)(int, struct cmdline_subpart *, void *), + void *param); #endif /* CMDLINEPARSEH */ -- 1.8.1.5 N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a��� 0��h���i
[PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib
From: CaiZhiyong Subject: mtd: cmdlinepart: use cmdline partition parser lib In the previous version, adjust the cmdline parser code to library-style code, and move it to a separate file "block/cmdline-parser.c", we can use it in some client code. there is no any functionality change in the adjusting. this patch use cmdline parser lib. For further information, see "https://lkml.org/lkml/2013/8/6/550"; Signed-off-by: CaiZhiyong --- drivers/mtd/Kconfig | 1 + drivers/mtd/cmdlinepart.c | 343 +- 2 files changed, 39 insertions(+), 305 deletions(-) diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig index 5fab4e6e..daf544a 100644 --- a/drivers/mtd/Kconfig +++ b/drivers/mtd/Kconfig @@ -75,6 +75,7 @@ endif # MTD_REDBOOT_PARTS config MTD_CMDLINE_PARTS tristate "Command line partition table parsing" + select BLK_CMDLINE_PARSER depends on MTD ---help--- Allow generic configuration of the MTD partition tables via the kernel diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c index 721caeb..ba934a4 100644 --- a/drivers/mtd/cmdlinepart.c +++ b/drivers/mtd/cmdlinepart.c @@ -47,344 +47,79 @@ * 1 NOR Flash with 2 partitions, 1 NAND with one * edb7312-nor:256k(ARMboot)ro,-(root);edb7312-nand:-(home) */ + /* + * Copyright © 2013 Cai Zhiyong + * Rewrite the cmdline parser code, adjust it to a library-style code. + * this module only use the cmdline parser lib. + */ #include -#include #include #include #include #include +#include -/* error message prefix */ -#define ERRP "mtd: " - -/* debug macro */ -#if 0 -#define dbg(x) do { printk("DEBUG-CMDLINE-PART: "); printk x; } while(0) -#else -#define dbg(x) -#endif - - -/* special size referring to all the remaining space in a partition */ -#define SIZE_REMAINING ULLONG_MAX -#define OFFSET_CONTINUOUS ULLONG_MAX - -struct cmdline_mtd_partition { - struct cmdline_mtd_partition *next; - char *mtd_id; - int num_parts; - struct mtd_partition *parts; -}; - -/* mtdpart_setup() parses into here */ -static struct cmdline_mtd_partition *partitions; - -/* the command line passed to mtdpart_setup() */ static char *mtdparts; -static char *cmdline; -static int cmdline_parsed; +static struct cmdline_parts *mtd_cmdline_parts; -/* - * Parse one partition definition for an MTD. Since there can be many - * comma separated partition definitions, this function calls itself - * recursively until no more partition definitions are found. Nice side - * effect: the memory to keep the mtd_partition structs and the names - * is allocated upon the last definition being found. At that point the - * syntax has been verified ok. - */ -static struct mtd_partition * newpart(char *s, - char **retptr, - int *num_parts, - int this_part, - unsigned char **extra_mem_ptr, - int extra_mem_size) +static int add_part(int slot, struct cmdline_subpart *subpart, void *param) { - struct mtd_partition *parts; - unsigned long long size, offset = OFFSET_CONTINUOUS; - char *name; - int name_len; - unsigned char *extra_mem; - char delim; - unsigned int mask_flags; - - /* fetch the partition size */ - if (*s == '-') { - /* assign all remaining space to this partition */ - size = SIZE_REMAINING; - s++; - } else { - size = memparse(s, &s); - if (size < PAGE_SIZE) { - printk(KERN_ERR ERRP "partition size too small (%llx)\n", - size); - return ERR_PTR(-EINVAL); - } - } - - /* fetch partition name and flags */ - mask_flags = 0; /* this is going to be a regular partition */ - delim = 0; - - /* check for offset */ - if (*s == '@') { - s++; - offset = memparse(s, &s); - } + struct mtd_partition *mtdpart = &((struct mtd_partition *)param)[slot]; - /* now look for name */ - if (*s == '(') - delim = ')'; + mtdpart->offset = subpart->from; + mtdpart->size = subpart->size; + mtdpart->name = subpart->name; + mtdpart->mask_flags = 0; - if (delim) { - char *p; + if (subpart->flags & PF_RDONLY) + mtdpart->mask_flags |= MTD_WRITEABLE; - name = ++s; - p = strchr(name, delim); - if (!p) { - printk(KERN_ERR ERRP "no closing %c found in partition name\n", delim); -
RE: [PATCH] mtd: cmdlinepart: use cmdline partition parser lib
> > I'd like to review the patch in detail and test it, but being a bit old, > the patch doesn't apply as it is on v3.12-rc5. Care to resend an update? > I have sent you a 2 patch base on v3.12-rc6. Are there any problems? I tested it for a long time and found no problems. Are there any plans for supporting synchronous NAND in the future? > Brian: I saw you want to move forward with this. I guess we can give > it a test, and if all the possible 'mtdparts' use cases work, then > we shouldn't object the cleanup, right? > -- > Ezequiel García, Free Electrons > Embedded Linux, Kernel and Android Engineering > http://free-electrons.com
[PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND
From: Cai Zhiyong Date: Fri, 13 Dec 2013 12:52:46 +0800 Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND. The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes. So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE. Signed-off-by: Huang Shijie Signed-off-by: Artem Bityutskiy Signed-off-by: David Woodhouse Signed-off-by: Cai Zhiyong --- include/linux/mtd/nand.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index f3ea8da..2f0a7f2 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len); * is supported now. If you add a chip with bigger oobsize/page * adjust this accordingly. */ -#define NAND_MAX_OOBSIZE 744 -#define NAND_MAX_PAGESIZE 8192 +#define NAND_MAX_OOBSIZE 1280 +#define NAND_MAX_PAGESIZE 16384 /* * Constants for hardware specific CLE/ALE/NCE function -- 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/
[PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND
From: Cai Zhiyong Date: Fri, 13 Dec 2013 12:52:46 +0800 Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize NAND. The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes. So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE. Signed-off-by: Cai Zhiyong --- include/linux/mtd/nand.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index f3ea8da..2f0a7f2 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len); * is supported now. If you add a chip with bigger oobsize/page * adjust this accordingly. */ -#define NAND_MAX_OOBSIZE 744 -#define NAND_MAX_PAGESIZE 8192 +#define NAND_MAX_OOBSIZE 1280 +#define NAND_MAX_PAGESIZE 16384 /* * Constants for hardware specific CLE/ALE/NCE function -- 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] mtd: increase max page/OOB size to support 16K pagesize NAND
> On Fri, Dec 13, 2013 at 05:03:49AM +0000, Caizhiyong wrote: > > From: Cai Zhiyong > > Date: Fri, 13 Dec 2013 12:52:46 +0800 > > Subject: [PATCH] mtd: increase max page/OOB size to support 16K pagesize > > NAND. > > > > The Toshiba's TC58TEG5DCJTA pagesize is 16K, oob size is 1280 bytes. > > So increase the NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE. > > It would help if we had nand_base/nand_ids support for TC58TEG5DCJTA > before we try to increase NAND_MAX_PAGESIZE and NAND_MAX_OOBSIZE yet > again. AFAICT, nand_base will not detect new, large Toshiba flash > properly anyway. There are a lot of 16K page size NAND, they are used in android system. Many NAND need read retry and data randomization, kernel does not support this feature. Kernel also does not support synchronous NAND. As you know, is someone doing some work to support those feature now? If Kernel support TC58TEG5DCJTA, the read retry and data randomization also need be supported, it will be a big project. > > > Signed-off-by: Cai Zhiyong > > --- > > include/linux/mtd/nand.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > > index f3ea8da..2f0a7f2 100644 > > --- a/include/linux/mtd/nand.h > > +++ b/include/linux/mtd/nand.h > > @@ -56,8 +56,8 @@ extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, > uint64_t len); > > * is supported now. If you add a chip with bigger oobsize/page > > * adjust this accordingly. > > */ > > -#define NAND_MAX_OOBSIZE 744 > > -#define NAND_MAX_PAGESIZE 8192 > > +#define NAND_MAX_OOBSIZE 1280 > > +#define NAND_MAX_PAGESIZE 16384 > > It might be time to draw a line in the sand: next person who needs to > increase MAX_{PAGE,OOB}SIZE gets the job of killing the macro? We are > long overdue for dynamic buffer allocation. > > There are two drivers that use the macros, plus nand_chip.buffers.*. The > drivers can be weaned off by allocating a small, fixed-size temporary > buffer for the few tasks that need it. With nand_base.c, I think we can > get by with waiting to allocate nand_chip.buffers until after we detect > the writesize/oobsize. > > So Cai, are you up for this? Ok, I will try update it. > > > > > /* > > * Constants for hardware specific CLE/ALE/NCE function > > > > Brian -- 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] mtd: increase max page/OOB size to support 16K pagesize NAND
> > > > There are a lot of 16K page size NAND, they are used in android system. > > > > Many NAND need read retry and data randomization, kernel does not support > > this > feature. > Brian is adding the read-retry feature. > > I think the data randomization should be done by the hardware. > If we do it by software, it costs lot of the cpu cycles. > > > > Kernel also does not support synchronous NAND. > > The synchronous NAND should be supported by the nand controller driver, > not the kernel. Yes. Those feature should be support by NAND controller hardware and driver. But the driver does not know what NAND should use those feature. Perhaps, the NAND id table, should add something. Looking forward to your feedback. Best regards. Cai Zhiyong. -- 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] mtd: increase max page/OOB size to support 16K pagesize NAND
> -Original Message- > From: Huang Shijie [mailto:b32...@freescale.com] > Sent: Monday, December 16, 2013 11:07 AM > To: Caizhiyong > Cc: Huang Shijie; Brian Norris; David Woodhouse; Quyaxin; > linux-kernel@vger.kernel.org; linux-...@lists.infradead.org; Artem Bityutskiy; > Wanglin (Albert); David Woodhouse > Subject: Re: [PATCH] mtd: increase max page/OOB size to support 16K pagesize > NAND > > On Mon, Dec 16, 2013 at 02:03:54AM +, Caizhiyong wrote: > > > > > > > > There are a lot of 16K page size NAND, they are used in android system. > > > > > > > > Many NAND need read retry and data randomization, kernel does not > > > > support > this > > > feature. > > > Brian is adding the read-retry feature. > > > > > > I think the data randomization should be done by the hardware. > > > If we do it by software, it costs lot of the cpu cycles. > > > > > > > > > > Kernel also does not support synchronous NAND. > > > > > > The synchronous NAND should be supported by the nand controller driver, > > > not the kernel. > > > > Yes. Those feature should be support by NAND controller hardware and driver. > > But the driver does not know what NAND should use those feature. > > Perhaps, the NAND id table, should add something. > We can get the synchronous info from the ONFI nand. > > In actually, if you do not add this patch, i will change it too. > I have several 16K nand chips on my hands now. :) I have not found a good way to solve these problems. I expect your patch. > > thanks > Huang Shijie > -- 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/
[PATCH] mtd: nand: remove unused function input parameter
From: Cai Zhiyong Date: Wed, 25 Dec 2013 20:11:15 +0800 Subject: [PATCH] mtd: nand: remove unused function input parameter The nand_get_flash_type parameter "busw" input value is not used by any branch, and it is updated before use it in the function, so remove it, define the "busw" as an internal variable. Signed-off-by: Cai Zhiyong --- drivers/mtd/nand/nand_base.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 9b3bb3c..a123afa 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3337,10 +3337,10 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip, */ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd, struct nand_chip *chip, - int busw, int *maf_id, int *dev_id, struct nand_flash_dev *type) { + int busw; int i, maf_idx; u8 id_data[8]; @@ -3497,18 +3497,16 @@ ident_done: int nand_scan_ident(struct mtd_info *mtd, int maxchips, struct nand_flash_dev *table) { - int i, busw, nand_maf_id, nand_dev_id; + int i, nand_maf_id, nand_dev_id; struct nand_chip *chip = mtd->priv; struct nand_flash_dev *type; - /* Get buswidth to select the correct functions */ - busw = chip->options & NAND_BUSWIDTH_16; /* Set the default functions */ - nand_set_defaults(chip, busw); + nand_set_defaults(chip, chip->options & NAND_BUSWIDTH_16); /* Read the flash type */ - type = nand_get_flash_type(mtd, chip, busw, - &nand_maf_id, &nand_dev_id, table); + type = nand_get_flash_type(mtd, chip, &nand_maf_id, + &nand_dev_id, table); if (IS_ERR(type)) { if (!(chip->options & NAND_SCAN_SILENT_NODEV)) -- 1.8.1.5 -- 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/
[PATCH] mtd: nand: assign mtd->name in find_full_id_nand
From: Cai Zhiyong Date: Wed, 25 Dec 2013 21:19:21 +0800 Subject: [PATCH] mtd: nand: assign mtd->name in find_full_id_nand This patch assigned the type->name to mtd->name when mtd->name is NULL in function "find_full_id_nand". mtd->name is NULL may cause some problem. Signed-off-by: Cai Zhiyong --- drivers/mtd/nand/nand_base.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 9b3bb3c..404e83d 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -3327,6 +3327,9 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip, *busw = type->options & NAND_BUSWIDTH_16; + if (!mtd->name) + mtd->name = type->name; + return true; } return false; -- 1.8.1.5 -- 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/
mtd: nand: remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE
From: Cai Zhiyong Date: Fri, 27 Dec 2013 20:23:09 +0800 Subject: [PATCH] mtd: nand: remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE, dynamic allocate NAND buffer after detecting NAND writesize and oobsize. - save memory. when use MACRO allocate static buffer, use a small page size NAND chip, the buffer is still 8k, the memory is wasted. - reduce maintenance. NAND chip pagesize and oobsize update frequently, allocate buffer according to the detecting size will reduce maintenance. - this fix was suggested by Brian Norris [computersforpe...@gmail.com] and Huang Shijie [b32...@freescale.com] Signed-off-by: Cai Zhiyong --- drivers/mtd/nand/cafe_nand.c | 15 +++-- drivers/mtd/nand/denali.c | 9 - drivers/mtd/nand/denali.h | 2 +- drivers/mtd/nand/fsmc_nand.c | 4 +-- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 4 +-- drivers/mtd/nand/mxc_nand.c| 5 ++- drivers/mtd/nand/nand_base.c | 60 -- drivers/mtd/nand/nand_bbt.c| 2 +- drivers/mtd/nand/omap2.c | 6 ++-- include/linux/mtd/nand.h | 17 +++--- 10 files changed, 66 insertions(+), 58 deletions(-) diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c index c34985a..c784621 100644 --- a/drivers/mtd/nand/cafe_nand.c +++ b/drivers/mtd/nand/cafe_nand.c @@ -53,6 +53,9 @@ #define CAFE_GLOBAL_IRQ_MASK 0x300c #define CAFE_NAND_RESET0x3034 +#define CAFE_MAX_OOBSIZE 744 +#define CAFE_MAX_PAGESIZE 8192 + /* Missing from the datasheet: bit 19 of CTRL1 sets CE0 vs. CE1 */ #define CTRL1_CHIPSELECT (1<<19) @@ -657,13 +660,21 @@ static int cafe_nand_probe(struct pci_dev *pdev, err = -ENOMEM; goto out_free_mtd; } - cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112 + sizeof(struct nand_buffers), + cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, + (2112 + CAFE_MAX_OOBSIZE * 3 + + CAFE_MAX_PAGESIZE), &cafe->dmaaddr, GFP_KERNEL); if (!cafe->dmabuf) { err = -ENOMEM; goto out_ior; } - cafe->nand.buffers = (void *)cafe->dmabuf + 2112; + + cafe->nand.buffers.buffer = (void *)cafe->dmabuf + 2112; + cafe->nand.buffers.ecccalc = cafe->nand.buffers.buffer; + cafe->nand.buffers.ecccode = cafe->nand.buffers.ecccalc + + CAFE_MAX_OOBSIZE; + cafe->nand.buffers.databuf = cafe->nand.buffers.ecccode + + CAFE_MAX_OOBSIZE; cafe->rs = init_rs_non_canonical(12, &cafe_mul, 0, 1, 8); if (!cafe->rs) { diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index 3a3a47f..e86c995 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -1475,15 +1475,6 @@ int denali_init(struct denali_nand_info *denali) goto failed_req_irq; } - /* MTD supported page sizes vary by kernel. We validate our -* kernel supports the device here. -*/ - if (denali->mtd.writesize > NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) { - ret = -ENODEV; - pr_err("Spectra: device size not supported by this version of MTD."); - goto failed_req_irq; - } - /* support for multi nand * MTD known nothing about multi nand, * so we should tell it the real pagesize diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h index cec5712..5feb466 100644 --- a/drivers/mtd/nand/denali.h +++ b/drivers/mtd/nand/denali.h @@ -455,7 +455,7 @@ #define ECC_SECTOR_SIZE 512 -#define DENALI_BUF_SIZE(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) +#define DENALI_BUF_SIZE(8192 + 744) struct nand_buf { int head; diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c index 8b27522..082c880 100644 --- a/drivers/mtd/nand/fsmc_nand.c +++ b/drivers/mtd/nand/fsmc_nand.c @@ -719,8 +719,8 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip, int eccbytes = chip->ecc.bytes; int eccsteps = chip->ecc.steps; uint8_t *p = buf; - uint8_t *ecc_calc = chip->buffers->ecccalc; - uint8_t *ecc_code = chip->buffers->ecccode; + uint8_t *ecc_calc = chip->buffers.ecccalc; + uint8_t *ecc_code = chip->buffers.ecccode; int off, len, group = 0; /* * ecc_oob is intentionally taken as uint16_t. In 16bit devices, we diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index e2f5820..7618e71 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -1298,7 +1298,7 @@ static int mx23_check_transcripti
[PATCH] mtd: nand: remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE
From: Cai Zhiyong Date: Fri, 27 Dec 2013 20:23:09 +0800 Subject: [PATCH] mtd: nand: remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE remove NAND_MAX_OOBSIZE and NAND_MAX_PAGESIZE, dynamic allocate NAND buffer after detecting NAND writesize and oobsize. - save memory. when use MACRO allocate static buffer, use a small page size NAND chip, the buffer is still 8k, the memory is wasted. - reduce maintenance. NAND chip pagesize and oobsize update frequently, allocate buffer according to the detecting size will reduce maintenance. - this fix was suggested by Brian Norris [computersforpe...@gmail.com] and Huang Shijie [b32...@freescale.com] Signed-off-by: Cai Zhiyong --- drivers/mtd/nand/cafe_nand.c | 15 +++-- drivers/mtd/nand/denali.c | 9 - drivers/mtd/nand/denali.h | 2 +- drivers/mtd/nand/fsmc_nand.c | 4 +-- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 4 +-- drivers/mtd/nand/mxc_nand.c| 5 ++- drivers/mtd/nand/nand_base.c | 60 -- drivers/mtd/nand/nand_bbt.c| 2 +- drivers/mtd/nand/omap2.c | 6 ++-- include/linux/mtd/nand.h | 17 +++--- 10 files changed, 66 insertions(+), 58 deletions(-) diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c index c34985a..c784621 100644 --- a/drivers/mtd/nand/cafe_nand.c +++ b/drivers/mtd/nand/cafe_nand.c @@ -53,6 +53,9 @@ #define CAFE_GLOBAL_IRQ_MASK 0x300c #define CAFE_NAND_RESET0x3034 +#define CAFE_MAX_OOBSIZE 744 +#define CAFE_MAX_PAGESIZE 8192 + /* Missing from the datasheet: bit 19 of CTRL1 sets CE0 vs. CE1 */ #define CTRL1_CHIPSELECT (1<<19) @@ -657,13 +660,21 @@ static int cafe_nand_probe(struct pci_dev *pdev, err = -ENOMEM; goto out_free_mtd; } - cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, 2112 + sizeof(struct nand_buffers), + cafe->dmabuf = dma_alloc_coherent(&cafe->pdev->dev, + (2112 + CAFE_MAX_OOBSIZE * 3 + + CAFE_MAX_PAGESIZE), &cafe->dmaaddr, GFP_KERNEL); if (!cafe->dmabuf) { err = -ENOMEM; goto out_ior; } - cafe->nand.buffers = (void *)cafe->dmabuf + 2112; + + cafe->nand.buffers.buffer = (void *)cafe->dmabuf + 2112; + cafe->nand.buffers.ecccalc = cafe->nand.buffers.buffer; + cafe->nand.buffers.ecccode = cafe->nand.buffers.ecccalc + + CAFE_MAX_OOBSIZE; + cafe->nand.buffers.databuf = cafe->nand.buffers.ecccode + + CAFE_MAX_OOBSIZE; cafe->rs = init_rs_non_canonical(12, &cafe_mul, 0, 1, 8); if (!cafe->rs) { diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c index 3a3a47f..e86c995 100644 --- a/drivers/mtd/nand/denali.c +++ b/drivers/mtd/nand/denali.c @@ -1475,15 +1475,6 @@ int denali_init(struct denali_nand_info *denali) goto failed_req_irq; } - /* MTD supported page sizes vary by kernel. We validate our -* kernel supports the device here. -*/ - if (denali->mtd.writesize > NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) { - ret = -ENODEV; - pr_err("Spectra: device size not supported by this version of MTD."); - goto failed_req_irq; - } - /* support for multi nand * MTD known nothing about multi nand, * so we should tell it the real pagesize diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h index cec5712..5feb466 100644 --- a/drivers/mtd/nand/denali.h +++ b/drivers/mtd/nand/denali.h @@ -455,7 +455,7 @@ #define ECC_SECTOR_SIZE 512 -#define DENALI_BUF_SIZE(NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE) +#define DENALI_BUF_SIZE(8192 + 744) struct nand_buf { int head; diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c index 8b27522..082c880 100644 --- a/drivers/mtd/nand/fsmc_nand.c +++ b/drivers/mtd/nand/fsmc_nand.c @@ -719,8 +719,8 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip, int eccbytes = chip->ecc.bytes; int eccsteps = chip->ecc.steps; uint8_t *p = buf; - uint8_t *ecc_calc = chip->buffers->ecccalc; - uint8_t *ecc_code = chip->buffers->ecccode; + uint8_t *ecc_calc = chip->buffers.ecccalc; + uint8_t *ecc_code = chip->buffers.ecccode; int off, len, group = 0; /* * ecc_oob is intentionally taken as uint16_t. In 16bit devices, we diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index e2f5820..7618e71 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -1298,7 +1298,7 @@ static int mx23_check_transcripti
RE: [PATCH] block: add verifier for cmdline partition
> -Original Message- > From: Wang YanQing [mailto:udkni...@gmail.com] > Sent: Thursday, May 03, 2018 7:18 PM > To: ax...@kernel.dk > Cc: gre...@linuxfoundation.org; pombreda...@nexb.com; > t...@linutronix.de; Caizhiyong ; linux- > bl...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH] block: add verifier for cmdline partition > > I meet strange filesystem corruption issue recently, the reason > is there are overlaps partitions in cmdline partition argument. > > This patch add verifier for cmdline partition, then if there are > overlaps partitions, cmdline_partition will return error and log > a error message. > Partition overlap was intentionally designed in this cmdline partition. some time, the cmdline partition save raw data(not filesystem), the overlap makes data access very convenient. reference http://lists.infradead.org/pipermail/linux-mtd/2013-August/048092.html > Signed-off-by: Wang YanQing > --- > block/partitions/cmdline.c | 63 > +- > 1 file changed, 62 insertions(+), 1 deletion(-) >
RE: [PATCH v3] block: add verifier for cmdline partition
> /* > * Purpose: allocate cmdline partitions. > * Returns: > @@ -93,6 +158,7 @@ int cmdline_partition(struct parsed_partitions *state) > disk_size = get_capacity(state->bdev->bd_disk) << 9; > > cmdline_parts_set(parts, disk_size, 1, add_part, (void *)state); > + cmdline_parts_verifier(1, (void *)state); cmdline_parts_verifier(1, state); > > strlcat(state->pp_buf, "\n", PAGE_SIZE); > > -- > 1.8.5.6.2.g3d8a54e.dirty
RE: [PATCH] Documentation: block: cmdline-partition.txt fixes and additions
> -Original Message- > From: Randy Dunlap [mailto:rdun...@infradead.org] > Sent: Monday, May 07, 2018 2:50 AM > To: linux-bl...@vger.kernel.org; axboe > Cc: LKML ; Caizhiyong > ; Andrew Morton ; > linux-...@vger.kernel.org; Jonathan Corbet > Subject: [PATCH] Documentation: block: cmdline-partition.txt fixes and > additions > > From: Randy Dunlap > > Make the description of the kernel command line option "blkdevparts" > a bit more flowing and readable. > > Fix a few typos. > Add the optional and suffixes. > Note that size can be "-" to indicate all of the remaining space. > > Signed-off-by: Randy Dunlap > Cc: Cai Zhiyong > --- > Should the "ro" and "lk" flags options be described? thank you for fix typos. The "ro" and "lk" feature are not implemented in block devices. could you implement them? > Documentation/block/cmdline-partition.txt | 21 +--- > 1 file changed, 14 insertions(+), 7 deletions(-) > > --- linux-next-20180504.orig/Documentation/block/cmdline-partition.txt > +++ linux-next-20180504/Documentation/block/cmdline-partition.txt > @@ -1,7 +1,9 @@ > Embedded device command line partition parsing > > == > === > > -Support for reading the block device partition table from the command line. > +The "blkdevparts" command line option adds support for reading the > +block device partition table from the kernel command line. > + > It is typically used for fixed block (eMMC) embedded devices. > It has no MBR, so saves storage space. Bootloader can be easily accessed > by absolute address of data on the block device. > @@ -14,22 +16,27 @@ blkdevparts=[;] > := [@](part-name) > > > -block device disk name, embedded device used fixed block device, > -it's disk name also fixed. such as: mmcblk0, mmcblk1, mmcblk0boot0. > +block device disk name. Embedded device uses fixed block device. > +Its disk name is also fixed, such as: mmcblk0, mmcblk1, mmcblk0boot0. > > > partition size, in bytes, such as: 512, 1m, 1G. > +size may contain an optional suffix of (upper or lower case): > + K, M, G, T, P, E. > +"-" is used to denote all remaining space. > > > partition start address, in bytes. > +offset may contain an optional suffix of (upper or lower case): > + K, M, G, T, P, E. > > (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. > +partition name. Kernel sends 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. > > Example: > -eMMC disk name is "mmcblk0" and "mmcblk0boot0" > +eMMC disk names are "mmcblk0" and "mmcblk0boot0". > >bootargs: > 'blkdevparts=mmcblk0:1G(data0),1G(data1),-;mmcblk0boot0:1m(boot),- > (kernel)'
[PATCH] devfreq: fix double call put_device
From: Cai Zhiyong Date: Sat, 14 May 2016 14:13:30 +0800 Subject: [PATCH] devfreq: fix double call put_device 1295 */ 1296 void device_unregister(struct device *dev) 1297 { 1298 pr_debug("device: '%s': %s\n", dev_name(dev), __func__); 1299 device_del(dev); 1300 put_device(dev); 1301 } 1302 EXPORT_SYMBOL_GPL(device_unregister); 1303 device_unregister is called put_device, there is no need to call put_device(&devfreq->dev) again. Signed-off-by: Cai Zhiyong --- drivers/devfreq/devfreq.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 1d6c803..8de22c0 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -621,7 +621,6 @@ int devfreq_remove_device(struct devfreq *devfreq) return -EINVAL; device_unregister(&devfreq->dev); - put_device(&devfreq->dev); return 0; } -- 1.7.9.5
[PATCH] devfreq: fix double call put_device
From: Cai Zhiyong Date: Sat, 14 May 2016 14:13:30 +0800 Subject: [PATCH] devfreq: fix double call put_device 1295 */ 1296 void device_unregister(struct device *dev) 1297 { 1298 pr_debug("device: '%s': %s\n", dev_name(dev), __func__); 1299 device_del(dev); 1300 put_device(dev); 1301 } 1302 EXPORT_SYMBOL_GPL(device_unregister); 1303 device_unregister is called put_device, there is no need to call put_device(&devfreq->dev) again. Signed-off-by: Cai Zhiyong --- drivers/devfreq/devfreq.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 1d6c803..8de22c0 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -621,7 +621,6 @@ int devfreq_remove_device(struct devfreq *devfreq) return -EINVAL; device_unregister(&devfreq->dev); - put_device(&devfreq->dev); return 0; } -- 1.7.9.5
RE: [RESEND PATCH] block: cmdline-parser: add support for hidden disk space
Dear Shawn Lin: The cmdline-parser is support skip some reserve disk space, or hidden some disk space. See Documentation/block/cmdline-partition.txt blkdevparts=[;] := :[,] := [@](part-name) The "offset" is next partition start address, If your bootargs is 'blkdevparts=mmcblk0:1G(data0),1G@2G(data1),-;', you will reserve 1G between mmcblk0p1 and mmcblk0p2; Looking forward to your feedback. Best regards. Cai Zhiyong. http://www.huawei.com > -Original Message- > From: Shawn Lin [mailto:shawn@rock-chips.com] > Sent: Monday, September 28, 2015 5:51 PM > To: Jens Axboe > Cc: Caizhiyong; linux-kernel@vger.kernel.org; Shawn Lin > Subject: [RESEND PATCH] block: cmdline-parser: add support for hidden disk > space > > cmdline-partition now only support continuous disk space taken from > cmdline. When we need to reserve a disk space, for instance, 100m between > mmcblk0p1 and mmcblk0p2 for special use and wouldn't let kernel space > realize this "disk hole", we add this patch to ship adding this kind of > "disk hole" into the partition table. For any such cases, simply append > "hidden" to the end of subpart which is need to be reserved. > > Signed-off-by: Shawn Lin > > --- > > block/cmdline-parser.c | 12 +++- > include/linux/cmdline-parser.h | 1 + > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/block/cmdline-parser.c b/block/cmdline-parser.c > index 9dbc67e..fcca57d 100644 > --- a/block/cmdline-parser.c > +++ b/block/cmdline-parser.c > @@ -68,6 +68,11 @@ static int parse_subpart(struct cmdline_subpart > **subpart, char *partdef) > partdef += 2; > } > > + if (!strncmp(partdef, "hidden", 6)) { > + new_subpart->flags |= PF_HIDDEN; > + partdef += 6; > + } > + > *subpart = new_subpart; > return 0; > fail: > @@ -128,7 +133,9 @@ static int parse_parts(struct cmdline_parts **parts, > const char *bdevdef) > if (ret) > goto fail; > > - newparts->nr_subparts++; > + if (!((*next_subpart)->flags & PF_HIDDEN)) > + newparts->nr_subparts++; > + > next_subpart = &(*next_subpart)->next_subpart; > } > > @@ -245,6 +252,9 @@ int cmdline_parts_set(struct cmdline_parts *parts, > sector_t disk_size, > > from += subpart->size; > > + if (subpart->flags & PF_HIDDEN) > + continue; > + > if (add_part(slot, subpart, param)) > break; > } > diff --git a/include/linux/cmdline-parser.h b/include/linux/cmdline-parser.h > index 2e6dce6..7c1cb3c 100644 > --- a/include/linux/cmdline-parser.h > +++ b/include/linux/cmdline-parser.h > @@ -14,6 +14,7 @@ > /* partition flags */ > #define PF_RDONLY 0x01 /* Device is read only */ > #define PF_POWERUP_LOCK 0x02 /* Always locked after reset > */ > +#define PF_HIDDEN 0x04 /* Hidden from partition > table */ > > struct cmdline_subpart { > char name[BDEVNAME_SIZE]; /* partition name, such as 'rootfs' */ > -- > 2.3.7 > -- 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: [RESEND x2][PATCH v2] block: partition: Add partition specific uevent callbacks for partition info
> > Interestingly, this feature appears to already be documented in > > Documentation/block/cmdline-partition.txt. I wonder how that happened. > > Maybe we used to do this but it got taken away? This documentation talk about the partition name usages, my patch support get partition name from android userspace. the mainline kernel appears does not support the 'PARTNAME' uevent, but this feature is very convenient. > > Heh. Looks like the documentation was added not too long ago (by Cai - > cc'ed). I suspect they had been working w/ the Android tree and > assumed the functionality was already upstream? > > > It seems bad that we don't document uevents in any organized fashion. > > But the audience is very small and knows how to find kernel source code > > so I guess it doesn't matter. > > > > Anyway, please do check that the conveniently self-adding documentation > > is accurate and complete. > > It does match the behavior this patch provides from Android. It is > somewhat tangential to the functionality described in the > documentation, so I'm not sure of its measure of completeness (for > example, it doesn't talk about PARTN parameter, but again, the > documentation is covering how to specify partition info via the boot > cmdline, and isn't really covering the uevents - the uevent was just a > mentioned side-effect for the partition name portion of the cmdline > option). > > thanks > -john