Skip to contentSkip to conversation

net: dsa: mxl862xx: support firmware update

PATCH Superseded Daniel Golle 2 participants 13 messages Jul 25 – Aug 1

Subsystems (6)

v5 of 19 Superseded Latest is v19 →

lore
Share & tools
lore

Daniel Golle

author maintainer
#0 lore raw

v5 cover net: dsa: mxl862xx: support firmware update

The firmware of MxL862xx managed Ethernet switches can be updated in-system via the same MDIO bus which is also used to manage the switch. Wire up the devlink flash_update operation for DSA drivers and implement firmware update and version reporting in the mxl862xx driver.

Changes since v4:
- report the numeric chip part number and version read from the static CHIP ID registers as the "asic.id" and "asic.rev" fixed versions instead of a model-name string, which does not belong in a devlink version identifier (Jakub Kicinski)
- report the running firmware version as the "stored" version too, since the switch boots it from its own flash, so userspace can distinguish a flash-backed part from a flashless one by the presence of "stored" without a future API change
- run the post-flash reprobe from a self-contained work item again instead of the v4 kernel thread, which tripped the hung-task watchdog while parked across the flash and returned -EINTR from kthread_create() when the devlink command was interrupted
- re-read the new firmware version through the reprobe's fresh probe and drop the SYS_MISC_FW_VERSION exemption from the host block
- raise the firmware command poll timeout so the FW_UPDATE command that reboots into MCUboot is not cut short
- detect the switch state from the value MCUboot publishes in the SB PDI STAT register (loader ready, wedged download, or running firmware), confirming a live console loader with a register-read challenge, instead of trusting a bare SMDIO scratch write
- fail probe with -ENODEV over SB PDI when the switch does not respond at all (absent, unpowered, or misdescribed in the device tree) instead of letting the clause-45 API flood the log with CRC errors
- drain a wedged interrupted download back to a clean ready state from a background work item so the multi-minute recovery never holds the devlink instance lock, reporting no firmware version and refusing flash with -EBUSY until it completes
- report the rescue-mode null firmware version "0.0.0" as both the running and stored version, matching the running/stored reporting above
- add devlink documentation (patch 5/5) adding Documentation/networking/devlink/mxl862xx.rst describing the info versions and the flash update behaviour (Jakub Kicinski)
- include example "devlink dev info" outputs in the commit messages of patches 3 and 4 (Jakub Kicinski)

Changes since v3:
- only install the flash_update devlink op for switches whose driver implements it, so the devlink core rejects unsupported requests before fetching the firmware file from userspace
- run the deferred reprobe from a kernel thread which ends in module_put_and_kthread_exit() instead of a work item that dropped its module reference while still executing module code
- fail firmware API read commands with -ENODEV after the update has finished instead of faking success with an unfilled buffer, which could send port_fdb_dump() into an endless loop
- keep the host block in place across the post-update version query by exempting SYS_MISC_FW_VERSION from block_host instead of briefly lifting the block, and write all blocking flags under the MDIO bus lock
- check the return value of all SB PDI control writes; a failed address write during the half-bank switch could otherwise place the second half of the payload at the wrong flash offset
- initialise the progress notification deadline from jiffies so notifications are not suppressed on 32-bit systems shortly after boot
- log a distinct diagnostic when rescue mode detection fails on an SMDIO bus error instead of silently treating it as not being in rescue mode
- flush the switchdev deferred queue after closing the ports so the bridge's deferred STP DISABLED transitions reach the firmware while it is still running instead of failing against the host block with "failed to set STP state" errors
- treat -ENODEV as successful deletion in port_mdb_del() so the post-update teardown no longer leaves host MDB entries behind for the DSA core to report when the tree is torn down

Changes since v2:
- validate the firmware image, including both CRCs, before taking down any ports, so that a malformed file is rejected without disturbing the running switch and without the needless flash and reprobe cycle it previously triggered
- reject images whose declared payload sizes overflow when summed (check_add_overflow) or sum up to zero; the latter previously erased the flash without writing anything back
- allocate the reprobe work item and take the module and device references before starting the update, so scheduling the reprobe can no longer fail after the switch has been pushed into MCUboot
- prevent the stats poll work from being re-armed and cancel the CRC error work before starting the transfer
- check the host-blocking flags in mxl862xx_api_wrap() under the MDIO bus lock to close the race window where an API command which had already passed the check could reach the bus after the switch rebooted into MCUboot
- check the return value of SB PDI data word writes so a failed MDIO transaction aborts the transfer instead of being noticed only through a corrupted image
- report a per-model chip name (e.g. "MaxLinear MxL86252") as the devlink "asic.id" fixed version instead of the devicetree compatible string, whose comma is awkward for userspace consumers such as fwupd (see discussion on v2 patch 3)
- report the canonical null version "0.0.0" instead of "mcuboot-rescue" as the running firmware version in rescue mode, so that version-comparing update tools like fwupd treat every available release as an upgrade and offer it for recovery

Changes since RFC [1]:
- detect a switch stuck in MCUboot rescue mode at probe, register the switch without any ports and report "mcuboot-rescue" as the running firmware version, so devlink flash can recover from a failed or interrupted update (Andrew Lunn)
- clarify in the commit message of patch 2 that the per-transaction MDIO bus locking is about other, non-switch devices on the same MDIO bus (Andrew Lunn)
- mention in the commit message of patch 3 that closing the ports also stops phylib from polling the switch-internal PHYs during the transfer (Andrew Lunn)
- split up run-on sentence and explain the dynamically allocated reprobe work item instead of just pointing at iwlwifi in the commit message of patch 3 (Manuel Ebner)
- use kzalloc_obj() (Manuel Ebner)
- state the actual duration of a complete flash and reprobe cycle (just under a minute) in comments and the commit message, and clarify that the timeout values are generous upper bounds (Manuel Ebner)

[1] https://lore.kernel.org/all/[email protected]/
Daniel Golle (5):
net: dsa: wire flash_update devlink callback to drivers
net: dsa: mxl862xx: add SMDIO clause-22 register access
net: dsa: mxl862xx: add devlink flash_update and info_get
net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
net: dsa: mxl862xx: document devlink flash and info support

 Documentation/networking/devlink/index.rst    |   1 +
 Documentation/networking/devlink/mxl862xx.rst |  63 ++
 MAINTAINERS                                   |   1 +
 drivers/net/dsa/mxl862xx/Makefile             |   2 +-
 drivers/net/dsa/mxl862xx/mxl862xx-api.h       |  10 +
 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h       |   2 +
 drivers/net/dsa/mxl862xx/mxl862xx-fw.c        | 884 ++++++++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-fw.h        |  20 +
 drivers/net/dsa/mxl862xx/mxl862xx-host.c      |  54 ++
 drivers/net/dsa/mxl862xx/mxl862xx-host.h      |   2 +
 drivers/net/dsa/mxl862xx/mxl862xx-phylink.c   |   2 +
 drivers/net/dsa/mxl862xx/mxl862xx.c           | 116 ++-
 drivers/net/dsa/mxl862xx/mxl862xx.h           |  24 +
 include/net/dsa.h                             |   3 +
 net/dsa/devlink.c                             |  50 +-
 15 files changed, 1214 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/networking/devlink/mxl862xx.rst
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-fw.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-fw.h

base-commit: 89d8006259b81dd25c962f6cc8d7ab268d6ea426
prerequisite-patch-id: 0000000000000000000000000000000000000000

Signature · 1 line
2.55.0
author maintainer
#1 lore raw

PATCH v5 1/5 net: dsa: wire flash_update devlink callback to drivers

Add a devlink_flash_update callback to dsa_switch_ops so that DSA drivers can support devlink dev flash without open-coding the devlink plumbing. Unlike the other trampolines in net/dsa/devlink.c, the flash_update op is only installed for switches whose driver implements the callback: the devlink core rejects flash requests up front when the op is absent, before fetching the firmware file from userspace, and an unconditionally present trampoline would defeat that early check and let unsupported requests block on request_firmware() only to fail with -EOPNOTSUPP afterwards.

---
v5: no changes

v4:
* only install the flash_update op for drivers implementing the callback so the devlink core keeps rejecting unsupported flash requests before fetching the firmware file

v3: no changes
v2: align continuation lines with the open parenthesis

 include/net/dsa.h |  3 +++
 net/dsa/devlink.c | 50 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 41 insertions(+), 12 deletions(-)
