Hi Jonas,
On 2/21/24 20:05, Jonas Karlman wrote:
Hi Quentin,
On 2024-02-21 19:18, Quentin Schulz wrote:
Hi Jonas,
On 2/17/24 19:35, Jonas Karlman wrote:
Port the RK3399 part of the Rockchip IO-domain driver from linux.
This differs from linux version in that pmu io iodomain bit is enabled
in the write ops instead of in an init ops as in linux, this way we can
avoid keeping a full state of all supply that have been configured.
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
Suggestions below, otherwise I only have one concern:
This actually postpones the moment the IO domain is properly set.
Before, we knew almost for sure when this was set, as it was pretty
early in the boot process (misc_init_r), I hope before any device was
actually probed. Now, because we're using the Device Model for that, we
may probe this IO domain device after another Device requires it? e.g.
for Puma, we need the IO domains set for PCIe to work. I assume if we
were to use PCIe in U-Boot on Puma, we may have this issue (we don't
right now use PCIe in U-Boot and I don't think we have any plan of doing
so).
Because this driver makes the following call in its bind ops, the
opposite is actually true and the IO domain is configured earlier than
it was previously done with misc_init_r.
With this the IO domain driver is probed directly after bind and
all regulators that is referenced by io-domain supply nodes will be
regulator_autoset() before the uV value is evaluated.
In rockchip_iodomain_bind():
dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
On the boards I can test on the GRF got configured exactly as before,
just a little bit earlier in the boot process.
Prior to the "rockchip: Port IO-domain driver for RK3568 from linux" [1]
series the rk8xx driver had some issues and incorrect voltage could be
read for some regulators.
[1] https://patchwork.ozlabs.org/cover/1823769/
Well, maybe I should read the source code before raising a potential
issue :) Thanks for the correction.
So in short:
common/board_r.c:init_sequence_r calls initr_dm before misc_init_r.
initr_dm -> dm_init_and_scan -> dm_scan -> dm_extended_scan which binds
the device and because the IO domain driver sets
DM_FLAG_PROBE_AFTER_BIND in its bind, dm_probe_devices at the end of
dm_scan will probe the device all that before misc_init_r is called.
This all reminds me of the attempts we've made in the Linux kernel to
make sure the IO domain devices are probed before the devices that
require them to be set up. Maybe we'll need something similar in U-Boot
as well.
The main thing missing compared to the linux counterpart is that changes
to regulator voltage is not tracked, so there will not be any changes
made the initial IO domain voltage configuration made by this driver.
There's a common issue for Linux and U-boot parts is that devices
dependent on the IO domain being properly configured is not properly
handled. Here in U-Boot, if there are devices which also have
DM_FLAG_PROBE_AFTER_BIND set, then I think this is dependent on the
order in udevice->sibling_node/child_head.
In any case, nothing that is specific to this patch series because it
was already the case before.
Therefore,
Reviewed-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>
Thanks,
Quentin
