| Message ID | 1456924001-1000-1-git-send-email-alexander.stein@systec-electronic.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2016-03-02 at 14:06 +0100, Alexander Stein wrote: > +config MTDRAM_WRITEBUF_SIZE > + int "MTDRAM write buf size in Bytes" > + depends on MTD_MTDRAM > + default "64" > + help > + This allows you to configure the write buffer size in the > device > + emulated by the MTDRAM driver. If the MTDRAM driver is > built > + as a module, it is also possible to specify this as a > parameter when > + loading the module. E.g. ubifs relies this in the recovery > algorithm. Why another Kconfig option (compile-time) instead of just a module parameter (run-time), which would be more flexible?
On Wed, 2016-03-02 at 16:32 +0200, Artem Bityutskiy wrote: > On Wed, 2016-03-02 at 14:06 +0100, Alexander Stein wrote: > > > > +config MTDRAM_WRITEBUF_SIZE > > + int "MTDRAM write buf size in Bytes" > > + depends on MTD_MTDRAM > > + default "64" > > + help > > + This allows you to configure the write buffer size in the > > device > > + emulated by the MTDRAM driver. If the MTDRAM driver is > > built > > + as a module, it is also possible to specify this as a > > parameter when > > + loading the module. E.g. ubifs relies this in the > > recovery > > algorithm. > Why another Kconfig option (compile-time) instead of just a module > parameter (run-time), which would be more flexible? Oh, pardon, you are adding both. I never found the Kconfig options very useful. Quite the opposite, I always thought MTD has too many of those little Kconfig options and the config menu is too large. I'd suggest to only add the runtime option, unless you really need the Kconfig option too (why?). -- Best Regards, Artem Bityutskiy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On Wednesday 02 March 2016 14:35:55, Bityutskiy, Artem wrote: > On Wed, 2016-03-02 at 16:32 +0200, Artem Bityutskiy wrote: > > On Wed, 2016-03-02 at 14:06 +0100, Alexander Stein wrote: > > > > > > +config MTDRAM_WRITEBUF_SIZE > > > + int "MTDRAM write buf size in Bytes" > > > + depends on MTD_MTDRAM > > > + default "64" > > > + help > > > + This allows you to configure the write buffer size in the > > > device > > > + emulated by the MTDRAM driver. If the MTDRAM driver is > > > built > > > + as a module, it is also possible to specify this as a > > > parameter when > > > + loading the module. E.g. ubifs relies this in the > > > recovery > > > algorithm. > > Why another Kconfig option (compile-time) instead of just a module > > parameter (run-time), which would be more flexible? > > Oh, pardon, you are adding both. I never found the Kconfig options very > useful. Quite the opposite, I always thought MTD has too many of those > little Kconfig options and the config menu is too large. I'd suggest to > only add the runtime option, unless you really need the Kconfig option > too (why?). I don't need it. Setting using modprobe is enough for me and a lot more flexible. I just kept it the same way as the other 2 parameters. Even those are rather useless in Kconfig, IMHO. Or do you prefer removing the existing Kconfig options all together and just rely on module parameters? Best regards, Alexander
On Wed, 2016-03-02 at 15:48 +0100, Alexander Stein wrote: > I don't need it. Setting using modprobe is enough for me and a lot > more flexible. I just kept it the same way as the other 2 parameters. > Even those are rather useless in Kconfig, IMHO. Or do you prefer > removing the existing Kconfig options all together and just rely on > module parameters? I'd recommend to avoid expanding the Kconfig, which is already complex and quite confusing for someone who is not an MTD expert but just tries to come up with a reasonable default Kconfig (e.g., and distro builder).
diff --git a/drivers/mtd/devices/Kconfig b/drivers/mtd/devices/Kconfig index f73c416..5431d75 100644 --- a/drivers/mtd/devices/Kconfig +++ b/drivers/mtd/devices/Kconfig @@ -171,6 +171,16 @@ config MTDRAM_ERASE_SIZE as a module, it is also possible to specify this as a parameter when loading the module. +config MTDRAM_WRITEBUF_SIZE + int "MTDRAM write buf size in Bytes" + depends on MTD_MTDRAM + default "64" + help + This allows you to configure the write buffer size in the device + emulated by the MTDRAM driver. If the MTDRAM driver is built + as a module, it is also possible to specify this as a parameter when + loading the module. E.g. ubifs relies this in the recovery algorithm. + #If not a module (I don't want to test it as a module) config MTDRAM_ABS_POS hex "SRAM Hexadecimal Absolute position or 0" diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c index 627a9bc..6977a66 100644 --- a/drivers/mtd/devices/mtdram.c +++ b/drivers/mtd/devices/mtdram.c @@ -19,14 +19,18 @@ static unsigned long total_size = CONFIG_MTDRAM_TOTAL_SIZE; static unsigned long erase_size = CONFIG_MTDRAM_ERASE_SIZE; +static unsigned long writebuf_size = CONFIG_MTDRAM_WRITEBUF_SIZE; #define MTDRAM_TOTAL_SIZE (total_size * 1024) #define MTDRAM_ERASE_SIZE (erase_size * 1024) +#define MTDRAM_WRITEBUF_SIZE (writebuf_size) #ifdef MODULE module_param(total_size, ulong, 0); MODULE_PARM_DESC(total_size, "Total device size in KiB"); module_param(erase_size, ulong, 0); MODULE_PARM_DESC(erase_size, "Device erase block size in KiB"); +module_param(writebuf_size, ulong, 0); +MODULE_PARM_DESC(writebuf_size, "Device write buf size in Bytes"); #endif // We could store these in the mtd structure, but we only support 1 device.. @@ -123,7 +127,7 @@ int mtdram_init_device(struct mtd_info *mtd, void *mapped_address, mtd->flags = MTD_CAP_RAM; mtd->size = size; mtd->writesize = 1; - mtd->writebufsize = 64; /* Mimic CFI NOR flashes */ + mtd->writebufsize = MTDRAM_WRITEBUF_SIZE; mtd->erasesize = MTDRAM_ERASE_SIZE; mtd->priv = mapped_address;