[PATCH] drm/bridge: it66121: acquire reset GPIO in probe

[PATCH] drm/bridge: it66121: acquire reset GPIO in probe

[Julien Chauveau]

The it66121_ctx structure has a gpio_reset field, and it66121_hw_reset()
calls gpiod_set_value() on it. However, the GPIO descriptor is never
acquired via devm_gpiod_get(), leaving gpio_reset as NULL throughout
the driver lifetime.

gpiod_set_value() silently returns when passed a NULL descriptor, so
the hardware reset sequence in it66121_hw_reset() is a no-op. This
leaves the chip in an undefined state at probe time, which can prevent
it from responding on the I2C bus.

The DT binding marks reset-gpios as a required property, so all
compliant device trees provide this GPIO. Add the missing
devm_gpiod_get() call after enabling power supplies and before the
hardware reset, so the chip is properly reset with power applied.

Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Julien Chauveau <chauveau.julien@gmail.com>
---
 drivers/gpu/drm/bridge/ite-it66121.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -1559,6 +1559,11 @@ static int it66121_probe(struct i2c_client *client)
 		return ret;
 	}
 
+	ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->gpio_reset))
+		return dev_err_probe(dev, PTR_ERR(ctx->gpio_reset),
+				     "Failed to get reset GPIO\n");
+
 	it66121_hw_reset(ctx);

Re: [PATCH] drm/bridge: it66121: acquire reset GPIO in probe

[Javier Martinez Canillas]

Julien Chauveau <chauveau.julien@gmail.com> writes:

Hello Julien,

> The it66121_ctx structure has a gpio_reset field, and it66121_hw_reset()
> calls gpiod_set_value() on it. However, the GPIO descriptor is never
> acquired via devm_gpiod_get(), leaving gpio_reset as NULL throughout
> the driver lifetime.
>
> gpiod_set_value() silently returns when passed a NULL descriptor, so
> the hardware reset sequence in it66121_hw_reset() is a no-op. This
> leaves the chip in an undefined state at probe time, which can prevent
> it from responding on the I2C bus.
>
> The DT binding marks reset-gpios as a required property, so all
> compliant device trees provide this GPIO. Add the missing
> devm_gpiod_get() call after enabling power supplies and before the
> hardware reset, so the chip is properly reset with power applied.
>
> Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Julien Chauveau <chauveau.julien@gmail.com>
> ---

The patch looks good to me. I've also tested it on my BeaglePlay board.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] drm/bridge: it66121: acquire reset GPIO in probe

[Javier Martinez Canillas]

Javier Martinez Canillas <javierm@redhat.com> writes:

> Julien Chauveau <chauveau.julien@gmail.com> writes:
>
> Hello Julien,
>
>> The it66121_ctx structure has a gpio_reset field, and it66121_hw_reset()
>> calls gpiod_set_value() on it. However, the GPIO descriptor is never
>> acquired via devm_gpiod_get(), leaving gpio_reset as NULL throughout
>> the driver lifetime.
>>
>> gpiod_set_value() silently returns when passed a NULL descriptor, so
>> the hardware reset sequence in it66121_hw_reset() is a no-op. This
>> leaves the chip in an undefined state at probe time, which can prevent
>> it from responding on the I2C bus.
>>
>> The DT binding marks reset-gpios as a required property, so all
>> compliant device trees provide this GPIO. Add the missing
>> devm_gpiod_get() call after enabling power supplies and before the
>> hardware reset, so the chip is properly reset with power applied.
>>
>> Fixes: 988156dc2fc9 ("drm: bridge: add it66121 driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Julien Chauveau <chauveau.julien@gmail.com>
>> ---
>
> The patch looks good to me. I've also tested it on my BeaglePlay board.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
>

Pushed to drm-misc (drm-misc-fixes). Thanks!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat