On 10/5/23 11:39, Paul Barker wrote:
On 05/10/2023 03:24, Marek Vasut wrote:
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've not measured this. I was just assuming that it is sensible to only do the
setup once.
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.
I'll drop the pfc_enabled flag and re-test.
You can stick get_timer() before and after the code to get a rough idea
of how long it took, likely it will be 0 .
Thanks
