| Message ID | 20220125123429.3490883-1-max.kellermann@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3] pwm-sun4i: convert "next_period" to local variable | expand |
Hello, On Tue, Jan 25, 2022 at 01:34:27PM +0100, Max Kellermann wrote: > Its value is calculated in sun4i_pwm_apply() and is used only there. > > Cc: stable@vger.kernel.org I think I'd drop this. This isn't a fix worth on it's own to be backported and if this is needed for one of the next patches, the stable maintainers will notice themselves (and it might be worth to shuffle this series to make the fixes come first). > Signed-off-by: Max Kellermann <max.kellermann@gmail.com> Other than that, LGTM: Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Thanks Uwe
On 2022/01/25 15:31, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > I think I'd drop this. This isn't a fix worth on it's own to be > backported and if this is needed for one of the next patches, the stable > maintainers will notice themselves (and it might be worth to shuffle > this series to make the fixes come first). The first two patches are preparation for the third patch, which fixes the actual bug. Of course, I could have done everything in one patch, but I thought splitting the first two out makes review easier. This way, every step is almost trivial. If you want me to fold the three patches into one, I can do that. But I can't reorder them (or backport only the bug fix to stable). Max
On Tue, Jan 25, 2022 at 03:39:16PM +0100, Max Kellermann wrote: > On 2022/01/25 15:31, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > I think I'd drop this. This isn't a fix worth on it's own to be > > backported and if this is needed for one of the next patches, the stable > > maintainers will notice themselves (and it might be worth to shuffle > > this series to make the fixes come first). > > The first two patches are preparation for the third patch, which fixes > the actual bug. > > Of course, I could have done everything in one patch, but I thought > splitting the first two out makes review easier. This way, every step > is almost trivial. > > If you want me to fold the three patches into one, I can do that. But > I can't reorder them (or backport only the bug fix to stable). That sounds fine. Note my statement "I'd drop this" only refers to the Cc: stable line. Best regards Uwe
On Tue, Jan 25, 2022 at 03:31:58PM +0100, Uwe Kleine-König wrote: > Hello, > > On Tue, Jan 25, 2022 at 01:34:27PM +0100, Max Kellermann wrote: > > Its value is calculated in sun4i_pwm_apply() and is used only there. > > > > Cc: stable@vger.kernel.org > > I think I'd drop this. This isn't a fix worth on it's own to be > backported and if this is needed for one of the next patches, the stable > maintainers will notice themselves (and it might be worth to shuffle > this series to make the fixes come first). > > > Signed-off-by: Max Kellermann <max.kellermann@gmail.com> > > Other than that, LGTM: > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Does that apply to patches 2 & 3 as well? Thierry
On Thu, Feb 24, 2022 at 02:04:23PM +0100, Thierry Reding wrote: > On Tue, Jan 25, 2022 at 03:31:58PM +0100, Uwe Kleine-König wrote: > > Hello, > > > > On Tue, Jan 25, 2022 at 01:34:27PM +0100, Max Kellermann wrote: > > > Its value is calculated in sun4i_pwm_apply() and is used only there. > > > > > > Cc: stable@vger.kernel.org > > > > I think I'd drop this. This isn't a fix worth on it's own to be > > backported and if this is needed for one of the next patches, the stable > > maintainers will notice themselves (and it might be worth to shuffle > > this series to make the fixes come first). > > > > > Signed-off-by: Max Kellermann <max.kellermann@gmail.com> > > > > Other than that, LGTM: > > > > Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > Does that apply to patches 2 & 3 as well? No, at that time I only looked at patch 1. I just looked at 2 and 3 and will reply to them individually. Best regards Uwe
On Tue, Jan 25, 2022 at 01:34:27PM +0100, Max Kellermann wrote: > Its value is calculated in sun4i_pwm_apply() and is used only there. > > Cc: stable@vger.kernel.org > Signed-off-by: Max Kellermann <max.kellermann@gmail.com> > --- > drivers/pwm/pwm-sun4i.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) Patches applied, though I dropped the Cc: stable on patches 1 and 2. Thanks, Thierry
diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c index 91ca67651abd..c7c564a6bf36 100644 --- a/drivers/pwm/pwm-sun4i.c +++ b/drivers/pwm/pwm-sun4i.c @@ -89,7 +89,6 @@ struct sun4i_pwm_chip { void __iomem *base; spinlock_t ctrl_lock; const struct sun4i_pwm_data *data; - unsigned long next_period[2]; }; static inline struct sun4i_pwm_chip *to_sun4i_pwm_chip(struct pwm_chip *chip) @@ -237,6 +236,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, int ret; unsigned int delay_us, prescaler = 0; unsigned long now; + unsigned long next_period; bool bypass; pwm_get_state(pwm, &cstate); @@ -284,7 +284,7 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, val = (duty & PWM_DTY_MASK) | PWM_PRD(period); sun4i_pwm_writel(sun4i_pwm, val, PWM_CH_PRD(pwm->hwpwm)); - sun4i_pwm->next_period[pwm->hwpwm] = jiffies + + next_period = jiffies + nsecs_to_jiffies(cstate.period + 1000); if (state->polarity != PWM_POLARITY_NORMAL) @@ -306,9 +306,8 @@ static int sun4i_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, /* We need a full period to elapse before disabling the channel. */ now = jiffies; - if (time_before(now, sun4i_pwm->next_period[pwm->hwpwm])) { - delay_us = jiffies_to_usecs(sun4i_pwm->next_period[pwm->hwpwm] - - now); + if (time_before(now, next_period)) { + delay_us = jiffies_to_usecs(next_period - now); if ((delay_us / 500) > MAX_UDELAY_MS) msleep(delay_us / 1000 + 1); else
Its value is calculated in sun4i_pwm_apply() and is used only there. Cc: stable@vger.kernel.org Signed-off-by: Max Kellermann <max.kellermann@gmail.com> --- drivers/pwm/pwm-sun4i.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)