You asked for review again... I guess I forgot about this patch series
for some time. I think this patch is OK, except for a trivial comment
below. But I have some comments on the next few.
On Sun, Nov 09, 2014 at 07:21:13AM -0500, Rodrigo Freire wrote:
> From: Felix Fietkau <n...@openwrt.org>
>
> mtd: block2mtd: Ensure that block2mtd is triggered after block devices are
> presented.
>
> Ensures that block2mtd is triggered after the block devices are enumerated
> at boot time.
> This issue is seen on BCM2835 (Raspberry Pi) systems when mounting JFFS2
> block2mtd filesystems, probably because of the delay on enumerating a USB
> MMC card reader.
>
> Signed-off-by: Felix Fietkau <n...@openwrt.org>
> Signed-off-by: Rodrigo Freire <rfre...@redhat.com>
> Signed-off-by: Herton Krzesinski <her...@redhat.com>
> ---
> V3: Separated on a single patch, fixes a compiling warning, cosmethic changes
> V2: Uses kstrdup, removed PAGE_MASK.
> --- a/drivers/mtd/devices/block2mtd.c 2014-11-07 16:40:14.638676860 -0200
> +++ b/drivers/mtd/devices/block2mtd.c 2014-11-07 17:44:45.277769924 -0200
[...]
> @@ -225,15 +237,28 @@ static struct block2mtd_dev *add_device(
>
> /* Get a handle on the device */
> bdev = blkdev_get_by_path(devname, mode, dev);
> -#ifndef MODULE
> - if (IS_ERR(bdev)) {
>
> - /* We might not have rootfs mounted at this point. Try
> - to resolve the device name by other means. */
> +#ifndef MODULE
> +/*
> + * We might not have the root device mounted at this point.
> + * Try to resolve the device name by other means.
> + */
That's some interesting comment indentation.
> + for (i = 0; IS_ERR(bdev) && i <= timeout; i++) {
> + dev_t devt;
>
> - dev_t devt = name_to_dev_t(devname);
> - if (devt)
> - bdev = blkdev_get_by_dev(devt, mode, dev);
> + if (i)
> + /*
> + * Calling wait_for_device_probe in the first loop
> + * was not enough, sleep for a bit in subsequent
> + * go-arounds.
> + */
> + msleep(1000);
> + wait_for_device_probe();
> +
> + devt = name_to_dev_t(devname);
> + if (!devt)
> + continue;
> + bdev = blkdev_get_by_dev(devt, mode, dev);
> }
> #endif
>
> @@ -280,6 +305,7 @@ static struct block2mtd_dev *add_device(
> /* Device didn't get added, so free the entry */
> goto err_destroy_mutex;
> }
> +
> list_add(&dev->list, &blkmtd_device_list);
> pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
> dev->mtd.index,
> @@ -348,16 +374,19 @@ static inline void kill_final_newline(ch
>
> #ifndef MODULE
> static int block2mtd_init_called = 0;
> -static char block2mtd_paramline[80 + 12]; /* 80 for device, 12 for erase
> size */
> +/* 80 for device, 12 for erase size */
> +static char block2mtd_paramline[80 + 12];
> #endif
>
> static int block2mtd_setup2(const char *val)
> {
> - char buf[80 + 12]; /* 80 for device, 12 for erase size */
> + /* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
> + char buf[80 + 12 + 80 + 8];
> char *str = buf;
> char *token[2];
> char *name;
> size_t erase_size = PAGE_SIZE;
> + unsigned long timeout = MTD_DEFAULT_TIMEOUT;
> int i, ret;
>
> if (strnlen(val, sizeof(buf)) >= sizeof(buf)) {
> @@ -395,7 +424,7 @@ static int block2mtd_setup2(const char *
> }
> }
>
> - add_device(name, erase_size);
> + add_device(name, erase_size, timeout);
>
> return 0;
> }
> @@ -463,8 +492,7 @@ static void block2mtd_exit(void)
> }
> }
>
> -
> -module_init(block2mtd_init);
> +late_initcall(block2mtd_init);
This technically could have problems if you want to use block2mtd with
UBI, now, since it also uses late_initcall(), and it can add UBI
attachments via module parameters too (ubi.mtd=<mtd-name>).
I'm not too worried about this, though, since block2mtd is really not
meant for serious use (despite your usage here).
> module_exit(block2mtd_exit);
>
> MODULE_LICENSE("GPL");
Pushed this patch to l2-mtd.git, with comment fixups.
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/
