On 10/4/23 21:43, Paul Barker wrote:
[...]
+/*
- We need to ensure that the module clock is enabled and all resets are
- de-asserted before using either the gpio or pinctrl functionality. Error
- handling can be quite simple here as if the PFC cannot be enabled then we
- will not be able to progress with the boot anyway.
- */
+static int rzg2l_pfc_enable(struct udevice *dev) +{
- struct rzg2l_pfc_data *data =
(struct rzg2l_pfc_data *)dev_get_driver_data(dev);- struct reset_ctl_bulk rsts;
- struct clk clk;
- int ret;
- if (data->pfc_enabled)
When does this get triggered ?
This is initialised to false in rzg2l_pfc_bind(), then this function rzg2l_pfc_enable() sets it to true before a successful return. The effect is that the PFC is enabled just once, regardless of whether the pinctrl or gpio driver is probed first.
Why would be call to rzg2l_pfc_enable() a problem in the first place ? It just grabs and enables clock and ungates reset, the second time this is called the impact on harware should be no-op , right ?
The hw impact is a no-op, but it wastes time unnecessarily re-reading data from the fdt and repeating the setup, e.g. in rzg2l_cpg_clk_set() we have to search the array of clocks each time to find the requested entry.
Does getting clock and enabling them have noticable overhead on this platform ? Look at CONFIG_OF_LIVE, that should mitigate the DT access overhead at least.
I think it's worth keeping the conditional here but can drop it if you're really against it.
It feels like fixing a problem at the wrong place really.