linux-kernel - Re: [PATCH v2 4/5] firmware: exynos-acpm: register ACPM clocks dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e8346a38-fef7-482f-81ab-20621988b047@kernel.org>
Date: Sun, 31 Aug 2025 12:50:36 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Tudor Ambarus <tudor.ambarus@...aro.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Peter Griffin <peter.griffin@...aro.org>,
 André Draszik <andre.draszik@...aro.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>,
 Sylwester Nawrocki <s.nawrocki@...sung.com>,
 Chanwoo Choi <cw00.choi@...sung.com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-clk@...r.kernel.org, willmcvicker@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH v2 4/5] firmware: exynos-acpm: register ACPM clocks dev

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