diff mbox series

[v3] drm/bridge: ti-sn65dsi86: Associate PWM device to auxiliary device

Message ID 20231209153108.1988551-2-u.kleine-koenig@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [v3] drm/bridge: ti-sn65dsi86: Associate PWM device to auxiliary device | expand

Commit Message

Uwe Kleine-König Dec. 9, 2023, 3:31 p.m. UTC
It's the ti_sn65dsi86.pwm auxiliary driver that creates the pwmchip, so
let the auxiliary device be the parent of the pwm device.

Note that getting a reference to the ti-sn65dsi86's pwm using pwm_get()
isn't affected by this change as ti_sn65dsi86_add_aux_device() sets the
auxiliary device's of_node to that of the main device.

Also change PM runtime tracking and diagnostic messages to use that one.
After enabling runtime PM operation for the auxiliary device, all works
as expected as parent devices are handled just fine.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since v2
(https://lore.kernel.org/dri-devel/20231209152520.1987483-2-u.kleine-koenig@pengutronix.de):

 - Make use of devm_pm_runtime_enable as suggested by Douglas Anderson
   in reply to v1 already. (Sorry, missed that while preparing v2 :-\)

Changes since (implicit) v1
(https://lore.kernel.org/dri-devel/20231127101547.734061-2-u.kleine-koenig@pengutronix.de):

 - Add a call to pm_runtime_enable() for the aux device
   (tested and diagnosed by Nikita Travkin).
 - Rebased to yesterday's next, which required some (easy) conflict
   resolution for commit c9d99c73940e ("drm/bridge: ti-sn65dsi86:
   Simplify using pm_runtime_resume_and_get()").

Best regards
Uwe

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)


base-commit: bc63de6e6ba0b16652c5fb4b9c9916b9e7ca1f23

Comments

Nikita Travkin Dec. 9, 2023, 4:07 p.m. UTC | #1
Uwe Kleine-König писал(а) 09.12.2023 20:31:
> It's the ti_sn65dsi86.pwm auxiliary driver that creates the pwmchip, so
> let the auxiliary device be the parent of the pwm device.
> 
> Note that getting a reference to the ti-sn65dsi86's pwm using pwm_get()
> isn't affected by this change as ti_sn65dsi86_add_aux_device() sets the
> auxiliary device's of_node to that of the main device.
> 
> Also change PM runtime tracking and diagnostic messages to use that one.
> After enabling runtime PM operation for the auxiliary device, all works
> as expected as parent devices are handled just fine.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This works well now, thanks!

Tested-by: Nikita Travkin <nikita@trvn.ru> # Acer Aspire 1

Nikita

> ---
> Changes since v2
> (https://lore.kernel.org/dri-devel/20231209152520.1987483-2-u.kleine-koenig@pengutronix.de):
> 
>  - Make use of devm_pm_runtime_enable as suggested by Douglas Anderson
>    in reply to v1 already. (Sorry, missed that while preparing v2 :-\)
> 
> Changes since (implicit) v1
> (https://lore.kernel.org/dri-devel/20231127101547.734061-2-u.kleine-koenig@pengutronix.de):
> 
>  - Add a call to pm_runtime_enable() for the aux device
>    (tested and diagnosed by Nikita Travkin).
>  - Rebased to yesterday's next, which required some (easy) conflict
>    resolution for commit c9d99c73940e ("drm/bridge: ti-sn65dsi86:
>    Simplify using pm_runtime_resume_and_get()").
> 
> Best regards
> Uwe
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 5b8e1dfc458d..9095d1453710 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -1413,7 +1413,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  	int ret;
>  
>  	if (!pdata->pwm_enabled) {
> -		ret = pm_runtime_resume_and_get(pdata->dev);
> +		ret = pm_runtime_resume_and_get(chip->dev);
>  		if (ret < 0)
>  			return ret;
>  	}
> @@ -1429,7 +1429,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  						 SN_GPIO_MUX_MASK << (2 * SN_PWM_GPIO_IDX),
>  						 SN_GPIO_MUX_SPECIAL << (2 * SN_PWM_GPIO_IDX));
>  			if (ret) {
> -				dev_err(pdata->dev, "failed to mux in PWM function\n");
> +				dev_err(chip->dev, "failed to mux in PWM function\n");
>  				goto out;
>  			}
>  		}
> @@ -1505,7 +1505,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  
>  		ret = regmap_write(pdata->regmap, SN_PWM_PRE_DIV_REG, pre_div);
>  		if (ret) {
> -			dev_err(pdata->dev, "failed to update PWM_PRE_DIV\n");
> +			dev_err(chip->dev, "failed to update PWM_PRE_DIV\n");
>  			goto out;
>  		}
>  
> @@ -1517,7 +1517,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  		     FIELD_PREP(SN_PWM_INV_MASK, state->polarity == PWM_POLARITY_INVERSED);
>  	ret = regmap_write(pdata->regmap, SN_PWM_EN_INV_REG, pwm_en_inv);
>  	if (ret) {
> -		dev_err(pdata->dev, "failed to update PWM_EN/PWM_INV\n");
> +		dev_err(chip->dev, "failed to update PWM_EN/PWM_INV\n");
>  		goto out;
>  	}
>  
> @@ -1525,7 +1525,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
>  out:
>  
>  	if (!pdata->pwm_enabled)
> -		pm_runtime_put_sync(pdata->dev);
> +		pm_runtime_put_sync(chip->dev);
>  
>  	return ret;
>  }
> @@ -1585,12 +1585,14 @@ static int ti_sn_pwm_probe(struct auxiliary_device *adev,
>  {
>  	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
>  
> -	pdata->pchip.dev = pdata->dev;
> +	pdata->pchip.dev = &adev->dev;
>  	pdata->pchip.ops = &ti_sn_pwm_ops;
>  	pdata->pchip.npwm = 1;
>  	pdata->pchip.of_xlate = of_pwm_single_xlate;
>  	pdata->pchip.of_pwm_n_cells = 1;
>  
> +	devm_pm_runtime_enable(&adev->dev);
> +
>  	return pwmchip_add(&pdata->pchip);
>  }
>  
> @@ -1601,7 +1603,7 @@ static void ti_sn_pwm_remove(struct auxiliary_device *adev)
>  	pwmchip_remove(&pdata->pchip);
>  
>  	if (pdata->pwm_enabled)
> -		pm_runtime_put_sync(pdata->dev);
> +		pm_runtime_put_sync(&adev->dev);
>  }
>  
>  static const struct auxiliary_device_id ti_sn_pwm_id_table[] = {
> 
> base-commit: bc63de6e6ba0b16652c5fb4b9c9916b9e7ca1f23
Doug Anderson Dec. 11, 2023, 4:25 p.m. UTC | #2
Hi,

On Sat, Dec 9, 2023 at 7:31 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> It's the ti_sn65dsi86.pwm auxiliary driver that creates the pwmchip, so
> let the auxiliary device be the parent of the pwm device.
>
> Note that getting a reference to the ti-sn65dsi86's pwm using pwm_get()
> isn't affected by this change as ti_sn65dsi86_add_aux_device() sets the
> auxiliary device's of_node to that of the main device.
>
> Also change PM runtime tracking and diagnostic messages to use that one.
> After enabling runtime PM operation for the auxiliary device, all works
> as expected as parent devices are handled just fine.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Changes since v2
> (https://lore.kernel.org/dri-devel/20231209152520.1987483-2-u.kleine-koenig@pengutronix.de):
>
>  - Make use of devm_pm_runtime_enable as suggested by Douglas Anderson
>    in reply to v1 already. (Sorry, missed that while preparing v2 :-\)
>
> Changes since (implicit) v1
> (https://lore.kernel.org/dri-devel/20231127101547.734061-2-u.kleine-koenig@pengutronix.de):
>
>  - Add a call to pm_runtime_enable() for the aux device
>    (tested and diagnosed by Nikita Travkin).
>  - Rebased to yesterday's next, which required some (easy) conflict
>    resolution for commit c9d99c73940e ("drm/bridge: ti-sn65dsi86:
>    Simplify using pm_runtime_resume_and_get()").
>
> Best regards
> Uwe
>
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I'll also note that, via IRC, Steev also confirmed that "on the c630
with the devm_pm.... display works too"

Pushed to drm-misc-next:

eb3f7cbee294 drm/bridge: ti-sn65dsi86: Associate PWM device to auxiliary device
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 5b8e1dfc458d..9095d1453710 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -1413,7 +1413,7 @@  static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	int ret;
 
 	if (!pdata->pwm_enabled) {
-		ret = pm_runtime_resume_and_get(pdata->dev);
+		ret = pm_runtime_resume_and_get(chip->dev);
 		if (ret < 0)
 			return ret;
 	}
@@ -1429,7 +1429,7 @@  static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 						 SN_GPIO_MUX_MASK << (2 * SN_PWM_GPIO_IDX),
 						 SN_GPIO_MUX_SPECIAL << (2 * SN_PWM_GPIO_IDX));
 			if (ret) {
-				dev_err(pdata->dev, "failed to mux in PWM function\n");
+				dev_err(chip->dev, "failed to mux in PWM function\n");
 				goto out;
 			}
 		}
@@ -1505,7 +1505,7 @@  static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 		ret = regmap_write(pdata->regmap, SN_PWM_PRE_DIV_REG, pre_div);
 		if (ret) {
-			dev_err(pdata->dev, "failed to update PWM_PRE_DIV\n");
+			dev_err(chip->dev, "failed to update PWM_PRE_DIV\n");
 			goto out;
 		}
 
@@ -1517,7 +1517,7 @@  static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		     FIELD_PREP(SN_PWM_INV_MASK, state->polarity == PWM_POLARITY_INVERSED);
 	ret = regmap_write(pdata->regmap, SN_PWM_EN_INV_REG, pwm_en_inv);
 	if (ret) {
-		dev_err(pdata->dev, "failed to update PWM_EN/PWM_INV\n");
+		dev_err(chip->dev, "failed to update PWM_EN/PWM_INV\n");
 		goto out;
 	}
 
@@ -1525,7 +1525,7 @@  static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 out:
 
 	if (!pdata->pwm_enabled)
-		pm_runtime_put_sync(pdata->dev);
+		pm_runtime_put_sync(chip->dev);
 
 	return ret;
 }
@@ -1585,12 +1585,14 @@  static int ti_sn_pwm_probe(struct auxiliary_device *adev,
 {
 	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
 
-	pdata->pchip.dev = pdata->dev;
+	pdata->pchip.dev = &adev->dev;
 	pdata->pchip.ops = &ti_sn_pwm_ops;
 	pdata->pchip.npwm = 1;
 	pdata->pchip.of_xlate = of_pwm_single_xlate;
 	pdata->pchip.of_pwm_n_cells = 1;
 
+	devm_pm_runtime_enable(&adev->dev);
+
 	return pwmchip_add(&pdata->pchip);
 }
 
@@ -1601,7 +1603,7 @@  static void ti_sn_pwm_remove(struct auxiliary_device *adev)
 	pwmchip_remove(&pdata->pchip);
 
 	if (pdata->pwm_enabled)
-		pm_runtime_put_sync(pdata->dev);
+		pm_runtime_put_sync(&adev->dev);
 }
 
 static const struct auxiliary_device_id ti_sn_pwm_id_table[] = {