diff mbox

[2/5] mtd: block2mtd: Add support for specifying MTD write size and subpage shift

Message ID 1496418222-23483-3-git-send-email-pali.rohar@gmail.com
State Rejected
Delegated to: Boris Brezillon
Headers show

Commit Message

Pali Rohár June 2, 2017, 3:43 p.m. UTC
It is needed for creating emulated devices suitable for using in UBI layer
and with UBIFS.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/mtd/devices/block2mtd.c |   55 ++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 13 deletions(-)

Comments

Richard Weinberger June 2, 2017, 4:13 p.m. UTC | #1
Pali,

Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> It is needed for creating emulated devices suitable for using in UBI layer
> and with UBIFS.

Why?

It is not clear to me why kind of MTD you are constructing.
subpages are NAND specific you create RAM and ROM MTDs.
Sure, the MTD allows such constructs but I'm not sure whether this is a good idea.

Thanks,
//richard
Pali Rohár June 5, 2017, 11:21 a.m. UTC | #2
On Friday 02 June 2017 18:13:02 Richard Weinberger wrote:
> Pali,
> 
> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> > It is needed for creating emulated devices suitable for using in UBI layer
> > and with UBIFS.
> 
> Why?

ubifs depends on write size of nand. And without those parameters as
specified in cover letter I'm unable to mount N900 rootfs image exported
via block2mtd. ubifs reject such image.
Richard Weinberger June 5, 2017, 11:23 a.m. UTC | #3
Pali,

Am 05.06.2017 um 13:21 schrieb Pali Rohár:
> On Friday 02 June 2017 18:13:02 Richard Weinberger wrote:
>> Pali,
>>
>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
>>> It is needed for creating emulated devices suitable for using in UBI layer
>>> and with UBIFS.
>>
>> Why?
> 
> ubifs depends on write size of nand. And without those parameters as
> specified in cover letter I'm unable to mount N900 rootfs image exported
> via block2mtd. ubifs reject such image.

Hmm, so you render block2mtd into a semi-NAND chip? :)

Thanks,
//richard
Pali Rohár June 5, 2017, 11:25 a.m. UTC | #4
On Monday 05 June 2017 13:23:22 Richard Weinberger wrote:
> Pali,
> 
> Am 05.06.2017 um 13:21 schrieb Pali Rohár:
> > On Friday 02 June 2017 18:13:02 Richard Weinberger wrote:
> >> Pali,
> >>
> >> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> >>> It is needed for creating emulated devices suitable for using in UBI layer
> >>> and with UBIFS.
> >>
> >> Why?
> > 
> > ubifs depends on write size of nand. And without those parameters as
> > specified in cover letter I'm unable to mount N900 rootfs image exported
> > via block2mtd. ubifs reject such image.
> 
> Hmm, so you render block2mtd into a semi-NAND chip? :)

Probably you can call it like that. But it is still MTD device...
Richard Weinberger June 5, 2017, 11:27 a.m. UTC | #5
Pali,

Am 05.06.2017 um 13:25 schrieb Pali Rohár:
> On Monday 05 June 2017 13:23:22 Richard Weinberger wrote:
>> Pali,
>>
>> Am 05.06.2017 um 13:21 schrieb Pali Rohár:
>>> On Friday 02 June 2017 18:13:02 Richard Weinberger wrote:
>>>> Pali,
>>>>
>>>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
>>>>> It is needed for creating emulated devices suitable for using in UBI layer
>>>>> and with UBIFS.
>>>>
>>>> Why?
>>>
>>> ubifs depends on write size of nand. And without those parameters as
>>> specified in cover letter I'm unable to mount N900 rootfs image exported
>>> via block2mtd. ubifs reject such image.
>>
>> Hmm, so you render block2mtd into a semi-NAND chip? :)
> 
> Probably you can call it like that. But it is still MTD device...

This is what I meant in my other mail.
You add NAND specific properties but still denote it as MTD_RAM/ROM.
I'm not sure whether this is a good idea.

Thanks,
//richard
Pali Rohár June 7, 2017, 8:46 a.m. UTC | #6
On Monday 05 June 2017 13:27:18 Richard Weinberger wrote:
> Pali,
> 
> Am 05.06.2017 um 13:25 schrieb Pali Rohár:
> > On Monday 05 June 2017 13:23:22 Richard Weinberger wrote:
> >> Pali,
> >>
> >> Am 05.06.2017 um 13:21 schrieb Pali Rohár:
> >>> On Friday 02 June 2017 18:13:02 Richard Weinberger wrote:
> >>>> Pali,
> >>>>
> >>>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> >>>>> It is needed for creating emulated devices suitable for using in UBI layer
> >>>>> and with UBIFS.
> >>>>
> >>>> Why?
> >>>
> >>> ubifs depends on write size of nand. And without those parameters as
> >>> specified in cover letter I'm unable to mount N900 rootfs image exported
> >>> via block2mtd. ubifs reject such image.
> >>
> >> Hmm, so you render block2mtd into a semi-NAND chip? :)
> > 
> > Probably you can call it like that. But it is still MTD device...
> 
> This is what I meant in my other mail.
> You add NAND specific properties but still denote it as MTD_RAM/ROM.
> I'm not sure whether this is a good idea.

