[PATCH] gpio: pca953x_gpio: support optional reset-gpios property

[Quentin Schulz]

Hi Rasmus,

On 4/10/24 13:11, Rasmus Villemoes wrote:
> The DT bindings for the pca953x family has an optional reset-gpios
> property. If present, ensure that the device is taken out of reset
> before attempting to read from it.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>   drivers/gpio/pca953x_gpio.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c
> index b0c66d18317..24b0732f89a 100644
> --- a/drivers/gpio/pca953x_gpio.c
> +++ b/drivers/gpio/pca953x_gpio.c
> @@ -306,6 +306,7 @@ static int pca953x_probe(struct udevice *dev)
>   	struct pca953x_info *info = dev_get_plat(dev);
>   	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>   	char name[32], label[8], *str;
> +	struct gpio_desc reset;
>   	int addr;
>   	ulong driver_data;
>   	int ret;
> @@ -321,6 +322,13 @@ static int pca953x_probe(struct udevice *dev)
>   
>   	driver_data = dev_get_driver_data(dev);
>   
> +	/* If a reset-gpios property is present, take the device out of reset. */
> +	ret = gpio_request_by_name(dev, "reset-gpios", 0, &reset, GPIOD_IS_OUT);
> +	if (ret && ret != -ENOENT) {

This seems to differ from the implementation we have for optionally 
getting gpios by index, c.f. 
https://elixir.bootlin.com/u-boot/latest/source/drivers/gpio/gpio-uclass.c#L1498

"""
struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
				       unsigned int index, int flags)
{
[...]
	rc = gpio_request_by_name(dev, propname, index, desc, flags);

end:
[...]
	if (rc)
		return ERR_PTR(rc);
[...]
	return desc;
}

struct gpio_desc *devm_gpiod_get_index_optional(struct udevice *dev,
						const char *id,
						unsigned int index,
						int flags)
{
	struct gpio_desc *desc = devm_gpiod_get_index(dev, id, index, flags);

	if (IS_ERR(desc))
		return NULL;

	return desc;
}
"""

It seems we only need to check whether rc is non-zero, but it doesn't 
check that it's not ENOENT. I think we would benefit from having the 
same logic here.

Also, maybe we need a devm_gpio_get_by_name_optional implementation in 
the subsystem so we don't have to reimplement it in drivers that want to 
use this?

Cheers,
Quentin