Re: [PATCH v11 3/6] clk: conf: Support assigned-clock-sscs

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Directly casting an array of `struct clk_spread_spectrum` to `(u32 *)` to read device tree properties tightly couples the compiler's memory layout to the DT array format, risking silent failures or memory corruption if the struct changes.
--

--- Patch [3]: [PATCH v11 3/6] clk: conf: Support assigned-clock-sscs ---
commit 9d153d59c6542e9110cebb9af6e487275c2559ca
Author: Peng Fan <[email protected]>

clk: conf: Support assigned-clock-sscs

Parse the Spread Spectrum Configuration (SSC) from the device tree and
configure them before using the clock. Each SSC configuration consists
of three u32 elements.

> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 303a0bb26e54a..550b8ae375a2c 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -155,6 +155,78 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
>  	return 0;
>  }
>  
> +static int __set_clk_spread_spectrum(struct device_node *node, bool clk_supplier)
> +{
> +	u32 elem_size = sizeof(struct clk_spread_spectrum);
> +	struct clk_spread_spectrum *sscs;
> +	struct of_phandle_args clkspec;
> +	int rc, count, index;
> +	struct clk *clk;
> +
> +	/* modfreq, spreadPercent, modmethod */
> +	count = of_property_count_elems_of_size(node, "assigned-clock-sscs", elem_size);
> +	if (count <= 0)
> +		return 0;
> +
> +	sscs = kcalloc(count, elem_size, GFP_KERNEL);
> +	if (!sscs)
> +		return -ENOMEM;
> +
> +	rc = of_property_read_u32_array(node, "assigned-clock-sscs", (u32 *)sscs,
> +					count * 3);

[Severity: Medium]
Could this direct cast to (u32 *) be problematic if the memory layout of
struct clk_spread_spectrum changes in the future?

Directly casting the array of structs to a primitive type array tightly
couples the device tree reading logic to the compiler's memory layout.

If a new field is added or padding is introduced to the structure,
of_property_count_elems_of_size() would miscalculate the count, and the
sequential read would cause misaligned struct access across the entire array.

> +	if (rc)
> +		goto free_sscs;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3