This series refactors the sparx5 init and deinit code out of
sparx5_start() and into probe(), adding proper per-subsystem cleanup
labels and deinit functions.

Currently, the sparx5 driver initializes most subsystems inside
sparx5_start(), which is called from probe(). This includes registering
netdevs, starting worker threads for stats and MAC table polling,
requesting PTP IRQs, and initializing VCAP. The function has grown to
handle many unrelated subsystems, and has no granular error handling —
it either succeeds entirely or returns an error, leaving cleanup to a
single catch-all label in probe().

The remove() path has a similar problem: teardown is not structured as
the reverse of initialization, and several subsystems lack proper deinit
functions. For example, the stats workqueue has no corresponding
cleanup, and the mact workqueue is destroyed without first cancelling
its delayed work.

Refactor this by moving each init function out of sparx5_start() and
into probe(), with a corresponding goto-based cleanup label. Add deinit
functions for subsystems that allocate resources, to properly cancel
work and destroy workqueues. Ensure that cleanup order in both error
paths and remove() follows the reverse of initialization order. What
remains in sparx5_start() is only hardware register setup and FDMA/XTR
initialization that does not require cleanup.

Before this series, most init functions live inside sparx5_start() with
no individual cleanup:

  probe():
    sparx5_start():              <- no granular error handling
      sparx5_mact_init()
      sparx_stats_init()           <- starts worker, no cleanup
      mact_queue setup             <- no cancel on teardown
      sparx5_register_netdevs()
      sparx5_register_notifier_blocks()
      sparx5_vcap_init()
    sparx5_ptp_init()

    probe() error path:
      cleanup_ports:
        sparx5_cleanup_ports()
        destroy_workqueue(mact_queue)

After this series, probe() initializes subsystems in order with
matching cleanup labels, and remove() tears down in reverse:

probe():
    sparx5_ptp_init()
    sparx5_vcap_init()
    sparx5_mact_init()
    sparx5_stats_init()
    sparx5_register_netdevs()
    sparx5_register_notifier_blocks()
    sparx5_start()

  remove():
    sparx5_unregister_notifier_blocks()
    sparx5_unregister_netdevs()
    sparx5_stats_deinit()
    sparx5_mact_deinit()
    sparx5_vcap_deinit()
    sparx5_ptp_deinit()
    sparx5_destroy_netdevs()

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Daniel Machon (7):
      net: sparx5: call sparx5_start() last in probe()
      net: sparx5: move netdev and notifier block registration to probe
      net: sparx5: move VCAP initialization to probe
      net: sparx5: move MAC table initialization and add deinit function
      net: sparx5: move stats initialization and add deinit function
      net: sparx5: move calendar initialization to probe
      net: sparx5: move remaining init functions from start() to probe()

 .../ethernet/microchip/sparx5/sparx5_calendar.c    |  15 ++-
 .../net/ethernet/microchip/sparx5/sparx5_ethtool.c |   9 +-
 .../ethernet/microchip/sparx5/sparx5_mactable.c    |  27 +++-
 .../net/ethernet/microchip/sparx5/sparx5_main.c    | 149 ++++++++++-----------
 .../net/ethernet/microchip/sparx5/sparx5_main.h    |  12 +-
 drivers/net/ethernet/microchip/sparx5/sparx5_ptp.c |  13 ++
 .../ethernet/microchip/sparx5/sparx5_vcap_impl.c   |   2 +-
 7 files changed, 135 insertions(+), 92 deletions(-)
---
base-commit: 17d0056f71b13050317a662a505b1a36fb7009e5
change-id: 20260216-sparx5-init-deinit-037cf165c62e

Best regards,
-- 
Daniel Machon <daniel.machon@microchip.com>

On Wed, Feb 25, 2026 at 10:05:23AM +0100, Daniel Machon wrote:
> This series refactors the sparx5 init and deinit code out of
> sparx5_start() and into probe(), adding proper per-subsystem cleanup
> labels and deinit functions.
> 
> Currently, the sparx5 driver initializes most subsystems inside
> sparx5_start(), which is called from probe(). This includes registering
> netdevs, starting worker threads for stats and MAC table polling,
> requesting PTP IRQs, and initializing VCAP. The function has grown to
> handle many unrelated subsystems, and has no granular error handling —
> it either succeeds entirely or returns an error, leaving cleanup to a
> single catch-all label in probe().
> 
> The remove() path has a similar problem: teardown is not structured as
> the reverse of initialization, and several subsystems lack proper deinit
> functions. For example, the stats workqueue has no corresponding
> cleanup, and the mact workqueue is destroyed without first cancelling
> its delayed work.
> 
> Refactor this by moving each init function out of sparx5_start() and
> into probe(), with a corresponding goto-based cleanup label. Add deinit
> functions for subsystems that allocate resources, to properly cancel
> work and destroy workqueues. Ensure that cleanup order in both error
> paths and remove() follows the reverse of initialization order. What
> remains in sparx5_start() is only hardware register setup and FDMA/XTR
> initialization that does not require cleanup.
> 
> Before this series, most init functions live inside sparx5_start() with
> no individual cleanup:
> 
>   probe():
>     sparx5_start():              <- no granular error handling
>       sparx5_mact_init()
>       sparx_stats_init()           <- starts worker, no cleanup
>       mact_queue setup             <- no cancel on teardown
>       sparx5_register_netdevs()
>       sparx5_register_notifier_blocks()
>       sparx5_vcap_init()
>     sparx5_ptp_init()
> 
>     probe() error path:
>       cleanup_ports:
>         sparx5_cleanup_ports()
>         destroy_workqueue(mact_queue)
> 
> After this series, probe() initializes subsystems in order with
> matching cleanup labels, and remove() tears down in reverse:
> 
> probe():
>     sparx5_ptp_init()
>     sparx5_vcap_init()
>     sparx5_mact_init()
>     sparx5_stats_init()
>     sparx5_register_netdevs()
>     sparx5_register_notifier_blocks()
>     sparx5_start()
> 
>   remove():
>     sparx5_unregister_notifier_blocks()
>     sparx5_unregister_netdevs()
>     sparx5_stats_deinit()
>     sparx5_mact_deinit()
>     sparx5_vcap_deinit()
>     sparx5_ptp_deinit()
>     sparx5_destroy_netdevs()
> 
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>

