On 27/08/2025 14:42, Tudor Ambarus wrote:
> +
> +static const struct acpm_clk_variant gs101_acpm_clks[] = {
> + ACPM_CLK(CLK_ACPM_DVFS_MIF, "mif"),
> + ACPM_CLK(CLK_ACPM_DVFS_INT, "int"),
> + ACPM_CLK(CLK_ACPM_DVFS_CPUCL0, "cpucl0"),
> + ACPM_CLK(CLK_ACPM_DVFS_CPUCL1, "cpucl1"),
> + ACPM_CLK(CLK_ACPM_DVFS_CPUCL2, "cpucl2"),
> + ACPM_CLK(CLK_ACPM_DVFS_G3D, "g3d"),
> + ACPM_CLK(CLK_ACPM_DVFS_G3DL2, "g3dl2"),
> + ACPM_CLK(CLK_ACPM_DVFS_TPU, "tpu"),
> + ACPM_CLK(CLK_ACPM_DVFS_INTCAM, "intcam"),
> + ACPM_CLK(CLK_ACPM_DVFS_TNR, "tnr"),
> + ACPM_CLK(CLK_ACPM_DVFS_CAM, "cam"),
> + ACPM_CLK(CLK_ACPM_DVFS_MFC, "mfc"),
> + ACPM_CLK(CLK_ACPM_DVFS_DISP, "disp"),
> + ACPM_CLK(CLK_ACPM_DVFS_BO, "b0"),
> +};
I don't understand why clocks are defined in the firmware driver, not in
the clock driver.
This creates dependency of this patch on the clock patch, so basically
there is no way I will take it in one cycle.
> +
> /**
> * acpm_get_saved_rx() - get the response if it was already saved.
> * @achan: ACPM channel info.
> @@ -606,6 +636,7 @@ static void acpm_setup_ops(struct acpm_info *acpm)
>
> static int acpm_probe(struct platform_device *pdev)
> {
> + const struct acpm_clk_platform_data *acpm_clk_pdata;
> const struct acpm_match_data *match_data;
> struct device *dev = &pdev->dev;
> struct device_node *shmem;
> @@ -647,7 +678,30 @@ static int acpm_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, acpm);
>
> - return devm_of_platform_populate(dev);
> + acpm_clk_pdata = match_data->acpm_clk_pdata;
> + acpm->clk_pdev = platform_device_register_data(dev, "acpm-clocks",
> + PLATFORM_DEVID_NONE,
> + acpm_clk_pdata,
> + sizeof(*acpm_clk_pdata));
> + if (IS_ERR(acpm->clk_pdev))
> + return dev_err_probe(dev, PTR_ERR(acpm->clk_pdev),
> + "Failed to register ACPM clocks device.\n");
> +
> + ret = devm_of_platform_populate(dev);
> + if (ret) {
> + platform_device_unregister(acpm->clk_pdev);
I think this should stick to devm-interfaces everywhere, not mix them,
to have exactly expected cleanup sequence. Now your remove() first
unregisters and then de-populates, which is different order than it was
done in probe(). Use devm-action handler for device unregistering.
Best regards,
Krzysztof
