Commit c3460457 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-sparx5-clean-up-probe-remove-init-and-deinit-paths'

Daniel Machon says:

====================
net: sparx5: clean up probe/remove init and deinit paths

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.
sparx5_start() is eliminated entirely — its hardware register setup
is renamed to sparx5_forwarding_init() and its FDMA/XTR setup is
extracted to sparx5_frame_io_init().

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_pgid_init()
    sparx5_vlan_init()
    sparx5_board_init()
    sparx5_forwarding_init()
    sparx5_calendar_init()         -> cleanup_ports
    sparx5_qos_init()              -> cleanup_ports
    sparx5_vcap_init()             -> cleanup_ports
    sparx5_mact_init()             -> cleanup_vcap
    sparx5_stats_init()            -> cleanup_mact
    sparx5_frame_io_init()         -> cleanup_stats
    sparx5_ptp_init()              -> cleanup_frame_io
    sparx5_register_netdevs()      -> cleanup_ptp
    sparx5_register_notifier_blocks() -> cleanup_netdevs

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

Link: https://patch.msgid.link/20260227-sparx5-init-deinit-v2-0-10ba54ccf005@microchip.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents d03c9ae6 1e540c4d
Loading
Loading
Loading
Loading
+13 −2
Original line number Diff line number Diff line
@@ -151,7 +151,7 @@ enum sparx5_cal_bw sparx5_get_port_cal_speed(struct sparx5 *sparx5, u32 portno)
}

/* Auto configure the QSYS calendar based on port configuration */
int sparx5_config_auto_calendar(struct sparx5 *sparx5)
static int sparx5_config_auto_calendar(struct sparx5 *sparx5)
{
	const struct sparx5_consts *consts = sparx5->data->consts;
	u32 cal[7], value, idx, portno;
@@ -578,7 +578,7 @@ static int sparx5_dsm_calendar_update(struct sparx5 *sparx5, u32 taxi,
}

/* Configure the DSM calendar based on port configuration */
int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
static int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
{
	const struct sparx5_ops *ops = sparx5->data->ops;
	int taxi;
@@ -610,3 +610,14 @@ int sparx5_config_dsm_calendar(struct sparx5 *sparx5)
	kfree(data);
	return err;
}

int sparx5_calendar_init(struct sparx5 *sparx5)
{
	int err;

	err = sparx5_config_auto_calendar(sparx5);
	if (err)
		return err;

	return sparx5_config_dsm_calendar(sparx5);
}
+8 −1
Original line number Diff line number Diff line
@@ -1244,7 +1244,7 @@ const struct ethtool_ops sparx5_ethtool_ops = {
	.set_pauseparam         = sparx5_set_pauseparam,
};

int sparx_stats_init(struct sparx5 *sparx5)
int sparx5_stats_init(struct sparx5 *sparx5)
{
	const struct sparx5_consts *consts = sparx5->data->consts;
	char queue_name[32];
@@ -1278,3 +1278,10 @@ int sparx_stats_init(struct sparx5 *sparx5)

	return 0;
}

void sparx5_stats_deinit(struct sparx5 *sparx5)
{
	cancel_delayed_work_sync(&sparx5->stats_work);
	destroy_workqueue(sparx5->stats_queue);
	mutex_destroy(&sparx5->queue_stats_lock);
}
+32 −2
Original line number Diff line number Diff line
@@ -419,7 +419,7 @@ static void sparx5_mact_handle_entry(struct sparx5 *sparx5,
				  true);
}

void sparx5_mact_pull_work(struct work_struct *work)
static void sparx5_mact_pull_work(struct work_struct *work)
{
	struct delayed_work *del_work = to_delayed_work(work);
	struct sparx5 *sparx5 = container_of(del_work, struct sparx5,
@@ -489,8 +489,10 @@ void sparx5_set_ageing(struct sparx5 *sparx5, int msecs)
		 LRN_AUTOAGE_CFG(0));
}

