On Thu Dec 12, 2024 at 1:18 PM CET, Roger Quadros wrote: > On 10/12/2024 19:13, Théo Lebrun wrote: > > At runtime_resume(), read the W1 (Wrapper Register 1) register to detect > > if an hardware reset occurred. If it did, run the hardware init sequence. > > > > This callback will be called at system-wide resume. Previously, if a > > reset occurred during suspend, we would crash. The wrapper config had > > not been written, leading to invalid register accesses inside cdns3. > > > > Did I understand right that the Controller reset can happen only at > system suspend and never at runtime suspend? J7200 + upstream kernel => if the power domain is cut off (it is implicitly at runtime PM) then it resets. This is 100% board dependent. We just never let it go into runtime suspend, for the moment. > If so do you really need the runtime suspend/resume hooks? > you should have different system suspend/resume hooks than runtime suspend/resume > hooks and deal with the re-initialization in system resume hook. The patch series works in the current setup with the wrapper that is never shut off. But it would also work if someone decides to use RPM on the wrapper. Overall, the current kernel-wide strategy is to minimise suspend/resume-specific code. Having only the concept of "runtime PM" and triggering that at system-wide suspend/resume is easier to reason about. It unifies concepts and reduces the states a device can be in. We could even imagine a future where ->suspend|resume() callbacks are pm_runtime_force_suspend|resume() by default. That'd be the dream, at least. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
