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@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#L...
""" 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