[PATCH v2 3/4] mtd: nand: gpio: Return real nand_scan() error code on fail and simplify error path

[Brian Norris]

On Tue, Jul 30, 2013 at 4:06 AM, Alexander Shiyan <shc_work at mail.ru> wrote:
>
> Signed-off-by: Alexander Shiyan <shc_work at mail.ru>
> ---
>  drivers/mtd/nand/gpio.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
> index e432f1d..5eabd19 100644
> --- a/drivers/mtd/nand/gpio.c
> +++ b/drivers/mtd/nand/gpio.c
> @@ -212,7 +212,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
>         struct nand_chip *chip;
>         struct resource *res;
>         struct mtd_part_parser_data ppdata = {};
> -       int ret = 0;
> +       int ret;
>
>         if (!pdev->dev.of_node && !pdev->dev.platform_data)
>                 return -EINVAL;
> @@ -288,23 +288,20 @@ static int gpio_nand_probe(struct platform_device *pdev)
>
>         gpio_nand_set_wp(gpiomtd, 1);
>
> -       if (nand_scan(&gpiomtd->mtd_info, 1)) {
> -               ret = -ENXIO;
> -               goto err_wp;
> +       ret = nand_scan(&gpiomtd->mtd_info, 1);
> +       if (!ret) {
> +               if (gpiomtd->plat.adjust_parts)
> +                       gpiomtd->plat.adjust_parts(&gpiomtd->plat,
> +                                                  gpiomtd->mtd_info.size);
> +
> +               ppdata.of_node = pdev->dev.of_node;
> +               ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL,
> +                                               &ppdata, gpiomtd->plat.parts,
> +                                               gpiomtd->plat.num_parts);
> +               if (!ret)
> +                       return 0;
>         }
>
> -       if (gpiomtd->plat.adjust_parts)
> -               gpiomtd->plat.adjust_parts(&gpiomtd->plat,
> -                                          gpiomtd->mtd_info.size);
> -
> -       ppdata.of_node = pdev->dev.of_node;
> -       ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL, &ppdata,
> -                                       gpiomtd->plat.parts,
> -                                       gpiomtd->plat.num_parts);
> -       if (!ret)
> -               return 0;
> -
> -err_wp:
>         gpio_nand_set_wp(gpiomtd, 0);
>
>         return ret;

Does this really simplify the error path? It removes the 'goto', but
it increases indentation and makes this less scalable (i.e.,
currently, the average indentation level remains constant; under your
change, any additional initialization steps may require modifying the
indentation of unrelated steps). Plus, this doesn't really follow the
style of most other drivers.

This patch could be very simple; just something like this (with proper
whitespace, of course):

--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -279,10 +279,9 @@ static int gpio_nand_probe(struct platform_device *pdev)
        if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
                gpio_direction_output(gpiomtd->plat.gpio_nwp, 1);

-       if (nand_scan(&gpiomtd->mtd_info, 1)) {
-               ret = -ENXIO;
+       ret = nand_scan(&gpiomtd->mtd_info, 1);
+       if (ret)
                goto err_wp;
-       }

        if (gpiomtd->plat.adjust_parts)
                gpiomtd->plat.adjust_parts(&gpiomtd->plat,

BTW, if you think it makes more sense to return the error code from
nand_scan directly, could you change this for all other NAND drivers?
My reading shows there's approx a 50/50 split between those that
choose -ENXIO and those that just return the NAND error code.
Uniformity is good in this case, and nand_base.c gives sane error
codes, I believe.

Brian