Re: [PATCH v11 4/6] clk: Add KUnit tests for assigned-clock-sscs

[email protected]
Newsgroups org.kernel.vger.linux-clk,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Commit message and code mismatch: .set_spread_spectrum is omitted from clk_dummy_single_parent_ops
--

--- Patch [4]: [PATCH v11 4/6] clk: Add KUnit tests for assigned-clock-sscs ---
commit 5c23da1873b6e03ee6da2afc142b2da0f14531f3
Author: Peng Fan <[email protected]>

clk: Add KUnit tests for assigned-clock-sscs

This patch adds KUnit test coverage for the assigned-clock-sscs DT
property. It extends the dummy clock context and ops to support spread
spectrum settings, and adds a new test suite for various scenarios.

>     Extend the existing test infrastructure to support spread spectrum:
>     - Add struct clk_spread_spectrum field to clk_dummy_context and a
>       clk_dummy_set_spread_spectrum callback
>     - Wire set_spread_spectrum into all dummy clock ops

[Severity: Medium]
The commit message mentions wiring set_spread_spectrum into all dummy clock
ops, but it appears to be omitted from clk_dummy_single_parent_ops in
clk_test.c. Is this omission intentional?

It looks like clk_dummy_set_spread_spectrum assumes the clk_hw is embedded
in a struct clk_dummy_context:

static int clk_dummy_set_spread_spectrum(...)
{
    struct clk_dummy_context *ctx =
        container_of(hw, struct clk_dummy_context, hw);
    
    ctx->sscs = *ss_conf;
    ...
}

Clocks using clk_dummy_single_parent_ops often allocate a bare struct
clk_hw or use struct clk_single_parent_ctx, which lacks the sscs field.
If the callback had been naively wired as the commit message implies,
configuring spread spectrum on these clocks might cause memory corruption
via an out-of-bounds write.

Could the commit message be updated to clarify that
clk_dummy_single_parent_ops was intentionally skipped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.