void sparx5_mact_init(struct sparx5 *sparx5)
int sparx5_mact_init(struct sparx5 *sparx5)
{
	char queue_name[32];

	mutex_init(&sparx5->lock);

	/*  Flush MAC table */
@@ -502,4 +504,32 @@ void sparx5_mact_init(struct sparx5 *sparx5)
		dev_warn(sparx5->dev, "MAC flush error\n");

	sparx5_set_ageing(sparx5, BR_DEFAULT_AGEING_TIME / HZ * 1000);

	/* Add host mode BC address (points only to CPU) */
	sparx5_mact_learn(sparx5, sparx5_get_pgid(sparx5, PGID_CPU),
			  (unsigned char[]){0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
			  NULL_VID);

	mutex_init(&sparx5->mdb_lock);
	INIT_LIST_HEAD(&sparx5->mdb_entries);
	mutex_init(&sparx5->mact_lock);
	INIT_LIST_HEAD(&sparx5->mact_entries);
	snprintf(queue_name, sizeof(queue_name), "%s-mact",
		 dev_name(sparx5->dev));
	sparx5->mact_queue = create_singlethread_workqueue(queue_name);
	if (!sparx5->mact_queue)
		return -ENOMEM;

	INIT_DELAYED_WORK(&sparx5->mact_work, sparx5_mact_pull_work);
	queue_delayed_work(sparx5->mact_queue, &sparx5->mact_work,
			   SPX5_MACT_PULL_DELAY);

	return 0;
}

void sparx5_mact_deinit(struct sparx5 *sparx5)
{
	cancel_delayed_work_sync(&sparx5->mact_work);
	destroy_workqueue(sparx5->mact_queue);
	mutex_destroy(&sparx5->mact_lock);
}
+118 −135
Original line number Diff line number Diff line
@@ -658,6 +658,58 @@ static int sparx5_qlim_set(struct sparx5 *sparx5)
	return 0;
}

static int sparx5_frame_io_init(struct sparx5 *sparx5)
{
	const struct sparx5_ops *ops = sparx5->data->ops;
	int err = -ENXIO;

	/* Start Frame DMA with fallback to register based INJ/XTR */
	if (sparx5->fdma_irq >= 0) {
		if (GCB_CHIP_ID_REV_ID_GET(sparx5->chip_id) > 0 ||
		    !is_sparx5(sparx5))
			err = devm_request_irq(sparx5->dev,
					       sparx5->fdma_irq,
					       sparx5_fdma_handler,
					       0,
					       "sparx5-fdma", sparx5);
		if (!err) {
			err = ops->fdma_init(sparx5);
			if (!err)
				sparx5_fdma_start(sparx5);
		}
		if (err)
			sparx5->fdma_irq = -ENXIO;
	} else {
		sparx5->fdma_irq = -ENXIO;
	}
	if (err && sparx5->xtr_irq >= 0) {
		err = devm_request_irq(sparx5->dev, sparx5->xtr_irq,
				       sparx5_xtr_handler, IRQF_SHARED,
				       "sparx5-xtr", sparx5);
		if (!err)
			err = sparx5_manual_injection_mode(sparx5);
		if (err)
			sparx5->xtr_irq = -ENXIO;
	} else {
		sparx5->xtr_irq = -ENXIO;
	}

	return err;
}

static void sparx5_frame_io_deinit(struct sparx5 *sparx5)
{
	if (sparx5->xtr_irq >= 0) {
		disable_irq(sparx5->xtr_irq);
		sparx5->xtr_irq = -ENXIO;
	}
	if (sparx5->fdma_irq >= 0) {
		disable_irq(sparx5->fdma_irq);
		sparx5->data->ops->fdma_deinit(sparx5);
		sparx5->fdma_irq = -ENXIO;
	}
}

/* Some boards needs to map the SGPIO for signal detect explicitly to the
 * port module
 */
@@ -683,14 +735,10 @@ static void sparx5_board_init(struct sparx5 *sparx5)
					GCB_HW_SGPIO_TO_SD_MAP_CFG(idx));
}

static int sparx5_start(struct sparx5 *sparx5)
static void sparx5_forwarding_init(struct sparx5 *sparx5)
{
	u8 broadcast[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
	const struct sparx5_consts *consts = sparx5->data->consts;
	const struct sparx5_ops *ops = sparx5->data->ops;
	char queue_name[32];
	u32 idx;
	int err;

	/* Setup own UPSIDs */
	for (idx = 0; idx < consts->n_own_upsids; idx++) {
@@ -728,117 +776,9 @@ static int sparx5_start(struct sparx5 *sparx5)
			 ANA_CL_FILTER_CTRL_FORCE_FCS_UPDATE_ENA,
			 sparx5, ANA_CL_FILTER_CTRL(idx));

	/* Init MAC table, ageing */
	sparx5_mact_init(sparx5);

	/* Init PGID table arbitrator */
	sparx5_pgid_init(sparx5);

	/* Setup VLANs */
	sparx5_vlan_init(sparx5);

	/* Add host mode BC address (points only to CPU) */
	sparx5_mact_learn(sparx5, sparx5_get_pgid(sparx5, PGID_CPU), broadcast,
			  NULL_VID);

	/* Enable queue limitation watermarks */
	sparx5_qlim_set(sparx5);

	err = sparx5_config_auto_calendar(sparx5);
	if (err)
		return err;

	err = sparx5_config_dsm_calendar(sparx5);
	if (err)
		return err;

	/* Init stats */
	err = sparx_stats_init(sparx5);
	if (err)
		return err;

	/* Init mact_sw struct */
	mutex_init(&sparx5->mact_lock);
	INIT_LIST_HEAD(&sparx5->mact_entries);
	snprintf(queue_name, sizeof(queue_name), "%s-mact",
		 dev_name(sparx5->dev));
	sparx5->mact_queue = create_singlethread_workqueue(queue_name);
	if (!sparx5->mact_queue)
		return -ENOMEM;

	INIT_DELAYED_WORK(&sparx5->mact_work, sparx5_mact_pull_work);
	queue_delayed_work(sparx5->mact_queue, &sparx5->mact_work,
			   SPX5_MACT_PULL_DELAY);

	mutex_init(&sparx5->mdb_lock);
	INIT_LIST_HEAD(&sparx5->mdb_entries);

	err = sparx5_register_netdevs(sparx5);
	if (err)
		return err;

	sparx5_board_init(sparx5);
	err = sparx5_register_notifier_blocks(sparx5);
	if (err)
		return err;

	err = sparx5_vcap_init(sparx5);
	if (err) {
		sparx5_unregister_notifier_blocks(sparx5);
		return err;
	}

	/* Start Frame DMA with fallback to register based INJ/XTR */
	err = -ENXIO;
	if (sparx5->fdma_irq >= 0) {
		if (GCB_CHIP_ID_REV_ID_GET(sparx5->chip_id) > 0 ||
		    !is_sparx5(sparx5))
			err = devm_request_irq(sparx5->dev,
					       sparx5->fdma_irq,
					       sparx5_fdma_handler,
					       0,
					       "sparx5-fdma", sparx5);
		if (!err) {
			err = ops->fdma_init(sparx5);
			if (!err)
				sparx5_fdma_start(sparx5);
		}
		if (err)
			sparx5->fdma_irq = -ENXIO;
	} else {
		sparx5->fdma_irq = -ENXIO;
	}
	if (err && sparx5->xtr_irq >= 0) {
		err = devm_request_irq(sparx5->dev, sparx5->xtr_irq,
				       sparx5_xtr_handler, IRQF_SHARED,
				       "sparx5-xtr", sparx5);
		if (!err)
			err = sparx5_manual_injection_mode(sparx5);
		if (err)
			sparx5->xtr_irq = -ENXIO;
	} else {
		sparx5->xtr_irq = -ENXIO;
	}

	if (sparx5->ptp_irq >= 0 &&
	    sparx5_has_feature(sparx5, SPX5_FEATURE_PTP)) {
		err = devm_request_threaded_irq(sparx5->dev, sparx5->ptp_irq,
						NULL, ops->ptp_irq_handler,
						IRQF_ONESHOT, "sparx5-ptp",
						sparx5);
		if (err)
			sparx5->ptp_irq = -ENXIO;

		sparx5->ptp = 1;
	}

	return err;
}

static void sparx5_cleanup_ports(struct sparx5 *sparx5)
{
	sparx5_unregister_netdevs(sparx5);
	sparx5_destroy_netdevs(sparx5);
}

static int mchp_sparx5_probe(struct platform_device *pdev)
@@ -999,9 +939,14 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
		}
	}

	err = sparx5_start(sparx5);
	sparx5_pgid_init(sparx5);
	sparx5_vlan_init(sparx5);
	sparx5_board_init(sparx5);
	sparx5_forwarding_init(sparx5);

	err = sparx5_calendar_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Start failed\n");
		dev_err(sparx5->dev, "Failed to initialize calendar\n");
		goto cleanup_ports;
	}

@@ -1011,20 +956,66 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
		goto cleanup_ports;
	}

	err = sparx5_ptp_init(sparx5);
	err = sparx5_vcap_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "PTP failed\n");
		dev_err(sparx5->dev, "Failed to initialize VCAP\n");
		goto cleanup_ports;
	}

	err = sparx5_mact_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to initialize MAC table\n");
		goto cleanup_vcap;
	}

	err = sparx5_stats_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to initialize stats\n");
		goto cleanup_mact;
	}

	INIT_LIST_HEAD(&sparx5->mall_entries);

	err = sparx5_frame_io_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to initialize frame I/O\n");
		goto cleanup_stats;
	}

	err = sparx5_ptp_init(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to initialize PTP\n");
		goto cleanup_frame_io;
	}

	err = sparx5_register_netdevs(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to register net devices\n");
		goto cleanup_ptp;
	}

	err = sparx5_register_notifier_blocks(sparx5);
	if (err) {
		dev_err(sparx5->dev, "Failed to register notifier blocks\n");
		goto cleanup_netdevs;
	}

	goto cleanup_config;

cleanup_netdevs:
	sparx5_unregister_netdevs(sparx5);
cleanup_ptp:
	sparx5_ptp_deinit(sparx5);
cleanup_frame_io:
	sparx5_frame_io_deinit(sparx5);
cleanup_stats:
	sparx5_stats_deinit(sparx5);
cleanup_mact:
	sparx5_mact_deinit(sparx5);
cleanup_vcap:
	sparx5_vcap_deinit(sparx5);
cleanup_ports:
	sparx5_cleanup_ports(sparx5);
	if (sparx5->mact_queue)
		destroy_workqueue(sparx5->mact_queue);
	sparx5_destroy_netdevs(sparx5);
cleanup_config:
	kfree(configs);
cleanup_pnode:
@@ -1035,24 +1026,16 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
static void mchp_sparx5_remove(struct platform_device *pdev)
{
	struct sparx5 *sparx5 = platform_get_drvdata(pdev);
	const struct sparx5_ops *ops = sparx5->data->ops;

	debugfs_remove_recursive(sparx5->debugfs_root);
	if (sparx5->xtr_irq) {
		disable_irq(sparx5->xtr_irq);
		sparx5->xtr_irq = -ENXIO;
	}
	if (sparx5->fdma_irq) {
		disable_irq(sparx5->fdma_irq);
		sparx5->fdma_irq = -ENXIO;
	}
	sparx5_ptp_deinit(sparx5);
	ops->fdma_deinit(sparx5);
	sparx5_cleanup_ports(sparx5);
	sparx5_vcap_destroy(sparx5);
	/* Unregister netdevs */
	sparx5_unregister_notifier_blocks(sparx5);
	destroy_workqueue(sparx5->mact_queue);
	sparx5_unregister_netdevs(sparx5);
	sparx5_ptp_deinit(sparx5);
	sparx5_frame_io_deinit(sparx5);
	sparx5_stats_deinit(sparx5);
	sparx5_mact_deinit(sparx5);
	sparx5_vcap_deinit(sparx5);
	sparx5_destroy_netdevs(sparx5);
}

static const struct sparx5_regs sparx5_regs = {
+6 −6
Original line number Diff line number Diff line
@@ -470,7 +470,6 @@ void sparx5_fdma_reload(struct sparx5 *sparx5, struct fdma *fdma);
void sparx5_fdma_injection_mode(struct sparx5 *sparx5);

/* sparx5_mactable.c */
void sparx5_mact_pull_work(struct work_struct *work);
int sparx5_mact_learn(struct sparx5 *sparx5, int port,
		      const unsigned char mac[ETH_ALEN], u16 vid);
bool sparx5_mact_getnext(struct sparx5 *sparx5,
@@ -489,7 +488,8 @@ int sparx5_del_mact_entry(struct sparx5 *sparx5,
int sparx5_mc_sync(struct net_device *dev, const unsigned char *addr);
int sparx5_mc_unsync(struct net_device *dev, const unsigned char *addr);
void sparx5_set_ageing(struct sparx5 *sparx5, int msecs);
void sparx5_mact_init(struct sparx5 *sparx5);
int sparx5_mact_init(struct sparx5 *sparx5);
void sparx5_mact_deinit(struct sparx5 *sparx5);

/* sparx5_vlan.c */
void sparx5_pgid_update_mask(struct sparx5_port *port, int pgid, bool enable);
@@ -504,8 +504,7 @@ int sparx5_vlan_vid_del(struct sparx5_port *port, u16 vid);
void sparx5_vlan_port_apply(struct sparx5 *sparx5, struct sparx5_port *port);

/* sparx5_calendar.c */
int sparx5_config_auto_calendar(struct sparx5 *sparx5);
int sparx5_config_dsm_calendar(struct sparx5 *sparx5);
int sparx5_calendar_init(struct sparx5 *sparx5);
int sparx5_dsm_calendar_calc(struct sparx5 *sparx5, u32 taxi,
			     struct sparx5_calendar_data *data);
u32 sparx5_cal_speed_to_value(enum sparx5_cal_bw speed);
@@ -514,7 +513,8 @@ enum sparx5_cal_bw sparx5_get_port_cal_speed(struct sparx5 *sparx5, u32 portno);

/* sparx5_ethtool.c */
void sparx5_get_stats64(struct net_device *ndev, struct rtnl_link_stats64 *stats);
int sparx_stats_init(struct sparx5 *sparx5);
int sparx5_stats_init(struct sparx5 *sparx5);
void sparx5_stats_deinit(struct sparx5 *sparx5);

/* sparx5_dcb.c */
#ifdef CONFIG_SPARX5_DCB
@@ -563,7 +563,7 @@ void sparx5_get_hwtimestamp(struct sparx5 *sparx5,

/* sparx5_vcap_impl.c */
int sparx5_vcap_init(struct sparx5 *sparx5);
void sparx5_vcap_destroy(struct sparx5 *sparx5);
void sparx5_vcap_deinit(struct sparx5 *sparx5);

/* sparx5_pgid.c */
enum sparx5_pgid_type {
Loading