include/net/dsa.h +3 -0
index 8c16ef23cc10..c9e19348de61 100644
@@ -1170,6 +1170,9 @@ struct dsa_switch_ops {
 	int	(*devlink_info_get)(struct dsa_switch *ds,
 				    struct devlink_info_req *req,
 				    struct netlink_ext_ack *extack);
+	int	(*devlink_flash_update)(struct dsa_switch *ds,
+					struct devlink_flash_update_params *params,
+					struct netlink_ext_ack *extack);
 	int	(*devlink_sb_pool_get)(struct dsa_switch *ds,
 				       unsigned int sb_index, u16 pool_index,
 				       struct devlink_sb_pool_info *pool_info);
net/dsa/devlink.c +38 -12

74 lines not on this page · raw message

Signature · 1 line
2.55.0
maintainer
#2 lore raw

Re: [PATCH net-next v5 1/5] net: dsa: wire flash_update devlink callback to drivers

Quoted 21 lines · Daniel Golle

Add a devlink_flash_update callback to dsa_switch_ops so that DSA drivers can support devlink dev flash without open-coding the devlink plumbing. Unlike the other trampolines in net/dsa/devlink.c, the flash_update op is only installed for switches whose driver implements the callback: the devlink core rejects flash requests up front when the op is absent, before fetching the firmware file from userspace, and an unconditionally present trampoline would defeat that early check and let unsupported requests block on request_firmware() only to fail with

net/dsa/devlink.c +11 -1
-EOPNOTSUPP afterwards.
 
+/* The devlink core rejects flash requests up front when the flash_update
+ * op is absent, before fetching the firmware file from userspace. Only
+ * install the op for switches whose driver implements it, so that
+ * unsupported requests keep failing early.
+ */
+#define DSA_DEVLINK_OPS							\
+	.info_get			= dsa_devlink_info_get,		\
+	.sb_pool_get			= dsa_devlink_sb_pool_get,	\
+	.sb_pool_set			= dsa_devlink_sb_pool_set,	\
+	.sb_port_pool_get		= dsa_devlink_sb_port_pool_get,	\
+	.sb_port_pool_set		= dsa_devlink_sb_port_pool_set,	\

net/dsa/devlink.c dsa_devlink_sb_occ_tc_port_bind_get(struct devlink_port *dlp, · line 199

Quoted 10 lines
net/dsa/devlink.c +9 -0
+	.sb_tc_pool_bind_get		= dsa_devlink_sb_tc_pool_bind_get, \
+	.sb_tc_pool_bind_set		= dsa_devlink_sb_tc_pool_bind_set, \
+	.sb_occ_snapshot		= dsa_devlink_sb_occ_snapshot,	\
+	.sb_occ_max_clear		= dsa_devlink_sb_occ_max_clear,	\
+	.sb_occ_port_pool_get		= dsa_devlink_sb_occ_port_pool_get, \
 	.sb_occ_tc_port_bind_get	= dsa_devlink_sb_occ_tc_port_bind_get,
+
+static const struct devlink_ops dsa_devlink_ops = {
+	DSA_DEVLINK_OPS
+};

Is this going to scale? How many different permutations are there?

Firmware upgrade is an infrequent operation, so do we actually care about a request_firmware() which might turn out to be pointless? I personally would prefer to avoid the scalability issue and just return -EOPNOTSUPP.

Andrew

author maintainer
#3 lore raw

Re: [PATCH net-next v5 1/5] net: dsa: wire flash_update devlink callback to drivers

On Thu, Jul 30, 2026 at 11:08:35PM +0200, Andrew Lunn wrote:

A quick note first: this review crossed mid-air with newer versions, v6 through v8 were already out. v8 is the current one, where this patch is retitled "net: dsa: add devlink flash_update callback to dsa_switch_ops":
https://lore.kernel.org/all/2103d1c460b35c5e59c96bd91a892596478474d1.1785389905.git.daniel@makrotopia.org/
This part is unchanged there, though, so your comment applies as-is.

Quoted 1 line
Quoted 1 line

+#define DSA_DEVLINK_OPS \

[...]

Quoted 5 lines

net/dsa/devlink.c dsa_devlink_sb_occ_tc_port_bind_get(struct devlink_port *dlp, · line 199

Quoted 3 lines
net/dsa/devlink.c +3 -0
+static const struct devlink_ops dsa_devlink_ops = {
+	DSA_DEVLINK_OPS
+};

Is this going to scale? How many different permutations are there?

It obviously doesn't scale. The choice to do this was because the cost of requesting an update file, which would happen before the -EOPNOTSUPP, is a relatively high cost. None of the other devlink operations have such costly side-effects even if not implemented by a DSA driver.

Quoted 4 lines

Firmware upgrade is an infrequent operation, so do we actually care about a request_firmware() which might turn out to be pointless? I personally would prefer to avoid the scalability issue and just return -EOPNOTSUPP.

I will drop the macro and the second struct, make .flash_update a regular member of the single dsa_devlink_ops, and have the trampoline return -EOPNOTSUPP when ds->ops->devlink_flash_update is absent in v9.

Cheers

Daniel

author maintainer
#4 lore raw

PATCH v5 2/5 net: dsa: mxl862xx: add SMDIO clause-22 register access

In reply to Daniel Golle

Add mxl862xx_smdio_read() and mxl862xx_smdio_write() for clause-22 SMDIO register access. The switch's MCUboot bootloader exposes only clause-22 registers; the clause-45 MMD interface needs the running firmware and is unavailable while the switch is in MCUboot. The MDIO bus lock is held per-transaction (not across polls) so that SB PDI polling during flash erase does not starve other non-switch users of the same MDIO bus, such as separate PHYs providing WAN or management interfaces.

Unlike mxl862xx_api_wrap(), which takes the bus lock with MDIO_MUTEX_NESTED because it can be entered from the accessors of the switch-internal MDIO bus while that bus's lock of the same lock class is already held, the SMDIO helpers take it with a plain mutex_lock(). They are only called from probe and devlink flash contexts where no other MDIO bus lock can be held.

---
v5: no changes

v4: no changes

v3: explain the plain mutex_lock() vs MDIO_MUTEX_NESTED choice in
the commit message

v2: clarify in the commit message that the per-transaction bus
locking is about unrelated non-switch devices on the same MDIO bus (Andrew Lunn)

 drivers/net/dsa/mxl862xx/mxl862xx-host.c | 35 ++++++++++++++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-host.h |  2 ++
 2 files changed, 37 insertions(+)
drivers/net/dsa/mxl862xx/mxl862xx-host.c +35 -0
index 4acd216f7cc0..6e582caea1fa 100644
@@ -495,6 +495,41 @@ int mxl862xx_reset(struct mxl862xx_priv *priv)
 	return ret;
 }
 
+#define MXL862XX_SMDIO_ADDR_REG		0x1f
+#define MXL862XX_SMDIO_PAGE_MASK	0xfff0
+#define MXL862XX_SMDIO_OFF_MASK		0x000f
+
+int mxl862xx_smdio_read(struct mxl862xx_priv *priv, u32 addr)
+{
+	struct mii_bus *bus = priv->mdiodev->bus;
+	int phy = priv->mdiodev->addr;
+	int ret;
+
+	mutex_lock(&bus->mdio_lock);
+	ret = __mdiobus_write(bus, phy, MXL862XX_SMDIO_ADDR_REG,
+			      addr & MXL862XX_SMDIO_PAGE_MASK);
+	if (ret >= 0)
+		ret = __mdiobus_read(bus, phy, addr & MXL862XX_SMDIO_OFF_MASK);
+	mutex_unlock(&bus->mdio_lock);
+	return ret;
+}
+
+int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val)
+{
+	struct mii_bus *bus = priv->mdiodev->bus;
+	int phy = priv->mdiodev->addr;
+	int ret;
+
+	mutex_lock(&bus->mdio_lock);
+	ret = __mdiobus_write(bus, phy, MXL862XX_SMDIO_ADDR_REG,
+			      addr & MXL862XX_SMDIO_PAGE_MASK);
+	if (ret >= 0)
+		ret = __mdiobus_write(bus, phy, addr & MXL862XX_SMDIO_OFF_MASK,
+				      val);
+	mutex_unlock(&bus->mdio_lock);
+	return ret;
+}
+
 void mxl862xx_host_init(struct mxl862xx_priv *priv)
 {
 	INIT_WORK(&priv->crc_err_work, mxl862xx_crc_err_work_fn);
drivers/net/dsa/mxl862xx/mxl862xx-host.h +2 -0
index 66d6ae198aff..4e054c6e4c0e 100644
@@ -18,5 +18,7 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *data, u16 size,
 	mxl862xx_api_wrap(dev, cmd, &(data), sizeof((data)), true, true)
 
 int mxl862xx_reset(struct mxl862xx_priv *priv);
+int mxl862xx_smdio_read(struct mxl862xx_priv *priv, u32 addr);
+int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val);
 
 #endif /* __MXL862XX_HOST_H */
Signature · 1 line
2.55.0
maintainer Reviewed-by on v5 2/5
#5 lore raw

Re: [PATCH net-next v5 2/5] net: dsa: mxl862xx: add SMDIO clause-22 register access

Quoted 9 lines · Daniel Golle

Add mxl862xx_smdio_read() and mxl862xx_smdio_write() for clause-22 SMDIO register access. The switch's MCUboot bootloader exposes only clause-22 registers; the clause-45 MMD interface needs the running firmware and is unavailable while the switch is in MCUboot. The MDIO bus lock is held per-transaction (not across polls) so that SB PDI polling during flash erase does not starve other non-switch users of the same MDIO bus, such as separate PHYs providing WAN or management interfaces.

Quoted 8 lines

Unlike mxl862xx_api_wrap(), which takes the bus lock with MDIO_MUTEX_NESTED because it can be entered from the accessors of the switch-internal MDIO bus while that bus's lock of the same lock class is already held, the SMDIO helpers take it with a plain mutex_lock(). They are only called from probe and devlink flash contexts where no other MDIO bus lock can be held.

Andrew

author maintainer
#6 lore raw

PATCH v5 3/5 net: dsa: mxl862xx: add devlink flash_update and info_get

In reply to Daniel Golle

Implement runtime firmware upgrade via "devlink dev flash" and version reporting via "devlink dev info":

  $ devlink dev info mdio_bus/mdio-bus:10
  mdio_bus/mdio-bus:10:
    driver mxl862xx
    versions:
        fixed:
          asic.id 8628
          asic.rev 0
        running:
          fw 1.0.70
        stored:
          fw 1.0.70

The "asic.id" and "asic.rev" fixed versions carry the numeric chip part number and revision from the static CHIP ID registers (SYS_MISC_REG_RD), which userspace such as fwupd matches firmware against. The switch boots its firmware from its own flash, so "fw" is reported as both the running and the stored version; a flashless part would omit "stored", distinguishing the two without an API change.

  $ devlink dev flash mdio_bus/mdio-bus:10 file mxl862xx-fw.bin

The image, including both payload CRCs, is validated first, so a malformed file is rejected without disturbing the running switch. The driver then sends SYS_MISC_FW_UPDATE, which reboots the switch into its MCUboot bootloader, and transfers the signed image over the SB PDI protocol (clause-22 SMDIO), checking every write: a failed address write at the half-bank boundary would otherwise misplace half the payload unnoticed. A successful transfer reboots the switch into the new firmware. The whole cycle takes just under a minute.

For its duration the driver closes all user and conduit interfaces and marks the user ports not-present with netif_device_detach() so userspace cannot reopen them; the conduit belongs to the MAC driver and is only closed. This also stops phylib polling the switch-internal PHYs, unreachable in MCUboot. The bridge's deferred STP DISABLED transitions are flushed under rtnl so they reach the firmware while it still runs; the stats poll and CRC error handler are stopped; and firmware API commands from other paths are blocked under the MDIO bus lock so none reaches the bus once the switch has rebooted. Progress is reported through devlink status notifications.

The switch leaves MCUboot on its own by booting the new image, but the driver has no in-place path back, so it reinitialises with a full device_reprobe() scheduled regardless of the transfer outcome -- after a failure the switch is still in MCUboot and probe re-detects it. During the teardown its API reads return -ENODEV and writes fake success, so it neither stalls on the absent firmware nor consumes buffers it never filled. The reprobe holds module and device references taken before the switch was disturbed and runs from a heap work item, not a kernel
thread: remove() frees the devm-managed priv, and kthread_create() in the devlink caller would return -EINTR on interrupt or trip the hung-task watchdog while parked across the minute-long flash.

---
v5:
- report the numeric chip part number and version read from the static CHIP ID registers as "asic.id" and "asic.rev" instead of a model-name string, which does not belong in a devlink version identifier (Jakub Kicinski)
- report the running firmware version as the "stored" version too, since the switch boots it from its own flash, so userspace can tell a flash-backed part from a flashless one by the presence of "stored" without a future API change
- run the post-flash reprobe from a self-contained work item again instead of the v4 kernel thread, which tripped the hung-task watchdog while parked across the flash and returned -EINTR from kthread_create() when the devlink command was interrupted
- re-read the new firmware version through the reprobe's fresh probe and drop the SYS_MISC_FW_VERSION exemption from the host block
- raise the firmware command poll timeout so the FW_UPDATE command that reboots into MCUboot is not cut short
- move the devlink documentation into its own patch

v4:
- run the deferred reprobe from a kernel thread ending in module_put_and_kthread_exit() instead of a work item whose final module_put() raced against module unload
- fail API read commands with -ENODEV after the update instead of faking success with an unfilled buffer, which sent port_fdb_dump() into an endless loop
- keep block_host set across the post-update version query by exempting SYS_MISC_FW_VERSION instead of briefly lifting the block, and write the blocking flags under the MDIO bus lock
- check the return value of every SB PDI control write; a failed address write during the half-bank switch could place the second half of the payload at the wrong flash offset undetected
- report SMDIO write failures through one shared error path instead of per-site messages
- initialise the progress notification deadline from jiffies so notifications are not suppressed on 32-bit shortly after boot
- flush the switchdev deferred queue after closing the ports so the bridge's deferred STP DISABLED transitions reach the firmware while it is still running instead of failing with -EBUSY against the host block
- treat -ENODEV as successful deletion in port_mdb_del() so the post-update teardown does not leave leftover host MDB entries behind for the DSA core to report

v3:
- validate the image, including both CRCs, before closing any ports so a malformed file no longer triggers a flash and reprobe cycle
- reject images whose declared payload sizes overflow when summed (check_add_overflow) or sum up to zero; the latter used to erase the flash without writing anything back
- allocate the reprobe work item and take the module and device references before disturbing the switch instead of silently skipping the reprobe when the allocation fails afterwards
- check block_host/skip_teardown under the MDIO bus lock to close the window where a command already past the check could reach the bus after the switch rebooted into MCUboot
- prevent the stats poll work from being re-armed and cancel the CRC error work before the transfer
- check the return value of SB PDI data word writes; control writes are verified by the subsequent status polls
- report a per-model chip name from the OF match data as "asic.id" instead of the devicetree compatible string whose comma is awkward for userspace consumers (Andrew Lunn)
- commit message: the conduit is only closed, not detached

v2:
- factor out SB PDI slice flush and devlink status notification helpers, resolving checkpatch issues
- use kzalloc_obj() (Manuel Ebner)
- add kernel-doc for the new mxl862xx_priv members
- trim comments and state the actual duration of a flash and reprobe cycle, just under a minute (Manuel Ebner)
- reword commit message: split up run-on sentence, explain the dynamically allocated reprobe work item (Manuel Ebner), mention that closing the ports stops phylib polling (Andrew Lunn)

 drivers/net/dsa/mxl862xx/Makefile        |   2 +-
 drivers/net/dsa/mxl862xx/mxl862xx-api.h  |  10 +
 drivers/net/dsa/mxl862xx/mxl862xx-cmd.h  |   2 +
 drivers/net/dsa/mxl862xx/mxl862xx-fw.c   | 572 +++++++++++++++++++++++
 drivers/net/dsa/mxl862xx/mxl862xx-fw.h   |  17 +
 drivers/net/dsa/mxl862xx/mxl862xx-host.c |  11 +
 drivers/net/dsa/mxl862xx/mxl862xx.c      |  49 ++
 drivers/net/dsa/mxl862xx/mxl862xx.h      |  12 +
 8 files changed, 674 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-fw.c
 create mode 100644 drivers/net/dsa/mxl862xx/mxl862xx-fw.h
drivers/net/dsa/mxl862xx/Makefile +1 -1
index a7be0e6669df..bccac0d0f703 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_NET_DSA_MXL862) += mxl862xx_dsa.o
-mxl862xx_dsa-y := mxl862xx.o mxl862xx-host.o mxl862xx-phylink.o
+mxl862xx_dsa-y := mxl862xx.o mxl862xx-host.o mxl862xx-phylink.o mxl862xx-fw.o
drivers/net/dsa/mxl862xx/mxl862xx-api.h +10 -0
index a180a5decffc..6f771895984c 100644
@@ -1224,6 +1224,16 @@ struct mxl862xx_sys_fw_image_version {
 	__le32 iv_build_num;
 } __packed;
 
