Received-SPF: pass (google.com: domain of linux-kernel+bounces-37212-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <83ad61e2f9d62621f42d8738f6028103fe8bfb94.camel@crapouillou.net>
Subject: Re: [PATCH 19/21] gpio: swnode: replace gpiochip_find() with
 gpio_device_find_by_label()
From: Paul Cercueil <paul@crapouillou.net>
To: Bartosz Golaszewski <brgl@bgdev.pl>, Aaro Koskinen
 <aaro.koskinen@iki.fi>,  Janusz Krzysztofik <jmkrzyszt@gmail.com>, Tony
 Lindgren <tony@atomide.com>, Russell King <linux@armlinux.org.uk>, Mika
 Westerberg <mika.westerberg@linux.intel.com>,  Andy Shevchenko
 <andriy.shevchenko@linux.intel.com>, Linus Walleij
 <linus.walleij@linaro.org>, Dipen Patel <dipenp@nvidia.com>, Thierry Reding
 <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Hans de
 Goede <hdegoede@redhat.com>, Mark Gross <markgross@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, 
	linux-acpi@vger.kernel.org, timestamp@lists.linux.dev, 
	linux-tegra@vger.kernel.org, platform-driver-x86@vger.kernel.org, Bartosz
	Golaszewski <bartosz.golaszewski@linaro.org>
Date: Wed, 24 Jan 2024 15:59:22 +0100
In-Reply-To: <20230905185309.131295-20-brgl@bgdev.pl>
References: <20230905185309.131295-1-brgl@bgdev.pl>
	 <20230905185309.131295-20-brgl@bgdev.pl>
Autocrypt: addr=paul@crapouillou.net; prefer-encrypt=mutual;
 keydata=mQENBF0KhcEBCADkfmrzdTOp/gFOMQX0QwKE2WgeCJiHPWkpEuPH81/HB2dpjPZNW03ZMLQfECbbaEkdbN4YnPfXgcc1uBe5mwOAPV1MBlaZcEt4M67iYQwSNrP7maPS3IaQJ18ES8JJ5Uf5UzFZaUawgH+oipYGW+v31cX6L3k+dGsPRM0Pyo0sQt52fsopNPZ9iag0iY7dGNuKenaEqkYNjwEgTtNz8dt6s3hMpHIKZFL3OhAGi88wF/21isv0zkF4J0wlf9gYUTEEY3Eulx80PTVqGIcHZzfavlWIdzhe+rxHTDGVwseR2Y1WjgFGQ2F+vXetAB8NEeygXee+i9nY5qt9c07m8mzjABEBAAG0JFBhdWwgQ2VyY3VlaWwgPHBhdWxAY3JhcG91aWxsb3UubmV0PokBTgQTAQoAOBYhBNdHYd8OeCBwpMuVxnPua9InSr1BBQJdCoXBAhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEHPua9InSr1BgvIH/0kLyrI3V0f33a6D3BJwc1grbygPVYGuC5l5eMnAI+rDmLR19E2yvibRpgUc87NmPEQPpbbtAZt8On/2WZoE5OIPdlId/AHNpdgAtGXo0ZX4LGeVPjxjdkbrKVHxbcdcnY+zzaFglpbVSvp76pxqgVg8PgxkAAeeJV+ET4t0823Gz2HzCL/6JZhvKAEtHVulOWoBh368SYdolp1TSfORWmHzvQiCCCA+j0cMkYVGzIQzEQhX7Urf9N/nhU5/SGLFEi9DcBfXoGzhyQyLXflhJtKm3XGB1K/pPulbKaPcKAl6rIDWPuFpHkSbmZ9r4KFlBwgAhlGy6nqP7O3u7q23hRW5AQ0EXQqFwQEIAMo+MgvYHsyjX3Ja4Oolg1Txzm8woj30ch2nACFCqaO0R/1kLj2VVeLrDyQUOlXx9PD6IQI4M8wy8m0sR4wV2p/g/paw7k65cjzYYLh+FdLNyO7IW
	YXndJO+wDPi3aK/YKUYepqlP+QsmaHNYNdXEQDRKqNfJg8t0f5rfzp9ryxd1tCnbV+tG8VHQWiZXNqN7062DygSNXFUfQ0vZ3J2D4oAcIAEXTymRQ2+hr3Hf7I61KMHWeSkCvCG2decTYsHlw5Erix/jYWqVOtX0roOOLqWkqpQQJWtU+biWrAksmFmCp5fXIg1Nlg39v21xCXBGxJkxyTYuhdWyu1yDQ+LSIUAEQEAAYkBNgQYAQoAIBYhBNdHYd8OeCBwpMuVxnPua9InSr1BBQJdCoXBAhsMAAoJEHPua9InSr1B4wsH/Az767YCT0FSsMNt1jkkdLCBi7nY0GTW+PLP1a4zvVqFMo/vD6uz1ZflVTUAEvcTi3VHYZrlgjcxmcGu239oruqUS8Qy/xgZBp9KF0NTWQSl1iBfVbIU5VV1vHS6r77W5x0qXgfvAUWOH4gmN3MnF01SH2zMcLiaUGF+mcwl15rHbjnT3Nu2399aSE6cep86igfCAyFUOXjYEGlJy+c6UyT+DUylpjQg0nl8MlZ/7Whg2fAU9+FALIbQYQzGlT4c71SibR9T741jnegHhlmV4WXXUD6roFt54t0MSAFSVxzG8mLcSjR2cLUJ3NIPXixYUSEn3tQhfZj07xIIjWxAYZo=
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0

Hi Bartosz,

Le mardi 05 septembre 2023 =C3=A0 20:53 +0200, Bartosz Golaszewski a =C3=A9=
crit=C2=A0:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>=20
> We're porting all users of gpiochip_find() to using
> gpio_device_find().
> Update the swnode GPIO code.
>=20
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> =C2=A0drivers/gpio/gpiolib-swnode.c | 29 ++++++++++++-----------------
> =C2=A01 file changed, 12 insertions(+), 17 deletions(-)
>=20
> diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-
> swnode.c
> index b5a6eaf3729b..56c8519be538 100644
> --- a/drivers/gpio/gpiolib-swnode.c
> +++ b/drivers/gpio/gpiolib-swnode.c
> @@ -31,31 +31,26 @@ static void swnode_format_propname(const char
> *con_id, char *propname,
> =C2=A0		strscpy(propname, "gpios", max_size);
> =C2=A0}
> =C2=A0
> -static int swnode_gpiochip_match_name(struct gpio_chip *chip, void
> *data)
> +static struct gpio_device *swnode_get_gpio_device(struct
> fwnode_handle *fwnode)
> =C2=A0{
> -	return !strcmp(chip->label, data);
> -}
> +	const struct software_node *gdev_node;
> +	struct gpio_device *gdev;
> =C2=A0
> -static struct gpio_chip *swnode_get_chip(struct fwnode_handle
> *fwnode)
> -{
> -	const struct software_node *chip_node;
> -	struct gpio_chip *chip;
> -
> -	chip_node =3D to_software_node(fwnode);
> -	if (!chip_node || !chip_node->name)
> +	gdev_node =3D to_software_node(fwnode);
> +	if (!gdev_node || !gdev_node->name)
> =C2=A0		return ERR_PTR(-EINVAL);
> =C2=A0
> -	chip =3D gpiochip_find((void *)chip_node->name,
> swnode_gpiochip_match_name);
> -	return chip ?: ERR_PTR(-EPROBE_DEFER);
> +	gdev =3D gpio_device_find_by_label((void *)gdev_node->name);
> +	return gdev ?: ERR_PTR(-EPROBE_DEFER);
> =C2=A0}
> =C2=A0
> =C2=A0struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
> =C2=A0				=C2=A0=C2=A0 const char *con_id, unsigned int
> idx,
> =C2=A0				=C2=A0=C2=A0 unsigned long *flags)
> =C2=A0{
> +	struct gpio_device *gdev __free(gpio_device_put) =3D NULL;
> =C2=A0	const struct software_node *swnode;
> =C2=A0	struct fwnode_reference_args args;
> -	struct gpio_chip *chip;
> =C2=A0	struct gpio_desc *desc;
> =C2=A0	char propname[32]; /* 32 is max size of property name */
> =C2=A0	int error;
> @@ -77,12 +72,12 @@ struct gpio_desc *swnode_find_gpio(struct
> fwnode_handle *fwnode,
> =C2=A0		return ERR_PTR(error);
> =C2=A0	}
> =C2=A0
> -	chip =3D swnode_get_chip(args.fwnode);
> +	gdev =3D swnode_get_gpio_device(args.fwnode);
> =C2=A0	fwnode_handle_put(args.fwnode);
> -	if (IS_ERR(chip))
> -		return ERR_CAST(chip);
> +	if (IS_ERR(gdev))
> +		return ERR_CAST(gdev);

I'm a bit late to the party, sorry.

I'm looking at how __free() should be used to use it in my own
patchset, and I was wondering if this code actually works.

What happens if swnode_get_gpio_device() returns an error pointer?
Won't that cause a call to gpio_device_put() with the invalid pointer?

Cheers,
-Paul

> =C2=A0
> -	desc =3D gpiochip_get_desc(chip, args.args[0]);
> +	desc =3D gpiochip_get_desc(gdev->chip, args.args[0]);
> =C2=A0	*flags =3D args.args[1]; /* We expect native GPIO flags */
> =C2=A0
> =C2=A0	pr_debug("%s: parsed '%s' property of node '%pfwP[%d]' -
> status (%d)\n",