Thread (16 messages) 16 messages, 4 authors, 2025-08-25

Re: [PATCH 3/3] clk: samsung: add Exynos ACPM clock driver

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-08-24 18:09:10
Also in: linux-clk, linux-devicetree, linux-samsung-soc, lkml

On 19/08/2025 13:45, Tudor Ambarus wrote:
+
+static int acpm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long parent_rate)
+{
+	struct acpm_clk *clk = to_acpm_clk(hw);
+
+	return clk->handle->ops.dvfs_ops.set_rate(clk->handle,
+					clk->acpm_chan_id, clk->id, rate);
+}
+
+static const struct clk_ops acpm_clk_ops = {
+	.recalc_rate = acpm_clk_recalc_rate,
+	.round_rate = acpm_clk_round_rate,
This should be determine_rate. Check recent patchset from Brian Masney.
I applied the samsung bits from it to samsung soc tree.

...
+
+static int __init acpm_clk_probe(struct platform_device *pdev)
module probe for sure should not be __init.
+{
+	const struct acpm_clk_match_data *match_data;
+	const struct acpm_handle *acpm_handle;
+	struct clk_hw_onecell_data *clk_data;
+	struct clk_hw **hws;
+	struct device *dev = &pdev->dev;
+	struct acpm_clk *aclks;
+	unsigned int acpm_chan_id;
+	int i, err, count;
+
+	acpm_handle = devm_acpm_get_by_node(dev, dev->parent->of_node);
+	if (IS_ERR(acpm_handle))
+		return dev_err_probe(dev, PTR_ERR(acpm_handle),
+				     "Failed to get acpm handle.\n");
+
+	match_data = of_device_get_match_data(dev);
+	if (!match_data)
+		return dev_err_probe(dev, -EINVAL,
+				     "Failed to get match data.\n");
+
+	count = match_data->nr_clks;
+	acpm_chan_id = match_data->acpm_chan_id;
+
+	clk_data = devm_kzalloc(dev, struct_size(clk_data, hws, count),
+				GFP_KERNEL);
+	if (!clk_data)
+		return -ENOMEM;
+
+	clk_data->num = count;
+	hws = clk_data->hws;
+
+	aclks = devm_kcalloc(dev, count, sizeof(*aclks), GFP_KERNEL);
+	if (!aclks)
+		return -ENOMEM;
+
+	for (i = 0; i < count; i++) {
+		const struct acpm_clk_variant *variant = &match_data->clks[i];
+		struct acpm_clk *aclk = &aclks[i];
+
+		hws[i] = &aclk->hw;
+
+		aclk->id = variant->id;
+		aclk->handle = acpm_handle;
+		aclk->acpm_chan_id = acpm_chan_id;
+
+		err = acpm_clk_ops_init(dev, aclk, variant->name);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "Failed to register clock.\n");
+	}
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					   clk_data);
+}
+
+#define ACPM_CLK(_id, cname)					\
+	{							\
+		.id		= _id,				\
+		.name		= cname,			\
+	}
+
+static const struct acpm_clk_variant gs101_acpm_clks[] __initconst = {
This goes to top of the file, after struct declarations.
+	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"),
+};
+
+static const struct acpm_clk_match_data acpm_clk_gs101 __initconst = {
Are you going to have more of such clk_match_data? More variants?
+	.clks = gs101_acpm_clks,
+	.nr_clks = ARRAY_SIZE(gs101_acpm_clks),
+	.acpm_chan_id = 0,
+};
+
+static const struct of_device_id acpm_clk_ids[] __initconst = {
+	{
+		.compatible = "google,gs101-acpm-dvfs-clocks",
+		.data =  &acpm_clk_gs101,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, acpm_clk_ids);
+
+static struct platform_driver acpm_clk_driver __refdata = {
__refdata feels wrong here. There is a long standing issue with Samsung
clock drivers - I sent a patchset for that but it did failed testing.

But your code is even simpler - no CLK_OFDECLARE - so why is this refdata?


Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help