Re: [PATCH 2/5] clk: qcom: Add NSS clock controller driver for IPQ5210 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] clk: qcom: nsscc-ipq5210: wrap .driver.pm with pm_ptr()
--

commit 32b660ef44d1803c715de35bfad45d1be9b88559
Author: Luo Jie <jie.luo@oss.qualcomm.com>

clk: qcom: Add NSS clock controller driver for IPQ5210 SoC

This commit adds the NSS clock controller driver for the IPQ5210 SoC to
provide clocks, resets, and interconnect clock paths for networking blocks
such as PPE, UNIPHY, and the integrated Ethernet PHY.

> diff --git a/drivers/clk/qcom/nsscc-ipq5210.c b/drivers/clk/qcom/nsscc-ipq5210.c
> new file mode 100644
> index 0000000000000..ca749e0af3231
> --- /dev/null
> +++ b/drivers/clk/qcom/nsscc-ipq5210.c
[ ... ]
> +static const struct dev_pm_ops nss_cc_ipq5210_pm_ops = {
> +	SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
> +};
> +
> +static const struct of_device_id nss_cc_ipq5210_match_table[] = {
> +	{ .compatible = "qcom,ipq5210-nsscc" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, nss_cc_ipq5210_match_table);
> +
> +static int nss_cc_ipq5210_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Fail to enable runtime PM\n");
> +
> +	ret = devm_pm_clk_create(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Fail to create PM clock\n");
> +
> +	ret = pm_clk_add(&pdev->dev, "bus");
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Fail to add bus clock\n");
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Fail to resume\n");
> +
> +	ret = qcom_cc_probe(pdev, &nss_cc_ipq5210_desc);
> +	pm_runtime_put(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static struct platform_driver nss_cc_ipq5210_driver = {
> +	.probe = nss_cc_ipq5210_probe,
> +	.driver = {
> +		.name = "qcom,ipq5210-nsscc",
> +		.of_match_table = nss_cc_ipq5210_match_table,
> +		.pm = &nss_cc_ipq5210_pm_ops,

[Severity: Low]
This isn't a bug, but in nss_cc_ipq5210_driver, .driver.pm is assigned
&nss_cc_ipq5210_pm_ops unconditionally while nss_cc_ipq5210_pm_ops uses
SET_RUNTIME_PM_OPS().

Should nss_cc_ipq5210_pm_ops use RUNTIME_PM_OPS() and .driver.pm be wrapped
with pm_ptr(&nss_cc_ipq5210_pm_ops) so the structure and callbacks can be
dropped when CONFIG_PM is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-qcom_ipq5210_nsscc-v1-0-d4fcb5c32b51@oss.qualcomm.com?part=2