diff mbox series

[v2,3/3] drm/bridge: parade-ps8640: Perform full poweroff if poweron fails

Message ID 20211102093618.114928-3-angelogioacchino.delregno@collabora.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/3] drm/bridge: parade-ps8640: Don't try to enable VDO if poweron fails | expand

Commit Message

AngeloGioacchino Del Regno Nov. 2, 2021, 9:36 a.m. UTC
In function ps8640_bridge_poweron(), in case of a failure not relative
to the regulators enablement, the code was disabling the regulators but
the gpio changes that happened during the poweron sequence were not
being reverted back to a clean poweroff state.

Since it is expected that, when we enter ps8640_bridge_poweron(), the
powerdown and reset GPIOs are both in active state exactly as they were
left in the poweroff function before, we can simply call function
__ps8640_bridge_poweroff() in the failure case, reverting every change
that was done during the power on sequence.

Of course it was chosen to call the poweroff function instead of adding
code to revert the GPIO changes to the poweron one to avoid duplicating
code, as we would be doing exactly what the poweroff function does.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/bridge/parade-ps8640.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Dafna Hirschfeld Nov. 10, 2021, 12:48 p.m. UTC | #1
On 02.11.21 11:36, AngeloGioacchino Del Regno wrote:
> In function ps8640_bridge_poweron(), in case of a failure not relative
> to the regulators enablement, the code was disabling the regulators but
> the gpio changes that happened during the poweron sequence were not
> being reverted back to a clean poweroff state.
> 
> Since it is expected that, when we enter ps8640_bridge_poweron(), the
> powerdown and reset GPIOs are both in active state exactly as they were
> left in the poweroff function before, we can simply call function
> __ps8640_bridge_poweroff() in the failure case, reverting every change
> that was done during the power on sequence.
> 
> Of course it was chosen to call the poweroff function instead of adding
> code to revert the GPIO changes to the poweron one to avoid duplicating
> code, as we would be doing exactly what the poweroff function does.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Reviewed-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

> ---
>   drivers/gpu/drm/bridge/parade-ps8640.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
> index 41f5d511d516..ef1b51d8b676 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -344,7 +344,7 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
>   
>   	if (ret < 0) {
>   		DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", ret);
> -		goto err_regulators_disable;
> +		goto err_poweroff;
>   	}
>   
>   	msleep(50);
> @@ -360,23 +360,22 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
>   	ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
>   	if (ret < 0) {
>   		DRM_ERROR("failed write PAGE2_MCS_EN: %d\n", ret);
> -		goto err_regulators_disable;
> +		goto err_poweroff;
>   	}
>   
>   	/* Switch access edp panel's edid through i2c */
>   	ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
>   	if (ret < 0) {
>   		DRM_ERROR("failed write PAGE2_I2C_BYPASS: %d\n", ret);
> -		goto err_regulators_disable;
> +		goto err_poweroff;
>   	}
>   
>   	ps_bridge->powered = true;
>   
>   	return 0;
>   
> -err_regulators_disable:
> -	regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
> -			       ps_bridge->supplies);
> +err_poweroff:
> +	__ps8640_bridge_poweroff(ps_bridge);
>   
>   	return ret;
>   }
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c
index 41f5d511d516..ef1b51d8b676 100644
--- a/drivers/gpu/drm/bridge/parade-ps8640.c
+++ b/drivers/gpu/drm/bridge/parade-ps8640.c
@@ -344,7 +344,7 @@  static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
 
 	if (ret < 0) {
 		DRM_ERROR("failed read PAGE2_GPIO_H: %d\n", ret);
-		goto err_regulators_disable;
+		goto err_poweroff;
 	}
 
 	msleep(50);
@@ -360,23 +360,22 @@  static int ps8640_bridge_poweron(struct ps8640 *ps_bridge)
 	ret = regmap_update_bits(map, PAGE2_MCS_EN, MCS_EN, 0);
 	if (ret < 0) {
 		DRM_ERROR("failed write PAGE2_MCS_EN: %d\n", ret);
-		goto err_regulators_disable;
+		goto err_poweroff;
 	}
 
 	/* Switch access edp panel's edid through i2c */
 	ret = regmap_write(map, PAGE2_I2C_BYPASS, I2C_BYPASS_EN);
 	if (ret < 0) {
 		DRM_ERROR("failed write PAGE2_I2C_BYPASS: %d\n", ret);
-		goto err_regulators_disable;
+		goto err_poweroff;
 	}
 
 	ps_bridge->powered = true;
 
 	return 0;
 
-err_regulators_disable:
-	regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies),
-			       ps_bridge->supplies);
+err_poweroff:
+	__ps8640_bridge_poweroff(ps_bridge);
 
 	return ret;
 }