+/**
+ * struct mxl862xx_sys_reg_rw - System register read/write
+ * @addr: 32-bit register address
+ * @val: register value
+ */
+struct mxl862xx_sys_reg_rw {
+	__le32 addr;
+	__le32 val;
+} __packed;
+
 /**
  * enum mxl862xx_port_type - Port Type
  * @MXL862XX_LOGICAL_PORT: Logical Port
drivers/net/dsa/mxl862xx/mxl862xx-cmd.h +2 -0
index c87a955c13c4..a865425aa61e 100644
@@ -70,7 +70,9 @@
 #define INT_GPHY_READ			(GPY_GPY2XX_MAGIC + 0x1)
 #define INT_GPHY_WRITE			(GPY_GPY2XX_MAGIC + 0x2)
 
+#define SYS_MISC_FW_UPDATE		(SYS_MISC_MAGIC + 0x1)
 #define SYS_MISC_FW_VERSION		(SYS_MISC_MAGIC + 0x2)
+#define SYS_MISC_REG_RD			(SYS_MISC_MAGIC + 0x8)
 
 #define MXL862XX_XPCS_PCS_CONFIG	(MXL862XX_XPCS_MAGIC + 0x1)
 #define MXL862XX_XPCS_PCS_GET_STATE	(MXL862XX_XPCS_MAGIC + 0x2)
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +572 -0

572 lines not on this page · raw message

drivers/net/dsa/mxl862xx/mxl862xx-fw.h new file +17 -0
new file mode 100644
index 000000000000..e96db19b2888
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MXL862XX_FW_H
+#define __MXL862XX_FW_H
+
+#include <net/dsa.h>
+
+struct mxl862xx_priv;
+
+int mxl862xx_devlink_info_get(struct dsa_switch *ds,
+			      struct devlink_info_req *req,
+			      struct netlink_ext_ack *extack);
+int mxl862xx_devlink_flash_update(struct dsa_switch *ds,
+				  struct devlink_flash_update_params *params,
+				  struct netlink_ext_ack *extack);
+
+#endif /* __MXL862XX_FW_H */
drivers/net/dsa/mxl862xx/mxl862xx-host.c +11 -0
index 6e582caea1fa..66b388eed0ce 100644
@@ -15,6 +15,7 @@
 #include <linux/unaligned.h>
 #include <net/dsa.h>
 #include "mxl862xx.h"
