Quoting Jeffrey Hugo (2019-02-11 10:57:47) > If a parent to a clock comes from outside that clock's provider, the parent > may not be present at the time the clock is registered (ie the parent comes > from another driver that has not yet probed). The clock can still be > registered, and a reference to it obtained, however that clock may not be > fully functional - ie get_rate might return an invalid value. > > This has been a problem that has resulted in the UART console breaking on > some Qualcomm SoCs, as the UART baud rate is based on a clock that is the > child of XO. Due to the large chain of dependencies, its possible that the > RPM has not provided XO by the time that the UART driver probes, gets the > baud rate clock, and calls get_rate - which returns 0 and results in a bad > configuration. > > An orphan clock is a clock that is missing a parent or some other ancestor. > Since the parent is defined, we can assume that it is expected to appear at > some point in a properly configured system (all bets are off if a required > driver is not compiled, etc), and it is unlikely that the clock can be > properly consumed during the time the clock is an orphan. Therefore, > return EPROBE_DEFER for orphan clocks so that consumers wait until the > parent chain is established, and proper clock operation can occur. > > Signed-off-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> > --- > > This is based upon the "Rewrite clk parent handling" series at [1], and assumes > that the suspected missing line commented on at [2] is added. > > The idea for this solution came from [3] and [4]. > > [1] https://lore.kernel.org/lkml/20190129061021.94775-1-sboyd@xxxxxxxxxx/T/#u > [2] https://lkml.org/lkml/2019/2/11/1634 > [3] https://lkml.org/lkml/2019/2/6/382 > [4] https://lkml.org/lkml/2015/12/27/209 There have been multiple attempts over the years to support probe defer for clks that don't have parents. If you search the kernel mailing list archives I'm sure you'll come across them (for example https://patchwork.kernel.org/patch/6313051/). That's why we have the first part of the code to indicate if a clk is an orphan or not, i.e. commit e6500344edbb ("clk: track the orphan status of clocks and their children"), but not this patch that you've sent. There are a couple requirements that we need to make sure we don't break first. 1. clk_get() should work for clks on the orphan list if that clk is parented to something that will never be registered with the framework 2. We need a way for drivers to express that the parent of a clk won't exist 3. Critical clks need to turn clks on even if they'll never get parents registered We've had problems in the past (http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/343007.html) where bootloaders configure clks in certain ways that the kernel doesn't care to even consider as possible. In these cases we either need to let clk_set_rate() reparent them when consumers are ready or we need to convert drivers that are forcing on clks early to use the CLK_IS_CRITICAL flag to turn on clks even if they're never going to find their parents. Last time we tried to do this in 2015 (wow so many years ago!) we were blocked on not having the critical clks infrastructure and the legacy sunxi clk driver needed to convert to DT and critical clks flag to keep working. I think we could have done it a year or two ago, because sunxi moved to a new design, but then we got more use-cases where clks may never get the parent they're currently configured for in the bootloader and then the kernel would never hand out the clk to consumers and the clk_set_rate() case would fail. To fix that last part, I'm proposing we introduce the .get_parent_hw() op and then rely on drivers to tell the framework that the parent is there either with a direct pointer reference or by knowing that the DT/firmware is telling us the parent is valid. If we just rely on string names and a u8 to indicate parents then we don't have enough information to figure out how the parent is provided and if it will ever appear at some point in the future. Once we have a way to describe this through DT/firmware then we're able to indicate the clk is an orphan when that's actually the case vs. when the clk is configured in hardware for something that we won't know about. You can see this work in the clk-parent-rewrite series in clk.git. There's also one more problem, which is what we do with clks that we've handed out to consumers and then the driver for the parent of that clk is removed and the parent is unregistered. Right now, we move these clks to the orphan list and set the clk_nodrv_ops on the parent that's unregistered. We probably need to set clk_nodrv_ops on all the children that get orphaned, and remove the cached clk_core pointer in all the clk_core::parents members (even ones that aren't currently using it!), and stash away the original clk_ops so we can restore them later when the clk is properly reparented if the parent comes back. It's a lot of book keeping to remove the dangling references and let it come back later. I haven't started on this part, but it's on my radar.
