Re: [PATCH v2 3/7] net: rfkill: gpio: remove gpio conversion support

[Alexandre Courbot]

On Fri, Nov 22, 2013 at 9:14 PM, Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>
> All platforms using this driver are now converted to the new
> descriptor-based GPIO interface.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
>  net/rfkill/rfkill-gpio.c | 61 ++++++++++--------------------------------------
>  1 file changed, 12 insertions(+), 49 deletions(-)
>
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 503432154616..bd2a5b90400c 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -68,35 +68,6 @@ static const struct rfkill_ops rfkill_gpio_ops = {
>         .set_block = rfkill_gpio_set_power,
>  };
>
> -static int rfkill_gpio_convert_to_desc(struct platform_device *pdev,
> -                                      struct rfkill_gpio_data *rfkill)
> -{
> -       struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
> -       int ret;
> -
> -       if (gpio_is_valid(pdata->reset_gpio)) {
> -               ret = devm_gpio_request_one(&pdev->dev, pdata->reset_gpio,
> -                                           0, rfkill->reset_name);
> -               if (ret) {
> -                       dev_err(&pdev->dev, "failed to get reset gpio.\n");
> -                       return ret;
> -               }
> -               rfkill->reset_gpio = gpio_to_desc(pdata->reset_gpio);
> -       }
> -
> -       if (gpio_is_valid(pdata->shutdown_gpio)) {
> -               ret = devm_gpio_request_one(&pdev->dev, pdata->shutdown_gpio,
> -                                           0, rfkill->shutdown_name);
> -               if (ret) {
> -                       dev_err(&pdev->dev, "failed to get shutdown gpio.\n");
> -                       return ret;
> -               }
> -               rfkill->shutdown_gpio = gpio_to_desc(pdata->shutdown_gpio);
> -       }
> -
> -       return 0;
> -}
> -
>  static int rfkill_gpio_acpi_probe(struct device *dev,
>                                   struct rfkill_gpio_data *rfkill)
>  {
> @@ -117,6 +88,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>         struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
>         struct rfkill_gpio_data *rfkill;
>         const char *clk_name = NULL;
> +       struct gpio_desc *gpio;
>         int ret;
>         int len;
>
> @@ -150,29 +122,20 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>
>         rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
>
> -       if (pdata && (pdata->reset_gpio || pdata->shutdown_gpio)) {
> -               ret = rfkill_gpio_convert_to_desc(pdev, rfkill);
> +       gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
> +       if (!IS_ERR(gpio)) {
> +               ret = gpiod_direction_output(gpio, 0);
>                 if (ret)
>                         return ret;
> -       } else {
> -               struct gpio_desc *gpio;
> -
> -               gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
> -               if (!IS_ERR(gpio)) {
> -                       ret = gpiod_direction_output(gpio, 0);
> -                       if (ret)
> -                               return ret;
> -                       rfkill->reset_gpio = gpio;
> -               }
> +               rfkill->reset_gpio = gpio;
> +       }
>
> -               gpio = devm_gpiod_get_index(&pdev->dev,
> -                                           rfkill->shutdown_name, 1);
> -               if (!IS_ERR(gpio)) {
> -                       ret = gpiod_direction_output(gpio, 0);
> -                       if (ret)
> -                               return ret;
> -                       rfkill->shutdown_gpio = gpio;
> -               }
> +       gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
> +       if (!IS_ERR(gpio)) {
> +               ret = gpiod_direction_output(gpio, 0);
> +               if (ret)
> +                       return ret;
> +               rfkill->shutdown_gpio = gpio;
>         }
>
>         /* Make sure at-least one of the GPIO is defined and that

Wouldn't it be possible (and simpler) to move patch 2 of your series
to first position, and then to merge patch 1 and 3 together in second
position? It seems to me that you are basically undoing much of the
work of your first patch here (notably by removing
rfkill_gpio_convert_to_desc() which ends up having a very short life)
and that this could be avoided if you defined the platform lookup
tables first.

Doing so would avoid prevent you from using gpio_to_desc() which you
should never ever use anyway. :P
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/