Ok, lets wait what other people think.

At least patches like fallback or check should be less problematic and
could be applied separately.
Pavel Machek June 18, 2017, 10:06 a.m. UTC | #7
Hi!

> >>>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> >>>>> It is needed for creating emulated devices suitable for using in UBI layer
> >>>>> and with UBIFS.
> >>>>
> >>>> Why?
> >>>
> >>> ubifs depends on write size of nand. And without those parameters as
> >>> specified in cover letter I'm unable to mount N900 rootfs image exported
> >>> via block2mtd. ubifs reject such image.
> >>
> >> Hmm, so you render block2mtd into a semi-NAND chip? :)
> > 
> > Probably you can call it like that. But it is still MTD device...
> 
> This is what I meant in my other mail.
> You add NAND specific properties but still denote it as MTD_RAM/ROM.
> I'm not sure whether this is a good idea.

Can you suggest any other way for Pali to mount rootfs image on his
PC?

									Pavel
Richard Weinberger June 18, 2017, 10:11 a.m. UTC | #8
Pavel,

Am 18.06.2017 um 12:06 schrieb Pavel Machek:
> Hi!
> 
>>>>>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
>>>>>>> It is needed for creating emulated devices suitable for using in UBI layer
>>>>>>> and with UBIFS.
>>>>>>
>>>>>> Why?
>>>>>
>>>>> ubifs depends on write size of nand. And without those parameters as
>>>>> specified in cover letter I'm unable to mount N900 rootfs image exported
>>>>> via block2mtd. ubifs reject such image.
>>>>
>>>> Hmm, so you render block2mtd into a semi-NAND chip? :)
>>>
>>> Probably you can call it like that. But it is still MTD device...
>>
>> This is what I meant in my other mail.
>> You add NAND specific properties but still denote it as MTD_RAM/ROM.
>> I'm not sure whether this is a good idea.
> 
> Can you suggest any other way for Pali to mount rootfs image on his
> PC?

Are you my micro manager? ;-)

As stated in my other mail, we have nandsim and there is work going on
to allow specifying arbitrary NAND sizes.
An alternative approach can be found here:
https://lkml.org/lkml/2016/5/12/296

Thanks,
//richard
Pali Rohár July 17, 2017, 12:34 p.m. UTC | #9
On Sunday 18 June 2017 12:11:40 Richard Weinberger wrote:
> Pavel,
> 
> Am 18.06.2017 um 12:06 schrieb Pavel Machek:
> > Hi!
> > 
> >>>>>> Am 02.06.2017 um 17:43 schrieb Pali Rohár:
> >>>>>>> It is needed for creating emulated devices suitable for using in UBI layer
> >>>>>>> and with UBIFS.
> >>>>>>
> >>>>>> Why?
> >>>>>
> >>>>> ubifs depends on write size of nand. And without those parameters as
> >>>>> specified in cover letter I'm unable to mount N900 rootfs image exported
> >>>>> via block2mtd. ubifs reject such image.
> >>>>
> >>>> Hmm, so you render block2mtd into a semi-NAND chip? :)
> >>>
> >>> Probably you can call it like that. But it is still MTD device...
> >>
> >> This is what I meant in my other mail.
> >> You add NAND specific properties but still denote it as MTD_RAM/ROM.
> >> I'm not sure whether this is a good idea.
> > 
> > Can you suggest any other way for Pali to mount rootfs image on his
> > PC?
> 
> Are you my micro manager? ;-)
> 
> As stated in my other mail, we have nandsim and there is work going on
> to allow specifying arbitrary NAND sizes.
> An alternative approach can be found here:
> https://lkml.org/lkml/2016/5/12/296
> 
> Thanks,
> //richard

Back to the this patch series. There are also other patches in this
series. If you at least comment/review other patches if this one is
"problematic"?
diff mbox

Patch

diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index ee47cdd..5ba5fad 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -3,6 +3,7 @@ 
  *
  * Copyright (C) 2001,2002	Simon Evans <spse@secret.org.uk>
  * Copyright (C) 2004-2006	Joern Engel <joern@wh.fh-wedel.de>
+ * Copyright (C) 2012-2017	Pali Rohár <pali.rohar@gmail.com>
  *
  * Licence: GPL
  */
@@ -218,8 +219,8 @@  static void block2mtd_free_device(struct block2mtd_dev *dev)
 }
 
 
