Re: [PATCH v5 037/111] drm/bridge: ti-sn65dsi86: Make use of pwmchip_parent() macro

2024-01-25 Thread Uwe Kleine-König 
Hello Doug,

On Thu, Jan 25, 2024 at 09:47:42AM -0800, Doug Anderson wrote:
> On Thu, Jan 25, 2024 at 4:11 AM Uwe Kleine-König
>  wrote:
> >
> > struct pwm_chip::dev is about to change. To not have to touch this
> > driver in the same commit as struct pwm_chip::dev, use the macro
> > provided for exactly this purpose.
> >
> > Signed-off-by: Uwe Kleine-König 
> > ---
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> This seems OK with me. Unless someone more senior in the drm-misc
> community contradicts me, feel free to take this through your tree.
> 
> Acked-by: Douglas Anderson 

Thanks.
 
> NOTE: though the patch seems OK to me, I have one small concern. If I
> understand correctly, your eventual goal is to add a separate "dev"
> for the PWM chip without further changes to the ti-sn65dsi86 driver.
> If that's true, you'll have to find some way to magically call
> devm_pm_runtime_enable() on the new "dev" since the code you have here
> is calling pm_runtime functions on what will eventually be this new
> "dev". Maybe you'll do something like enabling runtime PM on it
> automatically if its parent had runtime PM enabled?

The idea is that the pwmchip_parent macro always returns the pwmchip's
parent. So when this patch gets applied, we have

+static inline struct device *pwmchip_parent(struct pwm_chip *chip)
{
   return chip->dev;
}

and when the pwmchip gets its own struct device, it is adapted to return
chip->dev.parent (and not >dev). See patches #3 and #109.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature

Re: [PATCH v5 037/111] drm/bridge: ti-sn65dsi86: Make use of pwmchip_parent() macro

2024-01-25 Thread Doug Anderson 
Hi,

On Thu, Jan 25, 2024 at 4:11 AM Uwe Kleine-König
 wrote:
>
> struct pwm_chip::dev is about to change. To not have to touch this
> driver in the same commit as struct pwm_chip::dev, use the macro
> provided for exactly this purpose.
>
> Signed-off-by: Uwe Kleine-König 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

This seems OK with me. Unless someone more senior in the drm-misc
community contradicts me, feel free to take this through your tree.

Acked-by: Douglas Anderson 

NOTE: though the patch seems OK to me, I have one small concern. If I
understand correctly, your eventual goal is to add a separate "dev"
for the PWM chip without further changes to the ti-sn65dsi86 driver.
If that's true, you'll have to find some way to magically call
devm_pm_runtime_enable() on the new "dev" since the code you have here
is calling pm_runtime functions on what will eventually be this new
"dev". Maybe you'll do something like enabling runtime PM on it
automatically if its parent had runtime PM enabled?

[PATCH v5 037/111] drm/bridge: ti-sn65dsi86: Make use of pwmchip_parent() macro

2024-01-25 Thread Uwe Kleine-König 
struct pwm_chip::dev is about to change. To not have to touch this
driver in the same commit as struct pwm_chip::dev, use the macro
provided for exactly this purpose.

Signed-off-by: Uwe Kleine-König 
---
 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1f6e929c2f6a..f1fffbef3324 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -1415,7 +1415,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(chip->dev);
+   ret = pm_runtime_resume_and_get(pwmchip_parent(chip));
if (ret < 0)
return ret;
}
@@ -1431,7 +1431,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(chip->dev, "failed to mux in PWM 
function\n");
+   dev_err(pwmchip_parent(chip), "failed to mux in 
PWM function\n");
goto out;
}
}
@@ -1507,7 +1507,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(chip->dev, "failed to update PWM_PRE_DIV\n");
+   dev_err(pwmchip_parent(chip), "failed to update 
PWM_PRE_DIV\n");
goto out;
}
 
@@ -1519,7 +1519,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(chip->dev, "failed to update PWM_EN/PWM_INV\n");
+   dev_err(pwmchip_parent(chip), "failed to update 
PWM_EN/PWM_INV\n");
goto out;
}
 
@@ -1527,7 +1527,7 @@ static int ti_sn_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
 out:
 
if (!pdata->pwm_enabled)
-   pm_runtime_put_sync(chip->dev);
+   pm_runtime_put_sync(pwmchip_parent(chip));
 
return ret;
 }
-- 
2.43.0