Richard, would you please split this series differently: 1. Separate out the calculations to the get_bad_peb_limit() func. 2. Invent 2. Add the module parameter 3. Extends the ioctl 4. Removes the Kconfig option This will be much easier to review. See also some comments below. > --- a/drivers/mtd/ubi/build.c > +++ b/drivers/mtd/ubi/build.c > @@ -46,6 +46,8 @@ > /* Maximum length of the 'mtd=' parameter */ > #define MTD_PARAM_LEN_MAX 64 > > +#define MTD_PARAM_NB_MAX 3 Please, make it to be /* Maximum number of comma-separated items in ht 'mtd=' parameter */ #define MTD_PARAM_MAX_COUNT 3 instead. > @@ -57,10 +59,12 @@ > * @name: MTD character device node path, MTD device name, or MTD device number > * string > * @vid_hdr_offs: VID header offset > + * @max_beb_per1024: maximum expected number of bad blocks per 1024 erase blocks > */ > struct mtd_dev_param { > char name[MTD_PARAM_LEN_MAX]; > int vid_hdr_offs; > + unsigned int max_beb_per1024; > }; Please, make it to be just 'int' here and everywhere, just for consistency with other parameters, which are 'int' (no other stronger reason). > MODULE_PARM_DESC(mtd, "MTD devices to attach. Parameter format: " > - "mtd=[,].\n" > + "mtd=[,[,max_beb_per1024]].\n" > "Multiple \"mtd\" parameters may be specified.\n" > "MTD devices may be specified by their number, name, or " > "path to the MTD character device node.\n" > "Optional \"vid_hdr_offs\" parameter specifies UBI VID " > "header position to be used by UBI.\n" This comment needs improvement. Consider a situation when I do not want to specify vid_hdr_offs (want to use the default), but want to specify max_beb_per1024. I think I can put 0 here which means "default". Would you please verify this and add a comment about this in this help output. Also, please, verify that the output looks OK using 'modinfo ubi'. > + "Optional \"max_beb_per1024\" parameter specifies the " > + "maximum expected bad eraseblock per 1024 eraseblocks." > + "(default " __stringify(MTD_UBI_DEFAULT_BEB_LIMIT) > + " if 0 or not set)\n" Yeah, something like "if 0 or not set" is needed for 'vid_hdr_offs' as well. > "Example 1: mtd=/dev/mtd0 - attach MTD device " > "/dev/mtd0.\n" > "Example 2: mtd=content,1984 mtd=4 - attach MTD device " Could you also modify one of the examples and add "max_beb_per1024" there? -- Best Regards, Artem Bityutskiy