Hello,
On Fri, Aug 05, 2022 at 05:50:33PM +0100, Ben Dooks wrote:
> If we are not in PWM mode, then the output is technically a 50%
> output based on a single timer instead of the high-low based on
> the two counters. Add a check for the PWM mode in dwc_pwm_get_state()
> and if DWC_TIM_CTRL_PWM is not set, then return a 50% cycle.
>
> This may only be an issue on initialisation, as the rest of the
> code currently assumes we're always going to have the extended
> PWM mode using two counters.
>
> Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxx>
> ---
> drivers/pwm/pwm-dwc.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index 5edfb8f8acbf..49e666be7afd 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -171,23 +171,35 @@ static void dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> {
> struct dwc_pwm *dwc = to_dwc_pwm(chip);
> u64 duty, period;
> + u32 ctrl, ld, ld2;
>
> pm_runtime_get_sync(chip->dev);
>
> - state->enabled = !!(dwc_pwm_readl(dwc,
> - DWC_TIM_CTRL(pwm->hwpwm)) & DWC_TIM_CTRL_EN);
> + ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
> + ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
> + ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
>
> - duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
> - duty += 1;
> - duty *= dwc->clk_ns;
> - state->duty_cycle = duty;
> + state->enabled = !!(ctrl & DWC_TIM_CTRL_EN);
>
> - period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
> - period += 1;
> - period *= dwc->clk_ns;
> - period += duty;
> - state->period = period;
> + /* If we're not in PWM, technically the output is a 50-50
Huh, I expected checkpatch to warn about that. AFAIK the /* is supposed
to be on a line for itself?!
> + * based on the timer load-count only.
> + */
> + if (ctrl & DWC_TIM_CTRL_PWM) {
> + duty = ld;
> + duty += 1;
> + duty *= dwc->clk_ns;
I would prefer to write that as:
duty = (ld + 1) * dwc->clk_ns;
given that todays compilers are clever enough to optimize that just fine
and this version is better readable for humans.
> +
> + period = ld2;
> + period += 1;
> + period *= dwc->clk_ns;
> + period += duty;
> + } else {
> + duty = (ld + 1) * dwc->clk_ns;
> + period = duty * 2;
> + }
>
> + state->period = period;
> + state->duty_cycle = duty;
> state->polarity = PWM_POLARITY_INVERSED;
>
> pm_runtime_put_sync(chip->dev);
> --
> 2.35.1
I'm marking all patches in this series as "changes requested" even
though not all patches were commented. I assume that you continue to
care for all of them for the next revision. Please make sure to pass -v4
to git format-patch (or git send-email) then.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
