Re: [PATCH v15 09/11] clk: realtek: Add RTD1625-CRT clock controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: realtek: incorrect bitmask FREQ_MNO_MASK truncates O field for VODMA PLL
--

commit 0728493fca904449d29102f41b0c4a133dfc60a6
Author: Cheng-Yu Lee <cylee12@realtek.com>

clk: realtek: Add RTD1625-CRT clock controller driver

This commit introduces support for the Realtek RTD1625-CRT clock controller,
adding various clock sources, multiplexers, and gates. It also defines
frequency tables and masks for the PLLs used by the device.

> diff --git a/drivers/clk/realtek/clk-rtd1625-crt.c b/drivers/clk/realtek/clk-rtd1625-crt.c
> new file mode 100644
> index 0000000000000..b25717eb6301b
> --- /dev/null
> +++ b/drivers/clk/realtek/clk-rtd1625-crt.c
> @@ -0,0 +1,822 @@

[ ... ]

> +#define FREQ_NF_MASK     0x7ffff
> +#define FREQ_NF(_r, _nf) {.rate = _r, .val = (_nf),}
> +
> +/* PLL VODMA: M (11:4) / N (13:12) / O (19:17) */
> +#define FREQ_MNO_MASK            0x63ff0

[Severity: Medium]
Does this mask inadvertently exclude bit 19?

The comment documents the O field as covering bits 19:17, but the value
0x63ff0 (which covers bits 18:17, 13:12, and 11:4) leaves bit 19 unset.

If a bootloader configures the hardware with an O divider that requires
bit 19 to be set, would this cause the driver to incorrectly mask out that
bit, resulting in an incorrect frequency calculation during initialization?
It appears the mask should be 0xe3ff0 to fully cover the O field.

> +#define FREQ_MNO(_r, _m, _n, _o) { .rate = (_r), .val = ((_m) << 4) | \
> +				 ((_n) << 12) | ((_o) << 17), }
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002074331.3066919-1-eleanor.lin@realtek.com?part=9