Hi Brian, Thanks again for the review! Am Montag, 3. August 2026, 19:03:36 Ostafrikanische Zeit schrieb Brian Masney: > > + /* > > + * Sanity check: Make sure all parents are there and write a clear > > message rather than + * leave potential orphans. > > + */ > > It's not common for clk drivers to have a validate function like this. > That said, I know you are reverse engineering what's in the downstream > BSP without documentation, and there is value in having this. I think > it'd be useful to clarify that in the comments because code usually gets > copied and pasted between drivers over time. > > That said, once it's verified that the clock tree is correct, can these > checks be eventually dropped so that this validation logic isn't ran on > every boot? Yes, I plan to remove it eventually when support for the SoC is more mature. It has saved me some head-scratching as I have been working and re-working the clock tree. I'll clarify its driver specific role in the next submission. Another option I contemplated is wrapping it in #ifdef DEBUG. What do you think about that? > > +struct zx_clk_export { > > + unsigned int priv; > > + unsigned int binding; > > +}; > > Sashiko flagged this as unused. Indeed, that is a leftover from earlier - before I realized a simple unsigned int[] does the job fine. --- I have a long-standing question about tristate/module support for drivers like this: I don't think the driver can realistically be unloaded. I have been testing driver unloading by removing the UART clocks from the DT (otherwise the clock driver is busy) and marking all clocks critical (otherwise unloading/unbinding will shut down the UART (and more) and lock me out of the system). I have made it tristate because from early research into clock driver state of the art I gathered it was desired, even for drivers necessary for fundamental operation [0]. Did I understand this correctly? It also uncovered some linking errors that weren't obvious when compiling the driver into the kernel. That said, I'd prefer to mark my init tables discardable with __initdata and think the module load support is going to go untested and bitrot over time. Cheers, Stefan 0: Chen-Yu Tsai's tak here mostly: https://www.youtube.com/watch?v=d1VIAnVb3hI