On 12/24/24 9:06 PM, Mingwei Zheng wrote:
[...]
@@ -1390,15 +1379,11 @@ static int stm32_gpiolib_register_bank(struct stm32_pinctrl *pctl, struct fwnode
err = gpiochip_add_data(&bank->gpio_chip, bank);
There is one other nasty problem here -- this gpiochip_add_data() needs
to be undone (*) below, otherwise ...
...
+ ret = clk_bulk_prepare_enable(banks, pctl->clks);
+ if (ret) {
+ dev_err(dev, "failed to prepare_enable clk (%d)\n", ret);
+ return ret;
+ }
+
for_each_gpiochip_node(dev, child) {
ret = stm32_gpiolib_register_bank(pctl, child);
... if this stm32_gpiolib_register_bank() fails for second or later bank
, then ...
if (ret) {
fwnode_handle_put(child);
-
- for (i = 0; i < pctl->nbanks; i++)
- clk_disable_unprepare(pctl->banks[i].clk);
-
- return ret;
+ goto err_clk;
}
pctl->nbanks++;
@@ -1658,6 +1642,10 @@ int stm32_pctl_probe(struct platform_device *pdev)
dev_info(dev, "Pinctrl STM32 initialized\n");
return 0;
+
+err_clk:
+ clk_bulk_disable_unprepare(banks, pctl->clks);
... this clk_bulk_disable_unprepare() will disable all bank clocks,
including the clocks for the banks which got successfully registered.
Before calling clk_bulk_disable_unprepare(), it is necessary to
unregister the GPIO chips again, i.e.:
i = 0;
for_each_gpiochip_node(dev, child) {
struct stm32_gpio_bank *bank = &pctl->banks[i];
gpiochip_remove(*bank->gpio_chip);
}
clk_bulk_disable_unprepare(banks, pctl->clks);
Otherwise I think the patch is pretty good, thank you !