+#include "mxl862xx-cmd.h"
 #include "mxl862xx-host.h"
 
 #define CTRL_BUSY_MASK			BIT(15)
@@ -340,6 +341,16 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
 
 	mutex_lock_nested(&priv->mdiodev->bus->mdio_lock, MDIO_MUTEX_NESTED);
 
+	if (priv->skip_teardown) {
+		ret = read ? -ENODEV : 0;
+		goto out;
+	}
+
+	if (priv->block_host && cmd != SYS_MISC_FW_UPDATE) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	max = (size + 1) / 2;
 
 	ret = mxl862xx_busy_wait(priv);
drivers/net/dsa/mxl862xx/mxl862xx.c +49 -0

85 lines not on this page · raw message

drivers/net/dsa/mxl862xx/mxl862xx.h +12 -0
index 432a5f3f2e08..39535cd58706 100644
@@ -303,6 +303,10 @@ struct mxl862xx_fw_version {
  *                      flooding)
  * @fw_version:         cached firmware version, populated at probe and
  *                      compared with MXL862XX_FW_VER_MIN()
+ * @asic_id:            chip part number read from the CHIP ID registers,
+ *                      reported as the devlink "asic.id" fixed version
+ * @asic_rev:           chip version read from the CHIP ID registers,
+ *                      reported as the devlink "asic.rev" fixed version
  * @serdes_ports:       SerDes interfaces incl. sub-interfaces in case of
  *                      10G_QXGMII or QSGMII
  * @serdes_refcount:    per-XPCS count of sub-ports enabled by phylink;
@@ -319,6 +323,10 @@ struct mxl862xx_fw_version {
  * @evlan_ingress_size: per-port ingress Extended VLAN block size
  * @evlan_egress_size:  per-port egress Extended VLAN block size
  * @vf_block_size:      per-port VLAN Filter block size
+ * @block_host:         reject firmware API commands (except FW_UPDATE)
+ *                      during a firmware flash
+ * @skip_teardown:      discard firmware API commands during the teardown
+ *                      triggered by the post-flash reprobe
  * @stats_work:         periodic work item that polls RMON hardware counters
  *                      and accumulates them into 64-bit per-port stats
  */
@@ -329,6 +337,8 @@ struct mxl862xx_priv {
 	unsigned long flags;
 	u16 drop_meter;
 	struct mxl862xx_fw_version fw_version;
+	u16 asic_id;
+	u8 asic_rev;
 	struct mxl862xx_pcs serdes_ports[8];
 	int serdes_refcount[2];
 	struct mutex serdes_lock;
@@ -337,6 +347,8 @@ struct mxl862xx_priv {
 	u16 evlan_ingress_size;
 	u16 evlan_egress_size;
 	u16 vf_block_size;
+	bool block_host;
+	bool skip_teardown;
 	struct delayed_work stats_work;
 };
 
Signature · 1 line
2.55.0
maintainer
#7 lore raw

Re: [PATCH net-next v5 3/5] net: dsa: mxl862xx: add devlink flash_update and info_get

Quoted 11 lines

The switch leaves MCUboot on its own by booting the new image, but the driver has no in-place path back, so it reinitialises with a full device_reprobe() scheduled regardless of the transfer outcome -- after a failure the switch is still in MCUboot and probe re-detects it. During the teardown its API reads return -ENODEV and writes fake success, so it neither stalls on the absent firmware nor consumes buffers it never filled. The reprobe holds module and device references taken before the switch was disturbed and runs from a heap work item, not a kernel
thread: remove() frees the devm-managed priv, and kthread_create() in the devlink caller would return -EINTR on interrupt or trip the hung-task watchdog while parked across the minute-long flash.

Just for my understanding....

The reprobe causes all the user point netdevs to be destroyed, and are then recreated?

That seems like a good solution to the issue of all state information has been lost in the switch.

Quoted 6 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +6 -0
+static int mxl862xx_sb_pdi_poll_stat(struct mxl862xx_priv *priv, u16 expected,
+				     unsigned long timeout_ms)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	int ret;
+

drivers/net/dsa/mxl862xx/mxl862xx-fw.c line 147

Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +10 -0
+	do {
+		ret = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_STAT);
+		if (ret < 0)
+			return ret;
+		if ((u16)ret == expected)
+			return 0;
+		usleep_range(10000, 11000);
+	} while (time_before(jiffies, timeout));
+
+	return -ETIMEDOUT;

I suggest using iopoll.h. This code has the usual bug when rolling your own.

Quoted 3 lines

+/* Post-flash reprobe. Runs from a self-contained heap work (not a kthread and
+ * not the devlink caller's context): kthread_create() in the caller context
+ * fails -EINTR if the devlink command was Ctrl-C'd, and schedule_work() cannot.

A English sentence should not finish with the word "cannot".

drivers/net/dsa/mxl862xx/mxl862xx-fw.c line 296

Quoted 9 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +9 -0
+	/* Failures from here on must go through end_magic so MCUboot
+	 * reboots instead of waiting forever.
+	 */
+	ret = mxl862xx_sb_pdi_poll_stat(priv, MXL862XX_SB_PDI_READY,
+					MXL862XX_FW_READY_TIMEOUT_MS);
+	if (ret) {
+		dev_err(&priv->mdiodev->dev,
+			"flash: bootloader not ready: %pe\n", ERR_PTR(ret));
+		return ret;

What about the comment above? end_magic?

Andrew

author maintainer
#8 lore raw

Re: [PATCH net-next v5 3/5] net: dsa: mxl862xx: add devlink flash_update and info_get

Quoted 4 lines · Andrew Lunn
Quoted 4 lines

The switch leaves MCUboot on its own by booting the new image, but the driver has no in-place path back, so it reinitialises with a full device_reprobe() scheduled regardless of the transfer outcome -- after a failure the switch is still in MCUboot and probe re-detects it. During

[...]

Quoted 8 lines

Just for my understanding....

The reprobe causes all the user point netdevs to be destroyed, and are then recreated?

That seems like a good solution to the issue of all state information has been lost in the switch.

Yes, exactly. device_reprobe() runs remove(), which unregisters the switch and with it all user netdevs, and the fresh probe registers them anew. They come back administratively down and userspace brings them up again. Everything is rebuilt from scratch rather than restored, which is the point given the switch itself lost all state too.

Quoted 1 line
Quoted 1 line

+static int mxl862xx_sb_pdi_poll_stat(struct mxl862xx_priv *priv, u16 expected,

[...]

Quoted 7 lines

drivers/net/dsa/mxl862xx/mxl862xx-fw.c line 147

Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +4 -0
+		usleep_range(10000, 11000);
+	} while (time_before(jiffies, timeout));
+
+	return -ETIMEDOUT;

I suggest using iopoll.h. This code has the usual bug when rolling your own.

Agreed. Since v6 all SB PDI polling uses read_poll_timeout(), which also does the one more read after the deadline that the open-coded loop missed.

Quoted 5 lines
Quoted 3 lines

+/* Post-flash reprobe. Runs from a self-contained heap work (not a kthread and
+ * not the devlink caller's context): kthread_create() in the caller context
+ * fails -EINTR if the devlink command was Ctrl-C'd, and schedule_work() cannot.

A English sentence should not finish with the word "cannot".

That comment, and the kthread it talked about, are gone since v7; the reprobe is a plain delayed work item now.

Quoted 6 lines
Quoted 6 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c new file +6 -0
+	/* Failures from here on must go through end_magic so MCUboot
+	 * reboots instead of waiting forever.
+	 */
+	ret = mxl862xx_sb_pdi_poll_stat(priv, MXL862XX_SB_PDI_READY,
+					MXL862XX_FW_READY_TIMEOUT_MS);
+	if (ret) {

[...]

Quoted 3 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+		return ret;

What about the comment above? end_magic?

You are right, in v5 the comment and the code disagreed. It has moved on since: it turned out that END written while the loader still sits in its receive loop is consumed as a byte count and can underflow the receive counter, so since v7 failures deliberately do not send END (the loader finalises on its own 2s timeout), and v8 renamed the label to no_end with a comment that now matches the code.

author maintainer
#9 lore raw

PATCH v5 4/5 net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode

In reply to Daniel Golle

A broken or interrupted firmware image, or the sticky rescue bit, keeps the switch in its MCUboot bootloader, which exposes only the clause-22 SMDIO download interface. The clause-45 MMD API never comes up, so probe would fail with -ETIMEDOUT and the only way back would be the switch's UART console, or a power cycle or out-of-band reset (not currently implemented).

Probe for the loader over SB PDI at setup, before any clause-45 access, since the C45 API floods the log with CRC errors when no firmware answers. The same probe tells three cases apart without touching C45: a running firmware (probe continues normally), a switch in MCUboot (enter rescue mode), and a switch that does not answer at all -- absent, unpowered, or misdescribed in the device tree (wrong address or bus, or a reset GPIO with inverted polarity) -- which fails probe cleanly with -ENODEV instead of a CRC-error storm.

In rescue mode the switch registers without user interfaces so devlink stays available: user ports fail port_setup with -ENODEV (the DSA core re-registers them as unused) while shared and CPU ports succeed, and the CPU port works on its fixed link with mac_select_pcs returning no PCS. Firmware API commands fail fast with -ENODEV and the port and STP callbacks become no-ops.

An interrupted download can leave the loader wedged mid-payload. It is drained back to a clean ready state by feeding the outstanding byte count, which can take minutes, so it runs from a background work item off the devlink flash path; until it finishes, devlink dev info reports no version and devlink dev flash returns -EBUSY.

The CHIP ID registers need a running firmware, so no asic.id/asic.rev is reported in rescue mode. Once the loader is ready, devlink dev info reports the null firmware version "0.0.0" as both running and stored:

  $ devlink dev info mdio_bus/mdio-bus:10
  mdio_bus/mdio-bus:10:
    driver mxl862xx
    versions:
        running:
          fw 0.0.0
        stored:
          fw 0.0.0

An operational switch never reports 0.0.0 (a released firmware's major is non-zero), so version-comparing tools like fwupd offer every release as an upgrade, recovering the switch through the regular flash flow, matched on the driver name. The flash skips the FW_UPDATE command since MCUboot is already waiting. A successful flash reboots the switch into the new firmware and the reprobe then brings the driver up against it normally; after a failed one the switch is still in MCUboot, rescue mode is detected again, and the user can retry.

---
v5:
- detect the switch state from the value MCUboot publishes in the SB PDI STAT register (loader ready, wedged download, or running firmware), confirming a live console loader with a register-read challenge, instead of trusting a bare SMDIO scratch write
- fail probe with -ENODEV over SB PDI when the switch does not respond at all -- absent, unpowered, or misdescribed in the device tree -- instead of letting the clause-45 API flood the log with CRC errors
- drain a wedged interrupted download back to a clean ready state from a background work item so the multi-minute recovery never holds the devlink instance lock, and refuse devlink dev info and flash until it is ready
- report the null firmware version as the stored version too, matching the running/stored reporting of the previous patch
- do not report asic.id/asic.rev in rescue mode as the CHIP ID registers are unreadable without firmware; recovery tools match on the driver name and the "0.0.0" version instead (follows the numeric asic.id change in the previous patch)
- move the devlink documentation into its own patch

v4:
- log a distinct diagnostic when rescue mode detection fails on an SMDIO bus error instead of silently treating it as "not in rescue mode"
- clear the rescue_mode flag under the MDIO bus lock, following the flag write locking in the previous patch

v3:
- report the canonical null version "0.0.0" instead of "mcuboot-rescue" so that version-comparing update tools like fwupd offer any available release as an upgrade for recovery
- check the rescue_mode flag under the MDIO bus lock, following the block_host/skip_teardown change in the previous patch

v2: new patch, allowing recovery from a failed or interrupted update
without having to use a special recovery OS image (Andrew Lunn)

 drivers/net/dsa/mxl862xx/mxl862xx-fw.c      | 334 +++++++++++++++++++-
 drivers/net/dsa/mxl862xx/mxl862xx-fw.h      |   3 +
 drivers/net/dsa/mxl862xx/mxl862xx-host.c    |   8 +
 drivers/net/dsa/mxl862xx/mxl862xx-phylink.c |   2 +
 drivers/net/dsa/mxl862xx/mxl862xx.c         |  67 +++-
 drivers/net/dsa/mxl862xx/mxl862xx.h         |  12 +
 6 files changed, 408 insertions(+), 18 deletions(-)
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +323 -11
index cde00e5583ee..d28ceb98cd56 100644
@@ -27,9 +27,11 @@
  *
  * STAT magics:
  *   READY  0xc55c   loader idle in the console loop        (this driver)
+ *   DL_RDY 0xc33c   loader idle in the flashless loop
  *   START  0xf48f   host   -> begin download session
  *   ACK    0xf490   loader -> START acknowledged (START + 1)
  *   END    0x3cc3   host   -> end of transfer / finalise
+ *   RDREG  0xe2c0   host   -> register-read command (| index), see below
  *
  * Console flash path (STAT=0xc55c) - mxl862xx_flash_firmware():
  *
@@ -63,6 +65,36 @@
  *   EXACTLY 0. A count larger than r_remain underflows the 32-bit counter and
  *   wedges the loader until a power cycle. Hence:
  *     - never send a slice/chunk count larger than what is outstanding;
+ *     - interrupted-download recovery feeds 1 byte at a time (see below).
+ *
+ * Interrupted-flash recovery (mxl862xx_rescue_drain):
+ *   A host that dies mid-payload leaves the loader spinning in the slice loop
+ *   holding STAT=0 (no magic). Feed single 1-byte chunks (one DATA word +
+ *   STAT=1) until r_remain reaches 0, then STAT=END; the loader finalises the
+ *   (now corrupt) image and re-arms READY for a clean reflash.
+ *
+ * Register-read challenge (non-destructive liveness proof):
+ *   DATA := 0x7c23 (marker); STAT := 0xe2c0|idx
+ *     -> loader returns a runtime word in DATA and re-arms STAT=0xc55c.
+ *   The reply source is loader BSS, not a chip id; used only to prove a live
+ *   mailbox in mxl862xx_rescue_mode_detect().
+ *
+ * The other STAT ready magic, 0xc33c, marks the loader's flashless
+ * chip-to-chip download mode (MxL86281S 16-port tier); this driver does not
+ * use it.
+ *
+ * Rescue lifecycle (devlink): probe runs mxl862xx_rescue_mode_detect(); a wedged
+ * loader is drained back to READY by a background self-heal (rescue_heal_work) so
+ * the multi-minute recovery never holds the devlink lock. devlink dev info
+ * exposes the fw version (the "flashable" signal) only once at READY;
+ * flash_update returns -EBUSY until then, and reprobes to WSP firmware on success.
+ *
+ * Notes:
+ *   - Chip id/revision (0xc0d28884/88) are NOT reachable on this channel; they
+ *     need the clause-45 MMD firmware mailbox, which is dead under MCUboot.
+ *     Rescue identity is by SB PDI behaviour only (mxl862xx_rescue_mode_detect).
+ *   - The SMDIO PHY address and the 0xe1xx offsets are OTP-configurable; derive
+ *     them from the DT binding, do not assume fixed values.
  */
 
 #include <linux/crc32.h>
@@ -94,8 +126,15 @@
 
 /* SB PDI handshake magic (published/consumed via STAT) */
 #define MXL862XX_SB_PDI_READY		0xc55c	/* loader idle, console loop */
+#define MXL862XX_SB_PDI_DL_READY	0xc33c	/* loader idle, flashless loop */
 #define MXL862XX_SB_PDI_START		0xf48f
 #define MXL862XX_SB_PDI_END		0x3cc3
+#define MXL862XX_SB_PDI_RDREG		0xe2c0	/* register-read cmd (| index) */
+#define MXL862XX_SB_PDI_RDREG_MARK	0x7c23	/* marker placed in DATA for RDREG */
+
+/* Behavioural presence probe: two distinct 16-bit latches on ADDR/DATA. */
+#define MXL862XX_SB_PDI_PROBE_A		0x5a5a
+#define MXL862XX_SB_PDI_PROBE_D		0xa5a5
 
 /* Firmware transfer geometry */
 #define MXL862XX_FW_HDR_SIZE		20
@@ -110,6 +149,7 @@
 #define MXL862XX_FW_WRITE_TIMEOUT_MS	120000
 #define MXL862XX_FW_REBOOT_DELAY_MS	5000
 #define MXL862XX_FW_REPROBE_DELAY_MS	500
+#define MXL862XX_RESCUE_READY_TIMEOUT_MS 1000
 
 static int mxl862xx_sb_pdi_reset(struct mxl862xx_priv *priv)
 {
@@ -170,6 +210,242 @@ static void mxl862xx_flash_notify(struct devlink *dl, const char *status,
 	devlink_flash_update_status_notify(dl, status, NULL, done, total);
 }
 
+/* Byte-count of each chunk fed to the loader during drain. It MUST be 1: the
+ * loader only lets us observe "counter == 0", never "counter < step", so any
+ * step > 1 can subtract past zero, underflow the 32-bit counter and wedge the
+ * loader for ~2^32 more bytes (a state only a power cycle clears). Stepping by
+ * 1 walks the counter through every value and is guaranteed to land on zero
+ * whatever its (possibly odd) start. A 1-byte chunk is a path the loader
+ * already handles: the normal transfer ends with a single trailing byte for
+ * odd-sized images (see Step 6).
+ */
+#define MXL862XX_DRAIN_CHUNK_BYTES	1
+
+/* Poll STAT while draining a stuck download: 0 means "feed the next chunk",
+ * READY means the loader left the receive loop and re-armed its command loop,
+ * anything else is a transient (the count being consumed) - BUSY past the
+ * window means the counter has hit zero and the loader is finalising.
+ */
+enum { MXL862XX_DRAIN_FEED, MXL862XX_DRAIN_READY, MXL862XX_DRAIN_BUSY };
+static int mxl862xx_sb_pdi_poll_drain(struct mxl862xx_priv *priv,
+				      unsigned long timeout_ms)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	int ret;
+
+	do {
+		ret = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_STAT);
+		if (ret < 0)
+			return ret;
+		if ((u16)ret == MXL862XX_SB_PDI_READY)
+			return MXL862XX_DRAIN_READY;
+		if ((u16)ret == 0)
+			return MXL862XX_DRAIN_FEED;
+		usleep_range(50, 150);
+	} while (time_before(jiffies, timeout));
+
+	return MXL862XX_DRAIN_BUSY;
+}
+
+/* Recover a switch whose SB PDI download was interrupted mid-transfer - the
+ * host died after MCUboot began erasing flash, whether it aborted mid erase or
+ * mid image-write, both end up in the same place: the payload receive loop.
+ * There the loader publishes STAT=0, waits for the host to write a byte-count
+ * to STAT, DMAs that many bytes and subtracts the count from a remaining-bytes
+ * counter, leaving the loop only when the counter reaches exactly zero. The
+ * image size died with the host, so we feed single-byte chunks (see
+ * MXL862XX_DRAIN_CHUNK_BYTES) to walk the counter to zero without underflow,
+ * then send END. The loader runs its (failed) checksum and either re-arms
+ * READY or, if a valid image survived in flash, boots it. Returns 0 when READY
+ * is reached (the caller can flash), 1 when the loader booted instead (the
+ * switch is recovered but the intended image was not written), <0 on error.
+ * Does NOT recover a counter already underflowed by an earlier oversized-chunk
+ * attempt - that needs a power cycle.
+ */
+static int mxl862xx_rescue_drain(struct mxl862xx_priv *priv)
+{
+	struct device *dev = &priv->mdiodev->dev;
+	/* Bound: twice the loader's 16 MiB image cap, one byte per chunk. */
+	u32 max_chunks = 2u * (16u << 20) / MXL862XX_DRAIN_CHUNK_BYTES;
+	u32 chunk = 0;
+	int ret;
+
+	dev_warn(dev, "flash: draining interrupted download\n");
+
+	while (chunk < max_chunks) {
+		/* Teardown can interrupt this minutes-long drain. */
+		if (test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags))
+			return -ECANCELED;
+
+		ret = mxl862xx_sb_pdi_poll_drain(priv, 2000);
+		if (ret < 0)
+			return ret;
+		if (ret == MXL862XX_DRAIN_READY)
+			return 0;
+		if (ret == MXL862XX_DRAIN_BUSY)
+			break;
+
+		/* Feed one zero byte; reset cleared the write latch. */
+		ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_CTRL,
+					   MXL862XX_SB_PDI_CTRL_WR);
+		if (ret < 0)
+			return ret;
+		ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_DATA, 0x0000);
+		if (ret < 0)
+			return ret;
+		ret = mxl862xx_sb_pdi_reset(priv);
+		if (ret < 0)
+			return ret;
+		ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_STAT,
+					   MXL862XX_DRAIN_CHUNK_BYTES);
+		if (ret < 0)
+			return ret;
+		chunk++;
+		cond_resched();
+	}
+
+	if (chunk >= max_chunks) {
+		dev_err(dev,
+			"flash: interrupted download did not drain after %u chunks\n",
+			chunk);
+		return -ETIMEDOUT;
+	}
+
+	ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_STAT,
+				   MXL862XX_SB_PDI_END);
+	if (ret < 0)
+		return ret;
+
+	return mxl862xx_sb_pdi_poll_stat(priv, MXL862XX_SB_PDI_READY,
+					 MXL862XX_FW_ACK_TIMEOUT_MS);
+}
+
+/* Background self-heal: drain a wedged download back to READY off the devlink
+ * flash path, so the minutes-long recovery never holds the devlink lock.
+ * Scheduled from probe; publishes rescue_ready on success.
+ */
+void mxl862xx_rescue_heal_work_fn(struct work_struct *work)
+{
+	struct mxl862xx_priv *priv =
+		container_of(work, struct mxl862xx_priv, rescue_heal_work);
+	struct device *dev = &priv->mdiodev->dev;
+	int ret;
+
+	dev_info(dev, "recovering interrupted download in background\n");
+	ret = mxl862xx_rescue_drain(priv);
+	if (test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags))
+		return;
+	if (!ret) {
+		WRITE_ONCE(priv->rescue_ready, true);
+		dev_info(dev, "recovery complete, ready to flash\n");
+	}
+}
+
+/* Detect MCUboot rescue mode over clause-22 SMDIO alone, so the caller can rule
+ * the loader out before any C45 API request (which spews CRC errors when no WSP
+ * firmware answers). A scratch write to ADDR/DATA must latch or the chip is
+ * absent (-ENODEV); STAT then classifies the state, poked destructively only
+ * when 0, the one value a running firmware never holds:
+ *
+ *  - 0xc33c: flashless loop, ready.
+ *  - 0xc55c: console loop, if the register-read challenge is serviced.
+ *  - other non-zero: running firmware, left unpoked.
+ *  - 0: wedged receive loop, if a 1-byte slice-advance drains back to 0.
+ *
+ * Return: MXL862XX_IN_RESCUE, MXL862XX_NOT_RESCUE, or negative (-ENODEV/SMDIO).
+ */
+int mxl862xx_rescue_mode_detect(struct mxl862xx_priv *priv)
+{
+	int stat, dat, ret, rb, a, d;
+
+	/* rescue_ready gates flashing; a wedged loader needs the drain first. */
+	priv->rescue_ready = false;
+
+	/* Presence: a live chip latches the scratch write, an absent one floats. */
+	a = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_ADDR,
+				 MXL862XX_SB_PDI_PROBE_A);
+	if (a < 0)
+		return a;
+	d = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_DATA,
+				 MXL862XX_SB_PDI_PROBE_D);
+	if (d < 0)
+		return d;
+	a = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_ADDR);
+	if (a < 0)
+		return a;
+	d = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_DATA);
+	if (d < 0)
+		return d;
+	if ((u16)a != MXL862XX_SB_PDI_PROBE_A ||
+	    (u16)d != MXL862XX_SB_PDI_PROBE_D)
+		return -ENODEV;
+
+	ret = mxl862xx_sb_pdi_reset(priv);
+	if (ret < 0)
+		return ret;
+
+	stat = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_STAT);
+	if (stat < 0)
+		return stat;
+
+	/* Flashless-download loop: unambiguous ready magic, leave it alone. */
+	if ((u16)stat == MXL862XX_SB_PDI_DL_READY) {
+		priv->rescue_ready = true;
+		return MXL862XX_IN_RESCUE;
+	}
+
+	/* Console loop at READY: confirm the live mailbox with the register-read
+	 * challenge (consumes the marker from DATA and re-arms READY).
+	 */
+	if ((u16)stat == MXL862XX_SB_PDI_READY) {
+		ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_DATA,
+					   MXL862XX_SB_PDI_RDREG_MARK);
+		if (ret < 0)
+			return ret;
+		ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_STAT,
+					   MXL862XX_SB_PDI_RDREG);
+		if (ret < 0)
+			return ret;
+		rb = mxl862xx_sb_pdi_poll_stat(priv, MXL862XX_SB_PDI_READY,
+					       MXL862XX_RESCUE_READY_TIMEOUT_MS);
+		dat = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_DATA);
+		if (dat < 0)
+			return dat;
+		mxl862xx_sb_pdi_reset(priv);
+		if (!rb && (u16)dat != MXL862XX_SB_PDI_RDREG_MARK) {
+			priv->rescue_ready = true;
+			return MXL862XX_IN_RESCUE;
+		}
+		return -ENODEV;
+	}
+
+	/* Any other non-zero value is a running firmware, not a loader. */
+	if (stat)
+		return MXL862XX_NOT_RESCUE;
+
+	/* STAT == 0: a wedged receive loop consumes a 1-byte slice-advance back
+	 * to 0 (feed one DATA word first, like a drain chunk).
+	 */
+	ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_CTRL,
+				   MXL862XX_SB_PDI_CTRL_WR);
+	if (ret < 0)
+		return ret;
+	ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_DATA, 0x0000);
+	if (ret < 0)
+		return ret;
+	ret = mxl862xx_sb_pdi_reset(priv);
+	if (ret < 0)
+		return ret;
+	ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_STAT, 1);
+	if (ret < 0)
+		return ret;
+	rb = mxl862xx_sb_pdi_poll_stat(priv, 0, MXL862XX_RESCUE_READY_TIMEOUT_MS);
+	if (!rb)
+		return MXL862XX_IN_RESCUE;
+
+	return -ENODEV;
+}
+
 /* Post-flash reprobe. Runs from a self-contained heap work (not a kthread and
  * not the devlink caller's context): kthread_create() in the caller context
  * fails -EINTR if the devlink command was Ctrl-C'd, and schedule_work() cannot.
@@ -270,13 +546,15 @@ static int mxl862xx_flash_firmware(struct mxl862xx_priv *priv,
 	int ret, i;
 
 	/* Step 1: reboot the firmware into MCUboot rescue mode */
-	ret = mxl862xx_api_wrap(priv, SYS_MISC_FW_UPDATE, NULL, 0,
-				false, false);
-	if (ret) {
-		dev_err(&priv->mdiodev->dev,
-			"flash: FW_UPDATE command failed: %pe\n",
-			ERR_PTR(ret));
-		return ret;
+	if (!priv->rescue_mode) {
+		ret = mxl862xx_api_wrap(priv, SYS_MISC_FW_UPDATE, NULL, 0,
+					false, false);
+		if (ret) {
+			dev_err(&priv->mdiodev->dev,
+				"flash: FW_UPDATE command failed: %pe\n",
+				ERR_PTR(ret));
+			return ret;
+		}
 	}
 
 	/* Step 2: wait for bootloader ready */
@@ -449,6 +727,23 @@ int mxl862xx_devlink_info_get(struct dsa_switch *ds,
 	char buf[16];
 	int ret;
 
+	/* No chip-id/revision in MCUboot (needs the firmware MMD mailbox). The
+	 * fw version doubles as the "ready to flash" signal: report it only
+	 * once the loader is at a clean READY, nothing while still draining.
+	 */
+	if (priv->rescue_mode) {
+		if (!READ_ONCE(priv->rescue_ready))
+			return 0;
+
+		snprintf(buf, sizeof(buf), "%u.%u.%u",
+			 priv->fw_version.major, priv->fw_version.minor,
+			 priv->fw_version.revision);
+		ret = devlink_info_version_running_put(req, "fw", buf);
+		if (ret)
+			return ret;
+		return devlink_info_version_stored_put(req, "fw", buf);
+	}
+
 	snprintf(buf, sizeof(buf), "%04X", priv->asic_id);
 	ret = devlink_info_version_fixed_put(req,
 					     DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
@@ -496,6 +791,13 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,
 		return ret;
 	}
 
+	/* Refuse to flash while the background self-heal is still draining. */
+	if (priv->rescue_mode && !READ_ONCE(priv->rescue_ready)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "switch is recovering an interrupted download, retry shortly");
+		return -EBUSY;
+	}
+
 	/* The references the reprobe work needs to restore normal operation
 	 * must be held before the switch is disturbed; the work itself is
 	 * scheduled only once the flash is done (see below).
@@ -505,9 +807,13 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,
 
 	get_device(ds->dev);
 
-	dev_info(ds->dev, "flash: running firmware %u.%u.%u\n",
-		 priv->fw_version.major, priv->fw_version.minor,
-		 priv->fw_version.revision);
+	if (priv->rescue_mode)
+		dev_info(ds->dev,
+			 "flash: flashing switch via MCUboot rescue mode\n");
+	else
+		dev_info(ds->dev, "flash: running firmware %u.%u.%u\n",
+			 priv->fw_version.major, priv->fw_version.minor,
+			 priv->fw_version.revision);
 
 	/* Close ports while the firmware is still alive so the DSA
 	 * core's MDB/FDB tracking is drained, and detach user ports
@@ -544,7 +850,13 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,
 	if (ret)
 		NL_SET_ERR_MSG_MOD(extack, "firmware transfer failed");
 
-	/* The reprobe below re-reads and logs the new version. */
+	if (!ret) {
+		mutex_lock_nested(&priv->mdiodev->bus->mdio_lock,
+				  MDIO_MUTEX_NESTED);
+		priv->rescue_mode = false;
+		mutex_unlock(&priv->mdiodev->bus->mdio_lock);
+		/* The reprobe below re-reads and logs the new version. */
+	}
 
 	mutex_lock_nested(&priv->mdiodev->bus->mdio_lock, MDIO_MUTEX_NESTED);
 	priv->skip_teardown = true;
drivers/net/dsa/mxl862xx/mxl862xx-fw.h +3 -0
index e96db19b2888..7cd87c7ad871 100644
@@ -6,7 +6,10 @@
 #include <net/dsa.h>
 
 struct mxl862xx_priv;
+struct work_struct;
 
+int mxl862xx_rescue_mode_detect(struct mxl862xx_priv *priv);
+void mxl862xx_rescue_heal_work_fn(struct work_struct *work);
 int mxl862xx_devlink_info_get(struct dsa_switch *ds,
 			      struct devlink_info_req *req,
 			      struct netlink_ext_ack *extack);
drivers/net/dsa/mxl862xx/mxl862xx-host.c +8 -0
index 66b388eed0ce..2dbd074c0fe2 100644
@@ -16,6 +16,7 @@
 #include <net/dsa.h>
 #include "mxl862xx.h"
 #include "mxl862xx-cmd.h"
+#include "mxl862xx-fw.h"
 #include "mxl862xx-host.h"
 
 #define CTRL_BUSY_MASK			BIT(15)
@@ -346,6 +347,11 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
 		goto out;
 	}
 
