Re: [PATCH v9 05/12] clk: zte: Add Clock registration infrastructure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete refactoring of clock exports causing missing const, dead code, and unsafe aliasing for sparse DT IDs
--

--- Patch [5]: [PATCH v9 05/12] clk: zte: Add Clock registration infrastructure ---
commit c095bc91f84da44c659bbf8482a8f9e0d8e6f977
Author: Stefan Dösinger <stefandoesinger@gmail.com>

clk: zte: Add Clock registration infrastructure

This commit introduces the common clock registration infrastructure for ZTE
SoCs. It implements clock validation, initialization, and MFD child
integration.

> diff --git a/drivers/clk/zte/clk-zx.c b/drivers/clk/zte/clk-zx.c
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx.c
[ ... ]
> +	exports = devm_kzalloc(dev, struct_size(exports, hws, data->num_exports), GFP_KERNEL);
> +	if (!exports)
> +		return -ENOMEM;
> +	exports->num = data->num_exports;
> +
> +	for (i = 0; i < data->num_exports; ++i) {
> +		if (data->exports[i] >= data->num_clocks)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Export %u points to out of range clock %u\n",
> +					     i, data->exports[i]);
> +
> +		exports->hws[i] = clocks[data->exports[i]];

[Severity: Medium]
If the DT binding IDs are sparse, won't the gaps in the static exports array
default to 0? This would cause the uninitialized gaps to improperly alias to
the first internal clock (clocks[0]) instead of returning an error when
consumer drivers request an unused clock ID.

> +	}
> +
> +	devm_kfree(dev, clocks);
> +
> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, exports);
> +}
[ ... ]
> diff --git a/drivers/clk/zte/clk-zx.h b/drivers/clk/zte/clk-zx.h
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx.h
[ ... ]
> +struct clk_hw *zx_clk_register_gate(struct device *dev, struct regmap *regmap,
> +				    const struct zx_gate_desc *desc,
> +				    struct clk_hw * const *clocks);
> +
> +struct zx_clk_export {
> +	unsigned int priv;
> +	unsigned int binding;
> +};

[Severity: Medium]
Is struct zx_clk_export still needed? It appears to be unused dead code,
perhaps left over from an earlier iteration of the clock exports refactoring.

> +
> +struct zx_clk_data {
> +	int (*init)(struct regmap *map);
> +	const struct zx_clock *clocks;
> +	unsigned int num_clocks;
> +	unsigned int *exports;

[Severity: Medium]
Could the exports array pointer be marked as const? Without the const
qualifier, it prevents the static clock data from being placed in .rodata.

> +	unsigned int num_exports;
> +};
> +
> +int zx_clk_common_probe(struct device *dev, struct device_node *of_node,
> +			const struct zx_clk_data *data);
> +
> +#endif /* __DRV_CLK_ZX_H */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-zx29clk-v9-0-d05530d85d28@gmail.com?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy