lkml.org 
[lkml]   [2015]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/3] mtd: block2mtd: Ensure that block2mtd is triggered after block devices are presented.
    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 <nbd@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 <nbd@openwrt.org>
    > Signed-off-by: Rodrigo Freire <rfreire@redhat.com>
    > Signed-off-by: Herton Krzesinski <herton@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


    \
     
     \ /
      Last update: 2015-02-24 09:01    [W:4.418 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site