Re: [PATCH v4 4/4] PCI: tegra: Drop unused remove callback

[Aaron Kling <webgeek1234@gmail.com>]

On Sat, May 10, 2025 at 1:24 AM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> 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@linaro.org> 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@linaro.org> wrote:
> > > > >
> > > > > On Mon, May 05, 2025 at 09:59:01AM -0500, Aaron Kling via B4 Relay wrote:
> > > > > > From: Aaron Kling <webgeek1234@gmail.com>
> > > > > >
> > > > > > 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@gmail.com