Re: [PATCH 2/3] clk: nuvoton: ma35d1: use the generic divider for the ADC clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: core: underflow and incorrect rounding for CLK_DIVIDER_EVEN_INTEGERS without a table
--

commit c5f5b7cbd52c161c065e9dedca522e878812d14a
Author: Joey Lu <a0987203069@gmail.com>

clk: nuvoton: ma35d1: use the generic divider for the ADC clock

This patch migrates the ADC clock divider in the Nuvoton MA35D1 driver to use
the generic divider with CLK_DIVIDER_EVEN_INTEGERS and
CLK_DIVIDER_ROUND_CLOSEST. It replaces a custom implementation and drops an
out-of-range bitmask that was previously OR'd into the register.

> diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c
> index 3102c65bfe9a0..94a3e904202b2 100644
> --- a/drivers/clk/nuvoton/clk-ma35d1.c
> +++ b/drivers/clk/nuvoton/clk-ma35d1.c
> @@ -1017,10 +1017,11 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>  	hws[QEI2_GATE] = ma35d1_clk_gate(dev, "qei2_gate", "pclk1",
>  					 clk_base + REG_CLK_APBCLK2, 14);
>  
> -	hws[ADC_DIV] = ma35d1_reg_adc_clkdiv(dev, "adc_div", hws[PCLK0],
> -					     &ma35d1_lock, 0,
> -					     clk_base + REG_CLK_CLKDIV4,
> -					     4, 17, 0x1ffff);
> +	hws[ADC_DIV] = devm_clk_hw_register_divider_parent_hw(dev, "adc_div", hws[PCLK0], 0,
> +							      clk_base + REG_CLK_CLKDIV4, 4, 17,
> +							      CLK_DIVIDER_EVEN_INTEGERS |
> +							      CLK_DIVIDER_ROUND_CLOSEST,
> +							      &ma35d1_lock);

[Severity: High]
Does migrating to the table-less generic CLK_DIVIDER_EVEN_INTEGERS expose this
clock to an underflow bug?

If the clock framework computes an odd divisor (for example, if a requested
rate yields div=1 or div=3), the generic _get_val function in
drivers/clk/clk-divider.c evaluates this as:

    if (flags & CLK_DIVIDER_EVEN_INTEGERS)
        return (div >> 1) - 1;

If div is 1, this results in (1 >> 1) - 1 = -1. Because this is evaluated as an
unsigned integer, it underflows to the maximum possible value, programming the
hardware to its maximum division instead of the desired rate. If div is 3, it
computes 0, configuring the hardware for /2 instead of /4.

Could this lead to a misconfiguration of the ADC clock? Would it be safer to
explicitly provide a clk_div_table to restrict the valid even divisors and
prevent this framework math issue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012063020.3331443-1-a0987203069@gmail.com?part=2