linux-kernel - Re: [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230715193328.xqvhjcwef3b3f5kj@pengutronix.de>
Date:   Sat, 15 Jul 2023 21:33:28 +0200
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Ben Dooks <ben.dooks@...ive.com>
Cc:     linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, ben.dooks@...ethink.co.uk,
        Thierry Reding <thierry.reding@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Greentime Hu <greentime.hu@...ive.com>,
        jarkko.nikula@...ux.intel.com,
        William Salmon <william.salmon@...ive.com>,
        Jude Onyenegecha <jude.onyenegecha@...ive.com>
Subject: Re: [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call

On Wed, Jun 14, 2023 at 06:14:55PM +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@...ive.com>
> ---
> v8:
>  - fixed rename issues
> v4:
>  - fixed review comment on mulit-line calculations
> ---
>  drivers/pwm/pwm-dwc-core.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 4b4b7b9e1d82..38cd2163fe01 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -122,24 +122,31 @@ static int 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

Multiline comments should have /* on a line for itself.

> +	 * based on the timer load-count only.
> +	 */
> +	if (ctrl & DWC_TIM_CTRL_PWM) {
> +		duty = (ld + 1) * dwc->clk_ns;
> +		period = (ld2 + 1)  * dwc->clk_ns;

s/  / /

> +		period += duty;
> +	} else {
> +		duty = (ld + 1) * dwc->clk_ns;
> +		period = duty * 2;
> +	}
>  
>  	state->polarity = PWM_POLARITY_INVERSED;
> +	state->period = period;
> +	state->duty_cycle = duty;
>  
>  	pm_runtime_put_sync(chip->dev);

Best regards
Uwe

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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)