[PATCH v2 4/4] dm: core: Rename dm_remove_devices_active()

[Simon Glass]

Hi Tom,

On Mon, 7 Apr 2025 at 07:30, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Apr 07, 2025 at 10:49:11PM +1200, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 7 Apr 2025 at 19:57, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 07.04.25 03:35, Simon Glass wrote:
> > > > This function doesn't have a great name, since we talk of devices being
> > > > active once they are probed.
> > > >
> > > > Rename it to dm_remove_dma_devices().
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > Suggested-by: Jonas Karlman <jonas at kwiboo.se>
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > >   arch/arm/lib/bootm.c          | 4 ++--
> > > >   arch/riscv/lib/bootm.c        | 2 +-
> > > >   arch/x86/lib/bootm.c          | 2 +-
> > > >   drivers/core/root.c           | 2 +-
> > > >   include/dm/root.h             | 6 +++---
> > > >   lib/efi_loader/efi_boottime.c | 2 +-
> > > >   test/dm/core.c                | 2 +-
> > > >   7 files changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > > index 688c2f3f29b..ef859a2cd4c 100644
> > > > --- a/arch/arm/lib/bootm.c
> > > > +++ b/arch/arm/lib/bootm.c
> > > > @@ -74,10 +74,10 @@ static void announce_and_cleanup(int fake)
> > > >        * Call remove function of all devices with a removal flag set.
> > > >        * This may be useful for last-stage operations, like cancelling
> > > >        * of DMA operation or releasing device internal buffers.
> > > > -      * dm_remove_devices_active() ensures that vital devices are removed in
> > > > +      * dm_remove_dma_devices() ensures that vital devices are removed in
> > > >        * a second round.
> > > >        */
> > > > -     dm_remove_devices_active();
> > > > +     dm_remove_dma_devices();
> > > >
> > > >       cleanup_before_linux();
> > > >   }
> > > > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > > > index 76c610bcee0..9aecaafef43 100644
> > > > --- a/arch/riscv/lib/bootm.c
> > > > +++ b/arch/riscv/lib/bootm.c
> > > > @@ -57,7 +57,7 @@ static void announce_and_cleanup(int fake)
> > > >        * This may be useful for last-stage operations, like cancelling
> > > >        * of DMA operation or releasing device internal buffers.
> > > >        */
> > > > -     dm_remove_devices_active();
> > > > +     dm_remove_dma_devices();
> > > >
> > > >       cleanup_before_linux();
> > > >   }
> > > > diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c
> > > > index 3c420b00936..91e8de4e1ad 100644
> > > > --- a/arch/x86/lib/bootm.c
> > > > +++ b/arch/x86/lib/bootm.c
> > > > @@ -50,7 +50,7 @@ void bootm_announce_and_cleanup(void)
> > > >        * This may be useful for last-stage operations, like cancelling
> > > >        * of DMA operation or releasing device internal buffers.
> > > >        */
> > > > -     dm_remove_devices_active();
> > > > +     dm_remove_dma_devices();
> > > >   }
> > > >
> > > >   #if defined(CONFIG_OF_LIBFDT) && !defined(CONFIG_OF_NO_KERNEL)
> > > > diff --git a/drivers/core/root.c b/drivers/core/root.c
> > > > index c7fb58285ca..101cc4d43fa 100644
> > > > --- a/drivers/core/root.c
> > > > +++ b/drivers/core/root.c
> > > > @@ -148,7 +148,7 @@ int dm_remove_devices_flags(uint flags)
> > > >       return 0;
> > > >   }
> > > >
> > > > -void dm_remove_devices_active(void)
> > > > +void dm_remove_dma_devices(void)
> > > >   {
> > > >       /* Remove non-vital devices first */
> > > >       device_remove(dm_root(), DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL);
> > > > diff --git a/include/dm/root.h b/include/dm/root.h
> > > > index 5651b868c8b..39a069cd465 100644
> > > > --- a/include/dm/root.h
> > > > +++ b/include/dm/root.h
> > > > @@ -169,16 +169,16 @@ int dm_uninit(void);
> > > >   int dm_remove_devices_flags(uint flags);
> > > >
> > > >   /**
> > > > - * dm_remove_devices_active - Call remove function of all active drivers heeding
> > > > + * dm_remove_dma_devices - Call remove function of all active drivers heeding
> > > >    *                            device dependencies as far as know, i.e. removing
> > > >    *                            devices marked with DM_FLAG_VITAL last.
> > > >    *
> > > >    * All active devices will be removed
> > > >    */
> > > > -void dm_remove_devices_active(void);
> > > > +void dm_remove_dma_devices(void);
> > >
> > > We have to remove all active devices. Not only DMA devices but also any
> > > device that creates interrupts.
> > >
> > > The renaming is misleading.
> >
> > Yes, but so is the existing name, as mentioned.
> >
> > How about dm_remove_for_boot() ?
>
> I think the name itself is fine and maybe just a bit more
> documentation? It's dealing with the DM_REMOVE_ACTIVE_ALL flag
> after all.

Yes,  it is. I would like to more clearly separate the naming, but I
haven't come up with a great option. So let's drop this patch.

Regards,
SImon