Messages in this thread | | | Date | Tue, 26 Apr 2022 00:07:13 +0200 | Subject | Re: [PATCH] pinctrl: stm32: improve bank clocks management | From | Marek Vasut <> |
| |
On 4/25/22 11:27, Fabien DESSENNE wrote: > Hi Marek > > > On 22/04/2022 18:26, Marek Vasut wrote: >> On 4/22/22 16:36, Fabien Dessenne wrote: >>> Instead of enabling/disabling the clock at each IO configuration update, >>> just keep the clock enabled from the probe. >>> This makes things simpler and more efficient (e.g. the time required to >>> toggle an output IO is drastically decreased) without significantly >>> increasing the power consumption. >> >> [...] >> >>> static struct irq_domain *stm32_pctrl_get_irq_domain(struct >>> device_node *np) >>> @@ -1575,6 +1537,10 @@ int stm32_pctl_probe(struct platform_device >>> *pdev) >>> ret = stm32_gpiolib_register_bank(pctl, child); >>> if (ret) { >>> of_node_put(child); >>> + >>> + for (i = 0; i < pctl->nbanks; i++) >>> + clk_disable_unprepare(pctl->banks[i].clk); >>> + >> >> There are clk_bulk_*() functions, maybe you can use those to get rid >> of these loops ? > > This sounds goods, but checking more in details I see that moving to the > 'bulk' implementation would require to move the clk information from the > "struct stm32_gpio_bank *banks" member to its parent "struct > stm32_pinctrl". > > This would make the clk device information stored in a different > structure from the other device-related information (base address, reset > control, ...). It's better to keep all those information together in the > same struct. > > As another drawback we would loose access to 'clk' from any function > that have 'bank' (or 'struct gpio_chip *chip') as input parameter (e.g. > stm32_gpio_get() called from gpiolib). > > So I really prefer to keep the current implementation.
All right, I agree.
>> The rest looks good to me.
Reviewed-by: Marek Vasut <marex@denx.de>
| |