Re: [PATCH] [RFC] UBI: Implement Fastmap support

[Artem Bityutskiy <dedekind1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2725 bytes --]

Uhh, too many helpers.

On Mon, 2012-05-21 at 16:01 +0200, Richard Weinberger wrote:
>  /**
> - * ubi_attach - attach an MTD device.
> - * @ubi: UBI device descriptor
> + * scan_fastmap - attach MTD device using fastmap.
> + * @ubi: UBI device description object
>   *
> - * This function returns zero in case of success and a negative error code in
> - * case of failure.
> + * This function attaches a MTD device using a fastmap and returns complete
> + * information about it in form of a "struct ubi_attach_info" object. In case
> + * of failure, an error code is returned.
>   */
> -int ubi_attach(struct ubi_device *ubi)
> +static struct ubi_attach_info *scan_fastmap(struct ubi_device *ubi)
> +{
> +       int fm_start;
> +
> +       fm_start = ubi_find_fastmap(ubi);
> +       if (fm_start < 0)
> +               return ERR_PTR(-ENOENT);
> +
> +       return ubi_read_fastmap(ubi, fm_start);
> +}

This helper which does not do anything useful should not exist - just
teach 'ubi_read_fastmap()' to return 1 if the fastmap is not found, and
< 0 on error, and 0 on success. No one in attach.c should be interested
in fm_start. So this helper should die. See below as well.

> +
> +static int do_attach(struct ubi_device *ubi, bool fullscan)
>  {
>         int err;
>         struct ubi_attach_info *ai;
>  
> -       ai = scan_all(ubi);
> +       if (fullscan)
> +               ai = scan_all(ubi);
> +       else
> +               ai = scan_fastmap(ubi);
> +
>         if (IS_ERR(ai))
>                 return PTR_ERR(ai);
>  
> @@ -1256,6 +1277,31 @@ out_ai:
>  }

Another useless helper function, should die, see below.

>  
>  /**
> + * ubi_attach - attach an MTD device.
> + * @ubi: UBI device descriptor
> + *
> + * This function returns zero in case of success and a negative error code in
> + * case of failure.
> + */
> +int ubi_attach(struct ubi_device *ubi)
> +{
> +       int err;
> +
> +       err = do_attach(ubi, false);
> +       if (err) {
> +               if (err != -ENOENT)
> +                       ubi_err("Attach by fastmap failed! " \
> +                               "Falling back to attach by scanning. " \
> +                               "error = %i\n", err);
> +
> +               ubi->attached_by_scanning = true;
> +               err = do_attach(ubi, true);
> +       }
> +
> +       return err;
> +} 

Just add this code to this function:

       err = ubi_read_fastmap();
       if (err < 0)
               return err;
       if (err == 2)
               return scan_all();

       ... rest of the common nitializations (EBA, WL)...

       return err;

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]