[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <s5ieu7cciqgw3x5rnpcn43fm3nkinm4guz33nhwwi45apdz2ot@4zsbi7tckuwz>
Date: Mon, 15 Dec 2025 12:29:56 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: 20190311120055 created <payne.lin@...iatek.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com, sirius.wang@...iatek.com, vince-wl.liu@...iatek.com,
jh.hsu@...iatek.com
Subject: Re: [PATCH] pwm: mediatek: Add error handling for zero rate in PWM
state
Hello,
the From: line of your patch is strange.
On Mon, Dec 15, 2025 at 06:40:32PM +0800, 20190311120055 created wrote:
> From: Payne Lin <payne.lin@...iatek.com>
>
> Added a check to handle cases where the rate is zero in the
> mtk_disp_pwm_get_state function. This prevents division by zero errors
> when calculating the period.
> - Added error message for zero rate scenario
>
> Signed-off-by: Payne Lin <payne.lin@...iatek.com>
> ---
> drivers/pwm/pwm-mtk-disp.c | 31 +++++++++++++++++++------------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index bafd6b6195f6..dd0ae041af70 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -176,19 +176,18 @@ static int mtk_disp_pwm_get_state(struct pwm_chip *chip,
> struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> u64 rate, period, high_width;
> u32 clk_div, pwm_en, con0, con1;
> - int err;
> + int ret;
If you keep the variable name, the patch gets quite a bit more obivious.
> - err = clk_prepare_enable(mdp->clk_main);
> - if (err < 0) {
> - dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_main: %pe\n", ERR_PTR(err));
> - return err;
> + ret = clk_prepare_enable(mdp->clk_main);
> + if (ret < 0) {
> + dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_main: %pe\n", ERR_PTR(ret));
> + goto err_handle;
> }
Fixing error handling should go into a separate patch please.
> - err = clk_prepare_enable(mdp->clk_mm);
> - if (err < 0) {
> - dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_mm: %pe\n", ERR_PTR(err));
> - clk_disable_unprepare(mdp->clk_main);
> - return err;
> + ret = clk_prepare_enable(mdp->clk_mm);
> + if (ret < 0) {
> + dev_err(pwmchip_parent(chip), "Can't enable mdp->clk_mm: %pe\n", ERR_PTR(ret));
> + goto err_disable_clk_main;
> }
>
> /*
> @@ -212,15 +211,23 @@ static int mtk_disp_pwm_get_state(struct pwm_chip *chip,
> * period has 12 bits, clk_div 11 and NSEC_PER_SEC has 30,
> * so period * (clk_div + 1) * NSEC_PER_SEC doesn't overflow.
> */
That comment belongs to the assignment to state->period. Don't separate
them with your addition.
> + if (rate == 0) {
> + dev_err(pwmchip_parent(chip), "rate is zero, cannot calculate period\n");
All the other messages (at least those in the context above) start with
a capital letter.
> + ret = -EINVAL;
> + goto err_disable_clk_mm;
> + }
> state->period = DIV64_U64_ROUND_UP(period * (clk_div + 1) * NSEC_PER_SEC, rate);
> high_width = FIELD_GET(PWM_HIGH_WIDTH_MASK, con1);
> state->duty_cycle = DIV64_U64_ROUND_UP(high_width * (clk_div + 1) * NSEC_PER_SEC,
> rate);
> state->polarity = PWM_POLARITY_NORMAL;
> +
> +err_disable_clk_mm:
> clk_disable_unprepare(mdp->clk_mm);
> +err_disable_clk_main:
> clk_disable_unprepare(mdp->clk_main);
> -
> - return 0;
> +err_handle:
> + return ret;
> }
Best regards
Uwe
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists