Re: [PATCH v20 net-next 09/10] net/nebula-matrix: add common/ctrl dev init/remove operation
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-07-02 09:43:32
Also in:
linux-doc, lkml
From: AI Reviewer <redacted> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/ --- net/nebula-matrix: add common/ctrl dev init/remove operation This patch adds nbl_dev_setup_common_dev and nbl_dev_setup_ctrl_dev helpers, plus matching teardown paths, and wires them into nbl_dev_init and nbl_dev_remove.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c index 5deb21e35f8ef..8feae561d0f13 100644 --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dev.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -38,11 +190,29 @@ int nbl_dev_init(struct nbl_adapter *adapter) dev_mgt->chan_ops_tbl = chan_ops_tbl; adapter->core.dev_mgt = dev_mgt;
[Severity: High] Does this initialize the common device components before the chip is actually initialized? If nbl_dev_setup_common_dev() configures queues (which accesses hardware) before the hardware is brought up via nbl_dev_setup_ctrl_dev(), could writing to those queue registers cause hardware faults?
+ ret = nbl_dev_setup_common_dev(adapter);
+ if (ret)
+ return ret;
+
+ if (common->has_ctrl) {
+ ret = nbl_dev_setup_ctrl_dev(adapter);
+ if (ret)
+ goto setup_ctrl_dev_fail;
+ }
return 0;
+setup_ctrl_dev_fail:
+ nbl_dev_remove_common_dev(adapter);
+ return ret;
}
void nbl_dev_remove(struct nbl_adapter *adapter)
{[Severity: High] Are the teardown operations in the correct order here? The comment in nbl_dev_remove_ctrl_dev() mentions that setting the driver status to false triggers the firmware to clean up the qinfo registers. If nbl_dev_remove_ctrl_dev() deinitializes the chip first, will the subsequent call to nbl_dev_remove_common_dev() attempt to tear down the queues by writing directly to those deactivated qinfo registers? Could this lead to PCIe master aborts or panics during driver unbind?
+ struct nbl_common_info *common = &adapter->common; + + if (common->has_ctrl) + nbl_dev_remove_ctrl_dev(adapter); + nbl_dev_remove_common_dev(adapter); } /* ---------- Dev start process ---------- */
-- This is an AI-generated review.