MIME-Version: 1.0
References: <20230130131003.668888-1-arnd@kernel.org>
In-Reply-To: <20230130131003.668888-1-arnd@kernel.org>
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date:   Tue, 31 Jan 2023 19:29:37 -0800
Message-ID: <CAKdAkRQt15PNbwzDv0sciE5xfQ8LT76cyPepzmMdNsAYg1P1vA@mail.gmail.com>
Subject: Re: [PATCH] media: c8sectpfe: convert to gpio descriptors
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Patrice Chotard <patrice.chotard@foss.st.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Hans Verkuil <hverkuil-cisco@xs4all.nl>,
        Hugues Fruchet <hugues.fruchet@st.com>,
        linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jan 30, 2023 at 5:31 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The gpio usage in the function is fairly straightforward,
> but the normal gpiod_get() interface cannot be used here
> since the gpio is referenced by a child node of the device.
>
> Using devm_fwnode_gpiod_get_index() is the best alternative
> here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  .../st/sti/c8sectpfe/c8sectpfe-core.c         | 30 ++++++++-----------
>  .../st/sti/c8sectpfe/c8sectpfe-core.h         |  2 +-
>  2 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
> index c38b62d4f1ae..86a2c77c5471 100644
> --- a/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
> +++ b/drivers/media/platform/st/sti/c8sectpfe/c8sectpfe-core.c
> @@ -22,7 +22,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> -#include <linux/of_gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/of_platform.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/pinctrl/pinctrl.h>
> @@ -812,30 +812,24 @@ static int c8sectpfe_probe(struct platform_device *pdev)
>                 }
>                 of_node_put(i2c_bus);
>
> -               tsin->rst_gpio = of_get_named_gpio(child, "reset-gpios", 0);
> -
> -               ret = gpio_is_valid(tsin->rst_gpio);
> -               if (!ret) {
> -                       dev_err(dev,
> -                               "reset gpio for tsin%d not valid (gpio=%d)\n",
> -                               tsin->tsin_id, tsin->rst_gpio);
> -                       ret = -EINVAL;
> -                       goto err_node_put;
> -               }
> -
> -               ret = devm_gpio_request_one(dev, tsin->rst_gpio,
> -                                       GPIOF_OUT_INIT_LOW, "NIM reset");
> +               tsin->rst_gpio = devm_fwnode_gpiod_get_index(dev,
> +                                                            of_node_to_fwnode(child),
> +                                                            "reset-gpios",

Wrong name.

> +                                                            0, GPIOD_OUT_LOW,
> +                                                            "NIM reset");
> +               ret = PTR_ERR_OR_ZERO(tsin->rst_gpio);
>                 if (ret && ret != -EBUSY) {
> -                       dev_err(dev, "Can't request tsin%d reset gpio\n"
> -                               , fei->channel_data[index]->tsin_id);
> +                       dev_err_probe(dev, ret,
> +                                     "reset gpio for tsin%d not valid\n",
> +                                     tsin->tsin_id);
>                         goto err_node_put;
>                 }
>
>                 if (!ret) {
>                         /* toggle reset lines */
> -                       gpio_direction_output(tsin->rst_gpio, 0);
> +                       gpiod_direction_output(tsin->rst_gpio, 0);

It is already set up as output, and you either need to use "raw" or
fix polarity.

>                         usleep_range(3500, 5000);
> -                       gpio_direction_output(tsin->rst_gpio, 1);
> +                       gpiod_direction_output(tsin->rst_gpio, 1);
>                         usleep_range(3000, 5000);
>                 }

Thanks.

-- 
Dmitry