On Mon, May 05, 2025 at 11:59:27AM -0500, Aaron Kling wrote: > On Mon, May 5, 2025 at 11:48 AM Manivannan Sadhasivam > <manivannan.sadhasivam@xxxxxxxxxx> wrote: > > > > On Mon, May 05, 2025 at 11:43:26AM -0500, Aaron Kling wrote: > > > On Mon, May 5, 2025 at 11:32 AM Manivannan Sadhasivam > > > <manivannan.sadhasivam@xxxxxxxxxx> wrote: > > > > > > > > On Mon, May 05, 2025 at 09:59:01AM -0500, Aaron Kling via B4 Relay wrote: > > > > > From: Aaron Kling <webgeek1234@xxxxxxxxx> > > > > > > > > > > Debugfs cleanup is moved to a new shutdown callback to ensure the > > > > > debugfs nodes are properly cleaned up on shutdown and reboot. > > > > > > > > > > > > > Both are separate changes. You should remove the .remove() callback in the > > > > previous patch itself and add .shutdown() callback in this patch. > > > > > > > > And the shutdown callback should quiesce the device by putting it in L2/L3 state > > > > and turn off the supplies. It is not intended to perform resource cleanup. > > > > > > Then where would resource cleanup go? > > > > > > > .remove(), but it is not useful here since the driver is not getting removed. > > I may be misunderstanding how stuff works, but don't those resources > still need cleaned up on shutdown/reboot even if the driver can't be > unloaded? I recall seeing shutdown errors in the past when higher > level debugfs nodes tried to clean themselves up, but bad drivers had > left their nodes behind. > Each callback has its own purpose. The cleanup is only performed by the .remove() callback, but it will only get called when the driver gets removed. > In any case, do you want me to just not add .shutdown() at all, or try > to set the proper power state? Prior to the half-baked attempt to make > this driver a loadable module several years ago, there was no such > cleanup. > As a first step, you can ignore .shutdown() callback and just remove the .remove(). Shutdown callback implementation should follow the steps I mentioned above, so it can be done later. - Mani -- மணிவண்ணன் சதாசிவம்
