[PATCH] clk: x86: fch: validate registrations and manage their lifetime

[Slavin Liu]

Fixed-rate and mux registration can fail before clk_set_parent()
evaluates their clk members. Check each registration and the later
parent/clkdev operations. Use managed clock registration so every new
error exit unwinds only successfully registered clocks in reverse order.
Keep the hardware-clock array local and drop the manual remove callback
to avoid unregistering managed clocks twice.

Detected by static analysis and reviewed with AI-assisted source auditing.

Fixes: 421bf6a1f061 ("clk: x86: Add ST oscout platform clock")
Assisted-by: LLM
Signed-off-by: Slavin Liu <bolin.liu@xxxxxxxxxx>
---
 drivers/clk/x86/clk-fch.c | 109 ++++++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 47 deletions(-)

diff --git a/drivers/clk/x86/clk-fch.c b/drivers/clk/x86/clk-fch.c
index cf5cd3ad4647..b66ede14fc22 100644
--- a/drivers/clk/x86/clk-fch.c
+++ b/drivers/clk/x86/clk-fch.c
@@ -35,7 +35,6 @@
 #define AMD_CPU_ID_ST                  0x1576
 
 static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" };
-static struct clk_hw *hws[ST_MAX_CLKS];
 
 static const struct pci_device_id fch_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_ST) },
@@ -44,8 +43,10 @@ static const struct pci_device_id fch_pci_ids[] = {
 
 static int fch_clk_probe(struct platform_device *pdev)
 {
+	struct clk_hw *hws[ST_MAX_CLKS];
 	struct fch_clk_data *fch_data;
 	struct pci_dev *rdev;
+	int ret;
 
 	fch_data = dev_get_platdata(&pdev->dev);
 	if (!fch_data || !fch_data->base)
@@ -58,55 +59,70 @@ static int fch_clk_probe(struct platform_device *pdev)
 	}
 
 	if (pci_match_id(fch_pci_ids, rdev)) {
-		hws[ST_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz",
-			NULL, 0, 48000000);
-		hws[ST_CLK_25M] = clk_hw_register_fixed_rate(NULL, "clk25MHz",
-			NULL, 0, 25000000);
-
-		hws[ST_CLK_MUX] = clk_hw_register_mux(NULL, "oscout1_mux",
-			clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents),
-			0, fch_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0,
-			NULL);
-
-		clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk);
-
-		hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1",
-			"oscout1_mux", 0, fch_data->base + MISCCLKCNTL1,
-			OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL);
-
-		devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE],
-					    fch_data->name, NULL);
+		hws[ST_CLK_48M] = devm_clk_hw_register_fixed_rate(&pdev->dev, "clk48MHz",
+								  NULL, 0, 48000000);
+		if (IS_ERR(hws[ST_CLK_48M])) {
+			ret = PTR_ERR(hws[ST_CLK_48M]);
+			goto out_put;
+		}
+		hws[ST_CLK_25M] = devm_clk_hw_register_fixed_rate(&pdev->dev, "clk25MHz",
+								  NULL, 0, 25000000);
+		if (IS_ERR(hws[ST_CLK_25M])) {
+			ret = PTR_ERR(hws[ST_CLK_25M]);
+			goto out_put;
+		}
+
+		hws[ST_CLK_MUX] =
+			devm_clk_hw_register_mux(&pdev->dev, "oscout1_mux",
+						 clk_oscout1_parents,
+						 ARRAY_SIZE(clk_oscout1_parents),
+						 0, fch_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3,
+						 0,
+						 NULL);
+		if (IS_ERR(hws[ST_CLK_MUX])) {
+			ret = PTR_ERR(hws[ST_CLK_MUX]);
+			goto out_put;
+		}
+
+		ret = clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_48M]->clk);
+		if (ret)
+			goto out_put;
+
+		hws[ST_CLK_GATE] =
+			devm_clk_hw_register_gate(&pdev->dev, "oscout1",
+						  "oscout1_mux", 0, fch_data->base + MISCCLKCNTL1,
+						  OSCCLKENB, CLK_GATE_SET_TO_DISABLE, NULL);
+		if (IS_ERR(hws[ST_CLK_GATE])) {
+			ret = PTR_ERR(hws[ST_CLK_GATE]);
+			goto out_put;
+		}
+
+		ret = devm_clk_hw_register_clkdev(&pdev->dev, hws[ST_CLK_GATE],
+						  fch_data->name, NULL);
 	} else {
-		hws[CLK_48M_FIXED] = clk_hw_register_fixed_rate(NULL, "clk48MHz",
-			NULL, 0, 48000000);
-
-		hws[CLK_GATE_FIXED] = clk_hw_register_gate(NULL, "oscout1",
-			"clk48MHz", 0, fch_data->base + MISCCLKCNTL1,
-			OSCCLKENB, 0, NULL);
-
-		devm_clk_hw_register_clkdev(&pdev->dev, hws[CLK_GATE_FIXED],
-					    fch_data->name, NULL);
+		hws[CLK_48M_FIXED] = devm_clk_hw_register_fixed_rate(&pdev->dev, "clk48MHz",
+								     NULL, 0, 48000000);
+		if (IS_ERR(hws[CLK_48M_FIXED])) {
+			ret = PTR_ERR(hws[CLK_48M_FIXED]);
+			goto out_put;
+		}
+
+		hws[CLK_GATE_FIXED] =
+			devm_clk_hw_register_gate(&pdev->dev, "oscout1",
+						  "clk48MHz", 0, fch_data->base + MISCCLKCNTL1,
+						  OSCCLKENB, 0, NULL);
+		if (IS_ERR(hws[CLK_GATE_FIXED])) {
+			ret = PTR_ERR(hws[CLK_GATE_FIXED]);
+			goto out_put;
+		}
+
+		ret = devm_clk_hw_register_clkdev(&pdev->dev, hws[CLK_GATE_FIXED],
+						  fch_data->name, NULL);
 	}
 
+out_put:
 	pci_dev_put(rdev);
-	return 0;
-}
-
-static void fch_clk_remove(struct platform_device *pdev)
-{
-	int i, clks;
-	struct pci_dev *rdev;
-
-	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
-	if (!rdev)
-		return;
-
-	clks = pci_match_id(fch_pci_ids, rdev) ? CLK_MAX_FIXED : ST_MAX_CLKS;
-
-	for (i = 0; i < clks; i++)
-		clk_hw_unregister(hws[i]);
-
-	pci_dev_put(rdev);
+	return ret;
 }
 
 static struct platform_driver fch_clk_driver = {
@@ -115,6 +131,5 @@ static struct platform_driver fch_clk_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe = fch_clk_probe,
-	.remove = fch_clk_remove,
 };
 builtin_platform_driver(fch_clk_driver);