-static struct block2mtd_dev *add_device(char *devname, int erase_size,
-		int timeout)
+static struct block2mtd_dev *add_device(char *devname, uint32_t erase_size,
+		uint32_t write_size, int subpage_sft, int timeout)
 {
 #ifndef MODULE
 	int i;
@@ -279,6 +280,11 @@  static struct block2mtd_dev *add_device(char *devname, int erase_size,
 		goto err_free_block2mtd;
 	}
 
+	if ((long)dev->blkdev->bd_inode->i_size % write_size) {
+		pr_err("writesize must be divisor of device size\n");
+		goto err_free_block2mtd;
+	}
+
 	mutex_init(&dev->write_mutex);
 
 	/* Setup the MTD structure */
@@ -291,7 +297,8 @@  static struct block2mtd_dev *add_device(char *devname, int erase_size,
 
 	dev->mtd.size = dev->blkdev->bd_inode->i_size & PAGE_MASK;
 	dev->mtd.erasesize = erase_size;
-	dev->mtd.writesize = 1;
+	dev->mtd.writesize = write_size;
+	dev->mtd.subpage_sft = subpage_sft;
 	dev->mtd.writebufsize = PAGE_SIZE;
 	dev->mtd.type = MTD_RAM;
 	dev->mtd.flags = MTD_CAP_RAM;
@@ -308,10 +315,12 @@  static struct block2mtd_dev *add_device(char *devname, int erase_size,
 	}
 
 	list_add(&dev->list, &blkmtd_device_list);
-	pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
+	pr_info("mtd%d: [%s] erase_size = %dKiB [%d], write_size = %dKiB [%d], subpage_sft = %d\n",
 		dev->mtd.index,
 		dev->mtd.name + strlen("block2mtd: "),
-		dev->mtd.erasesize >> 10, dev->mtd.erasesize);
+		dev->mtd.erasesize >> 10, dev->mtd.erasesize,
+		dev->mtd.writesize >> 10, dev->mtd.writesize,
+		dev->mtd.subpage_sft);
 	return dev;
 
 err_destroy_mutex:
@@ -375,18 +384,20 @@  static inline void kill_final_newline(char *str)
 
 #ifndef MODULE
 static int block2mtd_init_called = 0;
-/* 80 for device, 12 for erase size */
-static char block2mtd_paramline[80 + 12];
+/* 80 for device, 12 for erase size, 12 for write size, 3 for subpage_sft */
+static char block2mtd_paramline[80 + 12 + 12 + 3];
 #endif
 
 static int block2mtd_setup2(const char *val)
 {
-	/* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
-	char buf[80 + 12 + 80 + 8];
+	/* 80 for name, 80 for device, 12 for erase size, 12 for write size, 3 for subpage_sft */
+	char buf[80 + 12 + 80 + 12 + 3];
 	char *str = buf;
-	char *token[2];
+	char *token[4];
 	char *name;
 	size_t erase_size = PAGE_SIZE;
+	size_t write_size = 1;
+	size_t subpage_sft = 0;
 	unsigned long timeout = MTD_DEFAULT_TIMEOUT;
 	int i, ret;
 
@@ -398,7 +409,7 @@  static int block2mtd_setup2(const char *val)
 	strcpy(str, val);
 	kill_final_newline(str);
 
-	for (i = 0; i < 2; i++)
+	for (i = 0; i < 4; i++)
 		token[i] = strsep(&str, ",");
 
 	if (str) {
@@ -425,7 +436,23 @@  static int block2mtd_setup2(const char *val)
 		}
 	}
 
-	add_device(name, erase_size, timeout);
+	if (token[2]) {
+		ret = parse_num(&write_size, token[2]);
+		if (ret || write_size > U32_MAX) {
+			pr_err("illegal write size");
+			return 0;
+		}
+	}
+
+	if (token[3]) {
+		ret = parse_num(&subpage_sft, token[3]);
+		if (ret || subpage_sft > INT_MAX) {
+			pr_err("illegal subpage_sft");
+			return 0;
+		}
+	}
+
+	add_device(name, erase_size, write_size, subpage_sft, timeout);
 
 	return 0;
 }
@@ -459,7 +486,9 @@  static int block2mtd_setup(const char *val, struct kernel_param *kp)
 
 
 module_param_call(block2mtd, block2mtd_setup, NULL, NULL, 0200);
-MODULE_PARM_DESC(block2mtd, "Device to use. \"block2mtd=<dev>[,<erasesize>]\"");
+MODULE_PARM_DESC(block2mtd, "Block device, erase size (default page size), "
+	"write size (default 1), nand subpage shift (default 0) "
+	"\"block2mtd=<dev>[,<erasesize>[,<writesize>[,<subpage_sft>]]]\"");
 
 static int __init block2mtd_init(void)
 {