Note that there is the general principle that drivers should not
"publish" themselves (aka register their netdevs and/or ptp) until
they have initialised enough so the facility that has been published
is functional.

That, in general, means that there should be very little initialisation
after the calls to register the netdevs and ptp.

It's fine if something gets published and then a later publication of
another interface fails, causing the first publication to be withdrawn,
that is pretty much unavoidable, but in such scenarios, one would want
to do as much of the initialisation that may fail before any
publication of any user interfaces.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Hi Russell,

> > This series refactors the sparx5 init and deinit code out of
> > sparx5_start() and into probe(), adding proper per-subsystem cleanup
> > labels and deinit functions.
> >
> > Currently, the sparx5 driver initializes most subsystems inside
> > sparx5_start(), which is called from probe(). This includes registering
> > netdevs, starting worker threads for stats and MAC table polling,
> > requesting PTP IRQs, and initializing VCAP. The function has grown to
> > handle many unrelated subsystems, and has no granular error handling —
> > it either succeeds entirely or returns an error, leaving cleanup to a
> > single catch-all label in probe().
> >
> > The remove() path has a similar problem: teardown is not structured as
> > the reverse of initialization, and several subsystems lack proper deinit
> > functions. For example, the stats workqueue has no corresponding
> > cleanup, and the mact workqueue is destroyed without first cancelling
> > its delayed work.
> >
> > Refactor this by moving each init function out of sparx5_start() and
> > into probe(), with a corresponding goto-based cleanup label. Add deinit
> > functions for subsystems that allocate resources, to properly cancel
> > work and destroy workqueues. Ensure that cleanup order in both error
> > paths and remove() follows the reverse of initialization order. What
> > remains in sparx5_start() is only hardware register setup and FDMA/XTR
> > initialization that does not require cleanup.
> >
> > Before this series, most init functions live inside sparx5_start() with
> > no individual cleanup:
> >
> >   probe():
> >     sparx5_start():              <- no granular error handling
> >       sparx5_mact_init()
> >       sparx_stats_init()           <- starts worker, no cleanup
> >       mact_queue setup             <- no cancel on teardown
> >       sparx5_register_netdevs()
> >       sparx5_register_notifier_blocks()
> >       sparx5_vcap_init()
> >     sparx5_ptp_init()
> >
> >     probe() error path:
> >       cleanup_ports:
> >         sparx5_cleanup_ports()
> >         destroy_workqueue(mact_queue)
> >
> > After this series, probe() initializes subsystems in order with
> > matching cleanup labels, and remove() tears down in reverse:
> >
> > probe():
> >     sparx5_ptp_init()
> >     sparx5_vcap_init()
> >     sparx5_mact_init()
> >     sparx5_stats_init()
> >     sparx5_register_netdevs()
> >     sparx5_register_notifier_blocks()
> >     sparx5_start()
> >
> >   remove():
> >     sparx5_unregister_notifier_blocks()
> >     sparx5_unregister_netdevs()
> >     sparx5_stats_deinit()
> >     sparx5_mact_deinit()
> >     sparx5_vcap_deinit()
> >     sparx5_ptp_deinit()
> >     sparx5_destroy_netdevs()
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> 
> Note that there is the general principle that drivers should not
> "publish" themselves (aka register their netdevs and/or ptp) until
> they have initialised enough so the facility that has been published
> is functional.
> 
> That, in general, means that there should be very little initialisation
> after the calls to register the netdevs and ptp.
> 
> It's fine if something gets published and then a later publication of
> another interface fails, causing the first publication to be withdrawn,
> that is pretty much unavoidable, but in such scenarios, one would want
> to do as much of the initialisation that may fail before any
> publication of any user interfaces.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


Thanks for pointing this out.

Before this series, quite a bit of initialization was already done after
publication. But as a consequence of moving netdev registration out of
sparx5_start() (patch #2), some initialization that previously happened before
publication now happens after — which is the wrong direction.

I will do some reordering to fix this in v2.

Thanks!