On Sat, May 10, 2025 at 1:24 AM Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> wrote: > > 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. This has already been done in v6 [0]. Sincerely, Aaron [0] https://lore.kernel.org/r/20250507-pci-tegra-module-v6-0-5fe363eaa302@xxxxxxxxx
