[PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[Alexander Stein]

ubifs uses the write buffer size in recovery algorithm. When inspecting
an unclean ubifs recovery fails with writebuf size 64 in mtdram while
recovery on actual mtd device with writebuf size of 1024 succeeds.
So add a parameter for setting this property.

Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
Reviewed-by: Richard Weinberger <richard@nod.at>
---
Changes in v2:
* Added Reviewed-by: Richard Weinberger <richard@nod.at>
* Removed outdated comment about CFI NOR flash write buffer sizes

 drivers/mtd/devices/Kconfig  | 10 ++++++++++
 drivers/mtd/devices/mtdram.c |  6 +++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

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;
 
-- 
2.4.10

Re: [PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[Artem Bityutskiy]

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?

Re: [PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[Bityutskiy, Artem]

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.

Re: [PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[Alexander Stein]

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
-- 
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
alexander.stein@systec-electronic.com

Legal and Commercial Address:
Am Windrad 2
08468 Heinsdorfergrund
Germany

Office: +49 (0) 3765 38600-0
Fax:    +49 (0) 3765 38600-4100
 
Managing Directors:
	Director Technology/CEO: Dipl.-Phys. Siegmar Schmidt;
	Director Commercial Affairs/COO: Dipl. Ing. (FH) Armin von Collrepp
Commercial Registry:
	Amtsgericht Chemnitz, HRB 28082; USt.-Id Nr. DE150534010

Re: [PATCH v2 1/1] mtd: mtdram: Add parameter for setting writebuf size

[Bityutskiy, Artem]

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).

-- 
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.