+	if (priv->rescue_mode) {
+		ret = -ENODEV;
+		goto out;
+	}
+
 	if (priv->block_host && cmd != SYS_MISC_FW_UPDATE) {
 		ret = -EBUSY;
 		goto out;
@@ -544,9 +550,11 @@ int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val)
 void mxl862xx_host_init(struct mxl862xx_priv *priv)
 {
 	INIT_WORK(&priv->crc_err_work, mxl862xx_crc_err_work_fn);
+	INIT_WORK(&priv->rescue_heal_work, mxl862xx_rescue_heal_work_fn);
 }
 
 void mxl862xx_host_shutdown(struct mxl862xx_priv *priv)
 {
 	cancel_work_sync(&priv->crc_err_work);
+	cancel_work_sync(&priv->rescue_heal_work);
 }
drivers/net/dsa/mxl862xx/mxl862xx-phylink.c +2 -0
index b689652aa9b9..a5b6940b552e 100644
@@ -406,6 +406,8 @@ mxl862xx_phylink_mac_select_pcs(struct phylink_config *config,
 
 	switch (port) {
 	case 9 ... 16:
+		if (priv->rescue_mode)
+			return NULL;
 		if (!MXL862XX_FW_VER_MIN(priv, 1, 0, 84)) {
 			dev_warn_once(dp->ds->dev,
 				      "SerDes PCS unsupported on old firmware.\n");
drivers/net/dsa/mxl862xx/mxl862xx.c +60 -7

98 lines not on this page · raw message

drivers/net/dsa/mxl862xx/mxl862xx.h +12 -0
index 39535cd58706..72dc31cc2b62 100644
@@ -14,6 +14,10 @@ struct mxl862xx_priv;
 #define MXL862XX_FIRST_SERDES_PORT	9
 #define MXL862XX_SERDES_SLOTS		4
 
+/* mxl862xx_rescue_mode_detect() return codes (negative values are errors) */
+#define MXL862XX_NOT_RESCUE		0
+#define MXL862XX_IN_RESCUE		1
+
 #define MXL862XX_DEFAULT_BRIDGE		0
 #define MXL862XX_MAX_BRIDGES		48
 #define MXL862XX_MAX_BRIDGE_PORTS	128
@@ -327,6 +331,11 @@ struct mxl862xx_fw_version {
  *                      during a firmware flash
  * @skip_teardown:      discard firmware API commands during the teardown
  *                      triggered by the post-flash reprobe
+ * @rescue_mode:        switch is in MCUboot; firmware API commands fail fast,
+ *                      only clause-22 SMDIO works
+ * @rescue_ready:       (rescue_mode) loader is at a clean READY and will accept
+ *                      a flash; false while rescue_heal_work is draining
+ * @rescue_heal_work:   background self-heal draining a wedged download to READY
  * @stats_work:         periodic work item that polls RMON hardware counters
  *                      and accumulates them into 64-bit per-port stats
  */
@@ -334,6 +343,7 @@ struct mxl862xx_priv {
 	struct dsa_switch *ds;
 	struct mdio_device *mdiodev;
 	struct work_struct crc_err_work;
+	struct work_struct rescue_heal_work;
 	unsigned long flags;
 	u16 drop_meter;
 	struct mxl862xx_fw_version fw_version;
@@ -349,6 +359,8 @@ struct mxl862xx_priv {
 	u16 vf_block_size;
 	bool block_host;
 	bool skip_teardown;
+	bool rescue_mode;
+	bool rescue_ready;
 	struct delayed_work stats_work;
 };
 
Signature · 1 line
2.55.0
maintainer
#10 lore raw

Re: [PATCH net-next v5 4/5] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode

Quoted 6 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +6 -0
+static int mxl862xx_sb_pdi_poll_drain(struct mxl862xx_priv *priv,
+				      unsigned long timeout_ms)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+	int ret;
+

drivers/net/dsa/mxl862xx/mxl862xx-fw.c static void mxl862xx_flash_notify(struct devlink *dl, const char *status, · line 245

Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +10 -0
+	do {
+		ret = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_STAT);
+		if (ret < 0)
+			return ret;
+		if ((u16)ret == MXL862XX_SB_PDI_READY)
+			return MXL862XX_DRAIN_READY;
+		if ((u16)ret == 0)
+			return MXL862XX_DRAIN_FEED;
+		usleep_range(50, 150);
+	} while (time_before(jiffies, timeout));

Again, iopoll.h should be better.

Andrew

author maintainer
#11 lore raw

PATCH v5 5/5 net: dsa: mxl862xx: document devlink flash and info support

In reply to Daniel Golle

Describe the devlink info versions and the flash update behaviour, including the MCUboot rescue mode recovery, in a dedicated file under Documentation/networking/devlink/ and link it from the index. Add the new file to the driver's MAINTAINERS entry.

---
v5: new patch

 Documentation/networking/devlink/index.rst    |  1 +
 Documentation/networking/devlink/mxl862xx.rst | 63 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 3 files changed, 65 insertions(+)
 create mode 100644 Documentation/networking/devlink/mxl862xx.rst
Documentation/networking/devlink/index.rst +1 -0
index 4745148fecf4..058999d0dc56 100644
@@ -94,6 +94,7 @@ parameters, info versions, and other features it supports.
    mlx5
    mlxsw
    mv88e6xxx
+   mxl862xx
    netdevsim
    nfp
    octeontx2
Documentation/networking/devlink/mxl862xx.rst new file +63 -0

63 lines not on this page · raw message

MAINTAINERS +1 -0
index 60cff00953dc..3a55ed2bad9c 100644
@@ -16107,6 +16107,7 @@ M:	Daniel Golle <[email protected]>
 L:	[email protected]
 S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/maxlinear,mxl862xx.yaml
+F:	Documentation/networking/devlink/mxl862xx.rst
 F:	drivers/net/dsa/mxl862xx/
 F:	net/dsa/tag_mxl862xx.c
 
Signature · 1 line
2.55.0
maintainer Reviewed-by on v5 5/5
#12 lore raw

Re: [PATCH net-next v5 5/5] net: dsa: mxl862xx: document devlink flash and info support

Quoted 6 lines · Daniel Golle

Describe the devlink info versions and the flash update behaviour, including the MCUboot rescue mode recovery, in a dedicated file under Documentation/networking/devlink/ and link it from the index. Add the new file to the driver's MAINTAINERS entry.

Andrew