Re: [PATCH v11 4/6] clk: Add KUnit tests for assigned-clock-sscs
| 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