| Message ID | 5ff5120f2b4ef6442a1d7c05916a772ec59a8c34.1706182805.git.u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pwm: Improve lifetime tracking for pwm_chips | expand |
Hi, On Thu, Jan 25, 2024 at 4:11 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> 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 <u.kleine-koenig@pengutronix.de> > --- > 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 <dianders@chromium.org> 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?
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 > <u.kleine-koenig@pengutronix.de> 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 <u.kleine-koenig@pengutronix.de> > > --- > > 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 <dianders@chromium.org> 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 &chip->dev). See patches #3 and #109. Best regards Uwe
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; }
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 <u.kleine-koenig@pengutronix.de> --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)