net: dsa: mxl862xx: support firmware update
Subsystems (6)
Daniel Golle
maintainerv9 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 v8 [8]:
- install the flash_update devlink op unconditionally and return -EOPNOTSUPP from the trampoline for drivers without the callback, instead of a second devlink_ops permutation (Andrew Lunn)
- picked up Andrew's Reviewed-by on patches 2 and 5, given on v5
Changes since v7 [7]:
- most of the changes below address findings of the Sashiko AI reviews of v7
- the MCUboot loader's transfer completion was reverse-engineered to settle two of them: it publishes an image verification verdict in its status register, finalises without the END magic once a 2 s timeout expires, and keeps the host's byte count visible while it programs a chunk. The interrupted-download drain therefore no longer sends END, which a loader still in its receive loop consumes as a byte count and underflows its receive counter on, and the flash path now reports a rejected image instead of a write timeout
- refuse a second devlink dev flash while the previous one's reprobe is still pending, and stop publishing a zeroed firmware version after a failed transfer
- initialise the SerDes state before the rescue-mode early return, so a successful rescue-mode flash cannot hand phylink an unconfigured PCS
- do not fail probe from the wedged-download branch of the rescue detection, which a download interrupted with exactly one byte outstanding would trigger, and report a failed recovery through devlink rather than refusing every flash for good
- reset the SB PDI mailbox before probing it, log the drain's progress, and correct the protocol and register comments throughout
Changes since v6 [6]:
- reprobe from a single delayed work item instead of a kthread spawned by a workqueue kickoff; the kthread existed only to drop the module reference from core code, but its creation-failure path did the racy module_put() from module text anyway and could strand the driver bound with skip_teardown set. The collapsed form matches iwl_trans_reprobe_wk(), and a failed reprobe now leaves the device unbound like a failed probe
- only signal END on a successful transfer; a failure leaves the loader mid-payload, where END is read as a byte count and can underflow the receive counter, so return the error and let the reprobe recover
- add cond_resched() to the payload loop so a long transfer over a bit-banged MDIO bus under CONFIG_PREEMPT_NONE does not trip the soft-lockup detector
- drop the cached firmware version and chip id on a failed flash so devlink dev info stops reporting the pre-flash version until the reprobe
- report the firmware version under DEVLINK_INFO_VERSION_GENERIC_FW instead of a bare "fw" string
- correct the SB PDI header comment's SMDIO register map and expand the note on why closing the shared conduit is safe
Changes since v5 [5]:
- run the post-flash reprobe from a kthread that drops the module reference with module_put_and_kthread_exit() from core code, fixing a use-after-free where a work item's trailing module_put() could return into module text a racing rmmod had freed; a workqueue kickoff spawns the kthread off the devlink caller where kthread_create() can return -EINTR
- send END on every flash failure from the ready handshake onward so an aborted transfer lets MCUboot reboot instead of leaving it waiting
- after the background drain finalises an interrupted download, reprobe and let the probe-time detection re-classify the switch, so a valid image a last-moment interruption left bootable comes up as running firmware; rescue_drain() no longer inspects or reports the outcome
- re-read the SB PDI status register once more after a poll timeout expires, so a preempted poll cannot report a spurious -ETIMEDOUT
- bail out of the periodic stats poll when the flash teardown has set WORK_STOPPED, closing a get_stats64() re-arm race
- allocate the reprobe kickoff before disturbing the switch, so an -ENOMEM cannot leave it flashed but never reprobed
- omit asic.id/asic.rev when the CHIP ID read returned 0, instead of publishing a bogus "0000" for fwupd to match firmware against
- treat the flashless-download loop (STAT 0xc33c) as an unsupported configuration and fail probe with -ENODEV instead of advertising it as flashable
Changes since v4 [4]:
- 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
- split the devlink documentation into its own patch and add 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 [3]:
- 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 [2]:
- 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]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/
[4] https://lore.kernel.org/all/[email protected]/
[5] https://lore.kernel.org/all/[email protected]/
[6] https://lore.kernel.org/all/[email protected]/
[7] https://lore.kernel.org/all/[email protected]/
[8] https://lore.kernel.org/all/[email protected]/
Daniel Golle (5):
net: dsa: add devlink flash_update callback to dsa_switch_ops
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 | 71 ++ 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 | 1102 +++++++++++++++++ drivers/net/dsa/mxl862xx/mxl862xx-fw.h | 20 + drivers/net/dsa/mxl862xx/mxl862xx-host.c | 60 + drivers/net/dsa/mxl862xx/mxl862xx-host.h | 2 + drivers/net/dsa/mxl862xx/mxl862xx-phylink.c | 2 + drivers/net/dsa/mxl862xx/mxl862xx.c | 166 ++- drivers/net/dsa/mxl862xx/mxl862xx.h | 48 + include/net/dsa.h | 3 + net/dsa/devlink.c | 13 + 15 files changed, 1484 insertions(+), 19 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: 69963a0678a347d57c4ac8b16939dba216eb95ce
prerequisite-patch-id: 0000000000000000000000000000000000000000
Signature · 1 line
2.55.0
Daniel Golle
maintainerPATCH v9 1/5 net: dsa: add devlink flash_update callback to dsa_switch_ops
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. Like the other trampolines in net/dsa/devlink.c, the op returns -EOPNOTSUPP when the driver does not implement the callback; the devlink core will then have fetched the firmware file from userspace before the request fails, which is acceptable for an operation as infrequent as a firmware update.
The devlink core calls the op with the devlink instance lock held and without rtnl_lock, whereas DSA serialises its switch and port ops under rtnl_lock, so a driver has to serialise a flash against its own ops itself.
---
v9: install the flash_update op unconditionally and return -EOPNOTSUPP
from the trampoline like the other DSA devlink trampolines, instead of a second devlink_ops permutation (Andrew Lunn)
v8:
- retitled: this patch adds the callback, its first user is patch 3
- describe the op's calling context in the commit message
v7: no changes
v6: no changes
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 | 13 +++++++++++++ 2 files changed, 16 insertions(+)
include/net/dsa.h +3 -0
@@ -1172,6 +1172,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 +13 -0
@@ -20,6 +20,18 @@ static int dsa_devlink_info_get(struct devlink *dl,return -EOPNOTSUPP ; } +static int dsa_devlink_flash_update (struct devlink *dl, +struct devlink_flash_update_params *params, +struct netlink_ext_ack *extack) +{ +struct dsa_switch *ds =dsa_devlink_to_ds (dl); + +if (!ds->ops->devlink_flash_update) +return -EOPNOTSUPP ; + +return ds->ops->devlink_flash_update (ds, params, extack); +} +static int dsa_devlink_sb_pool_get (struct devlink *dl,unsigned int sb_index,u16 pool_index,struct devlink_sb_pool_info *pool_info)
@@ -169,6 +181,7 @@ dsa_devlink_sb_occ_tc_port_bind_get(struct devlink_port *dlp,static const struct devlink_ops dsa_devlink_ops = { .info_get = dsa_devlink_info_get, + .flash_update = dsa_devlink_flash_update, .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,
Signature · 1 line
2.55.0
Daniel Golle
maintainerPATCH v9 2/5 net: dsa: mxl862xx: add SMDIO clause-22 register access
In reply to Daniel GolleAdd 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.
---
v9: no changes, picked up Andrew's v5 Reviewed-by
v8: document the paged register window and the per-transaction bus
locking next to the helpers, rather than only in this changelog (found by Sashiko AI review)
v7: no changes
v6: no changes
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 | 40 ++++++++++++++++++++++++ drivers/net/dsa/mxl862xx/mxl862xx-host.h | 2 ++ 2 files changed, 42 insertions(+)
drivers/net/dsa/mxl862xx/mxl862xx-host.c +40 -0
@@ -495,6 +495,46 @@ 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 + +/* Paged clause-22 window: the page goes into MII register 0x1f, the low nibble +* of addr selects one of the 16 registers within it. Both helpers take the MDIO +* bus lock per transaction, so callers must not already hold it -- unlike +* mxl862xx_api_wrap(), which holds it across a whole firmware command. +*/ +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
@@ -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
Daniel Golle
maintainerPATCH v9 3/5 net: dsa: mxl862xx: add devlink flash_update and info_get
In reply to Daniel GolleImplement 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.70The "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; they are omitted if the read fails or the part is unfused, so no bogus "0000" is published. 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 loader verifies the image once the last slice has been programmed and publishes the verdict, so a rejected image is reported as such rather than as a transfer timeout. 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. 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; port_mdb_del() takes that -ENODEV for a MAC table that is gone and reports success, since there is nothing left to delete. The reprobe holds module and device references taken before the switch was disturbed and runs from a work item, off the devlink caller's locking and signal context; it frees itself and drops both references when it finishes. A failed reprobe leaves the device unbound, exactly as a failed initial probe would, and a further flash is refused until it has run. An aborted transfer leaves the switch in MCUboot, where nothing here can reach it; the next patch adds the detection that makes such a switch flashable again.
The closed user ports and conduit are not returned to their pre-flash administrative state across the reprobe; userspace brings them back up, and restoring it in-driver would need DSA-core support that does not yet exist.
---
v9: no changes
v8:
- refuse a second devlink dev flash while the previous one's reprobe is still pending: the firmware API is short-circuited by then, so the FW_UPDATE command faked success and the raw SB PDI writes ran against a switch the driver no longer tracks (found by Sashiko AI review)
- omit the firmware version from devlink dev info while the cached copy is all-zero, so a failed transfer no longer publishes 0.0.0 as both running and stored, and clear asic_rev along with asic_id (found by Sashiko AI review)
- only translate -ENODEV into success in port_mdb_del() during the post-flash teardown; outside it, a genuine bus error was reported to switchdev as a successful deletion (found by Sashiko AI review)
- evaluate the image verification verdict the loader publishes after the last slice, instead of polling for a value it had already published, so a rejected image is no longer reported as a write timeout
- rename the end_magic label to no_end, which is what it does, and correct the protocol comment: END is optional as the loader finalises on a 2 s timeout, the status register keeps the byte count while a chunk is programmed, and the half-bank switch does not clear DATA (found by Sashiko AI review)
v7:
- reprobe from a single delayed work item instead of a kthread spawned by a workqueue kickoff; the kthread existed only to drop the module reference from core code, but its creation-failure path did the racy module_put() from module text anyway and could strand the driver bound with skip_teardown set. The collapsed form matches iwl_trans_reprobe_wk(), and a failed reprobe now leaves the device unbound like a failed probe
- only signal END on a successful transfer; a failure leaves the loader mid-payload, where a STAT write is a byte count and END (0x3cc3) is read as one, risking a receive-counter underflow, so return the error and let the reprobe recover
- add cond_resched() to the payload loop so a long transfer over a bit-banged MDIO bus under CONFIG_PREEMPT_NONE does not trip the soft-lockup detector
- drop the cached firmware version and chip id on a failed flash so devlink dev info stops reporting the pre-flash version until the reprobe re-reads it
- report the firmware version under DEVLINK_INFO_VERSION_GENERIC_FW instead of a bare "fw" string
- correct the SB PDI header comment's SMDIO register map (page in MII reg 0x1f, register from the low nibble) and expand the note on why closing the shared conduit is safe
v6:
- confirm the new firmware is running with mxl862xx_wait_ready() and lift the host block before reporting success, so devlink dev flash completes only once the update has taken effect instead of relying on the later reprobe to pick up the new version
- run the post-flash reprobe from a kthread that drops the module reference with module_put_and_kthread_exit(), spawned from a workqueue kickoff, closing a use-after-free where a work item's trailing module_put() could return into module text a racing rmmod had already freed
- jump to the end_magic teardown on every flash failure from the ready handshake onward, so an aborted transfer sends END and lets MCUboot reboot instead of leaving the loader waiting
- poll the SB PDI status register with read_poll_timeout(), which evaluates the condition once more after the deadline, so a preempted poll cannot report a spurious -ETIMEDOUT
- bail out of the periodic stats poll when the flash teardown has set WORK_STOPPED, closing a get_stats64() re-arm race
- allocate the reprobe kickoff before disturbing the switch, so an -ENOMEM cannot leave it flashed but never reprobed with block_host and skip_teardown stuck set
- omit asic.id/asic.rev when the CHIP ID read returned 0, instead of publishing a bogus "0000" for fwupd to match firmware against
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 | 698 +++++++++++++++++++++++ drivers/net/dsa/mxl862xx/mxl862xx-fw.h | 17 + drivers/net/dsa/mxl862xx/mxl862xx-host.c | 11 + drivers/net/dsa/mxl862xx/mxl862xx.c | 68 ++- drivers/net/dsa/mxl862xx/mxl862xx.h | 14 + 8 files changed, 820 insertions(+), 2 deletions(-) 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
@@ -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
@@ -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
@@ -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 +698 -0
698 lines not on this page · raw message
drivers/net/dsa/mxl862xx/mxl862xx-fw.h new file +17 -0
@@ -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
@@ -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 +67 -1
110 lines not on this page · raw message
drivers/net/dsa/mxl862xx/mxl862xx.h +14 -0
@@ -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,7 +347,11 @@ 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; }; +int mxl862xx_wait_ready (struct dsa_switch *ds); +#endif /* __MXL862XX_H */
Signature · 1 line
2.55.0
Daniel Golle
maintainerPATCH v9 4/5 net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
In reply to Daniel GolleA 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 status register the loader publishes tells the cases apart without touching C45: a running firmware, which answers with its own non-zero status word (0x0003 on the firmware tested) and is left untouched, so probe continues normally; a loader idle in its console loop, confirmed live by a register-read challenge; a loader wedged mid-download, the one case that publishes zero; the flashless download loop, which this driver cannot drive; 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). The first three enter rescue mode or normal operation, the last two fail probe cleanly with -EOPNOTSUPP and -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. A background work item off the devlink flash path drains it back to a clean ready state by feeding the outstanding byte count one byte at a time, which takes tens of minutes for a large image and is logged as it progresses; until then devlink dev info reports no version and devlink dev flash returns -EBUSY, and -EIO once a drain has failed for good. Reaching the end of the count is all the drain does: the loader then verifies the corrupt image and returns to its console loop on its own, so the drain reprobes and lets the probe-time detection re-classify the switch -- a valid image that a last-moment interruption left bootable comes up as running firmware, with no second-guessing in the drain path.
The reprobe the drain queues owns itself, so a drain that finishes just as the driver is unbound can still rebind the device afterwards; a mutex keeps that hand-off out of a teardown that has already begun, which is as far as a self-owned work item can be serialised.
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.0An 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.
---
v9: no changes
v8:
- never send END from the drain: the loader keeps the host's byte count in its status register while it programs a chunk, so a lingering count cannot be told from the "image rejected" verdict, and END written into the receive loop is consumed as a 15555-byte count and underflows the receive counter. Wait for the loader to ask for the next chunk or to return to its console loop instead, since it finalises on its own (found by Sashiko AI review)
- initialise the SerDes state before the rescue-mode early return, so the window between a successful rescue-mode flash clearing rescue_mode and the reprobe cannot hand phylink a PCS with no ops and an uninitialised mutex (found by Sashiko AI review)
- do not fail probe when the 1-byte slice-advance leaves the wedged loader somewhere other than asking for the next chunk: a download interrupted with exactly one byte outstanding completes on that byte, after which the loader verifies and returns to its console loop (found by Sashiko AI review)
- report a failed drain and refuse further flashes with -EIO and an extack asking for a power cycle, instead of leaving devlink to answer "retry shortly" forever for a switch that never becomes ready (found by Sashiko AI review)
- reset the mailbox before the presence probe: a download interrupted with the write latch armed made the scratch write land in switch memory instead, so detection returned -ENODEV for the very state it exists to recover (found by Sashiko AI review)
- tell an SMDIO bus error apart from a loader failing the register-read challenge, and check the reset issued after it (found by Sashiko AI review)
- reject DSA links before the rescue-mode shortcut, so an unsupported cascade topology fails probe in rescue mode too (found by Sashiko AI review)
- serialise the self-heal's reprobe hand-off against teardown with a mutex (found by Sashiko AI review)
- log the drain's progress, name its timeouts, and describe its real duration (found by Sashiko AI review)
- WRITE_ONCE() the rescue_ready stores, document what orders rescue_mode, and correct the detection kernel-doc and the note on the OTP-configurable SB PDI register offsets (found by Sashiko AI review)
v7:
- queue the reprobe as a delayed work item from the background self-heal, following the previous patch's move off the reprobe kthread
- report the rescue-mode firmware version under DEVLINK_INFO_VERSION_GENERIC_FW too
- drop two redundant rescue-recovery log lines; the setup message ("switch in MCUboot with an interrupted download, recovering in background") already says it
- return distinct errno from rescue_mode_detect() so an absent switch (-ENODEV), one strapped into flashless-download mode (-EOPNOTSUPP) and one that answers SB PDI READY but fails the register-read challenge, or wedges without draining (-ENXIO), are no longer all reported as -ENODEV
v6:
- after the background drain finalises the interrupted transfer, reprobe and let the probe-time detection re-classify the switch, so a valid image a last-moment interruption left bootable is picked up as running firmware; rescue_drain() no longer inspects or reports the outcome (its stale kernel-doc claiming a "return 1" case is gone)
- poll the drain status register with read_poll_timeout() as well, which evaluates the condition once more after the deadline, matching the poll fix in the previous patch
- treat the flashless-download loop (STAT 0xc33c) as an unsupported configuration and fail probe with -ENODEV, rather than advertising it as flashable when the console flash path cannot drive it
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 | 428 +++++++++++++++++++- drivers/net/dsa/mxl862xx/mxl862xx-fw.h | 3 + drivers/net/dsa/mxl862xx/mxl862xx-host.c | 9 + drivers/net/dsa/mxl862xx/mxl862xx-phylink.c | 2 + drivers/net/dsa/mxl862xx/mxl862xx.c | 106 ++++- drivers/net/dsa/mxl862xx/mxl862xx.h | 34 ++ 6 files changed, 549 insertions(+), 33 deletions(-)
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +416 -12
@@ -29,9 +29,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 -> finalise now (optional, see below) +* RDREG 0xe2c0 host -> register-read command (| index), see below * * Console flash path (STAT=0xc55c) - mxl862xx_flash_firmware(): *
@@ -71,7 +73,40 @@* - never send a slice/chunk count larger than what is outstanding; * - a STAT write is a command only once the loader has left the loop; * - the loader leaves the count in STAT while it programs the chunk, so -* a lingering count does not distinguish "busy" from "verdict". +* a lingering count does not distinguish "busy" from "verdict"; +* - 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 in the receive loop holding +* STAT=0. Feed single 1-byte chunks (one DATA word + STAT=1) until r_remain +* reaches 0; the loader then verifies the (now corrupt) image, publishes its +* verdict and comes back to READY by itself. END is never sent here: while +* r_remain is non-zero it would be consumed as a 15555-byte count, and a +* lingering STAT=1 cannot be told from a chunk still being programmed. +* +* 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 long 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 comes from the device tree; the 0xe1xx register +* offsets are the OTP reset defaults and the only layout supported here. */ #include <linux/crc32.h>
@@ -104,8 +139,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/* Image verification verdict published in STAT once the receive loop ends */ #define MXL862XX_SB_PDI_VERIFY_OK 0
@@ -124,6 +166,10 @@#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 +#define MXL862XX_DRAIN_POLL_MS 2000 +/* Covers the loader's END wait, verification and the reset into READY */ +#define MXL862XX_DRAIN_DONE_MS 15000static int mxl862xx_sb_pdi_reset (struct mxl862xx_priv *priv) {
@@ -260,6 +306,323 @@ static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev)return rp; } +/* 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 + +/* Log the drain's progress every so many bytes; it can run for a long time */ +#define MXL862XX_DRAIN_LOG_BYTES (128 * 1024) + +/* Wait for the loader to ask for the next chunk (STAT 0) or to come back to its +* command loop (STAT READY), and return the STAT value either way. On timeout +* that is whatever STAT still holds, which carries no further information: the +* loader keeps the count we wrote visible while it programs the chunk, and that +* is the same value it publishes as the "image rejected" verdict once the +* counter reaches zero. +*/ +static int mxl862xx_sb_pdi_poll_drain (struct mxl862xx_priv *priv, +unsigned long timeout_ms) +{ +int val; + +read_poll_timeout (mxl862xx_smdio_read, val, + val <0 || (u16 )val ==MXL862XX_SB_PDI_READY || + (u16 )val ==0 , +50 , timeout_ms *1000 , false, + priv,MXL862XX_SB_PDI_STAT ); +if (val <0 ) +return val; +return (u16 )val; +} + +/* The loader is not asking for a chunk: it may still be programming the last +* one, or the counter has reached zero and it is verifying the image and +* resetting into READY. Wait that out -- STAT cannot tell the two apart, and +* guessing would mean writing END into a live receive loop. Returns 0 once the +* loader has left the loop, -EAGAIN if it asks for another chunk after all. +*/ +static int mxl862xx_rescue_drain_finish (struct mxl862xx_priv *priv,u32 chunk) +{ +struct device *dev = &priv->mdiodev->dev; +int stat; + + stat =mxl862xx_sb_pdi_poll_drain (priv,MXL862XX_DRAIN_DONE_MS ); +if (stat <0 ) +return stat; +if (stat ==MXL862XX_SB_PDI_READY ) +return 0 ; +if (stat ==MXL862XX_SB_PDI_VERIFY_BAD ) { +dev_err (dev, +"flash: loader stuck after %u chunks, power cycle it\n" , + chunk); +return -EIO ; + } +if (stat) { +/* A firmware is answering, not the loader: an image survived +* in flash and booted. +*/ +dev_info (dev,"flash: firmware booted while draining\n" ); +return 0 ; + } + +return -EAGAIN ; +} + +/* 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. +* The loader then verifies the (now corrupt) image and returns to READY by +* itself, or boots a valid image that happened to survive in flash; either way +* the caller's reprobe classifies the result. Every byte costs about a dozen +* MDIO frames, so a multi-MiB outstanding count takes tens of minutes. +* Returns 0 once the loader has left the receive loop, <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, stat; + +while (chunk < max_chunks) { +/* Teardown can interrupt this long drain. */ +if (test_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags)) +return -ECANCELED ; + + stat =mxl862xx_sb_pdi_poll_drain (priv,MXL862XX_DRAIN_POLL_MS ); +if (stat <0 ) +return stat; +if (stat ==MXL862XX_SB_PDI_READY ) +return 0 ; + +if (stat) { + ret =mxl862xx_rescue_drain_finish (priv, chunk); +if (ret != -EAGAIN ) +return ret; + } + +/* 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++; +if (!(chunk %MXL862XX_DRAIN_LOG_BYTES )) +dev_info (dev,"flash: drained %u KiB so far\n" , + chunk /1024 ); +cond_resched (); + } + +dev_err (dev, +"flash: interrupted download did not drain after %u chunks\n" , + chunk); + +return -ETIMEDOUT ; +} + +/* Background self-heal: drain a wedged download off the devlink flash path, so +* the long recovery never holds the devlink lock. Scheduled from probe; +* reprobes on success so the probe-time detection re-classifies the switch. +*/ +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; +struct mxl862xx_reprobe *ko; +int ret; + + ret =mxl862xx_rescue_drain (priv); +if (ret == -ECANCELED ) +return ; +if (ret) { +/* Nothing retries this, so say so: rescue_ready stays clear +* and devlink dev flash reports why it refuses. +*/ +dev_err (dev,"flash: download recovery failed: %pe\n" , +ERR_PTR (ret)); +WRITE_ONCE (priv->rescue_failed, true); +return ; + } + +/* The interrupted transfer is finalised; reprobe so the probe-time +* detection brings the driver up -- flashable in rescue mode if the +* loader is at READY, or normally if a valid image booted. The refs +* are released by the reprobe once it completes. heal_lock makes the +* flag test and the queueing atomic against teardown, which cannot +* cancel a work item this one has already queued. +*/ +mutex_lock (&priv->heal_lock); +if (test_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags)) + goto out; +if (!try_module_get (THIS_MODULE )) + goto out; +get_device (dev); + ko =mxl862xx_reprobe_alloc (dev); +if (!ko) { +put_device (dev); +module_put (THIS_MODULE ); + goto out; + } +queue_delayed_work (system_long_wq, &ko->work, +msecs_to_jiffies (MXL862XX_FW_REPROBE_DELAY_MS )); +out: +mutex_unlock (&priv->heal_lock); +} + +/* 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); the mailbox is reset first, or a transfer interrupted with +* CTRL=WR would take that write as a payload word instead of latching it. STAT +* then classifies the state, poked destructively only when 0, the one value a +* running firmware never holds: +* +* - 0xc33c: flashless loop; recognised but not supported here. +* - 0xc55c: console loop, if the register-read challenge is serviced. +* - other non-zero: running firmware, left unpoked. +* - 0: wedged receive loop; the 1-byte slice-advance then says whether it +* still needs draining or has just finished. +* +* Return: MXL862XX_IN_RESCUE, MXL862XX_NOT_RESCUE, -ENODEV when nothing +* answers, -EOPNOTSUPP for the flashless loop, -ENXIO for a READY loader whose +* mailbox fails the challenge, or an SMDIO bus error. +*/ +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. */ +WRITE_ONCE (priv->rescue_ready, false); + + ret =mxl862xx_sb_pdi_reset (priv); +if (ret <0 ) +return ret; + +/* 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 (MxL86281S tier): this driver does not +* support it -- the console flash path expects READY. Treat it as an +* unusable configuration, like any other unsupported state. +*/ +if ((u16 )stat ==MXL862XX_SB_PDI_DL_READY ) +return -EOPNOTSUPP ; + +/* 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 ); + ret =mxl862xx_sb_pdi_reset (priv); +if (rb <0 ) +return rb; +if (dat <0 ) +return dat; +if (ret <0 ) +return ret; +if (!rb && (u16 )dat !=MXL862XX_SB_PDI_RDREG_MARK ) { +WRITE_ONCE (priv->rescue_ready, true); +return MXL862XX_IN_RESCUE ; + } +return -ENXIO ; + } + +/* Any other non-zero value is a running firmware, not a loader. */ +if (stat) +return MXL862XX_NOT_RESCUE ; + +/* STAT == 0: a wedged receive loop takes a 1-byte slice-advance (feed +* one DATA word first, like a drain chunk) and asks for the next chunk +* by publishing 0 again. Had that byte been the last one outstanding, +* the loader leaves the loop instead and returns to READY, having +* consumed the advance -- proof enough of a live mailbox to skip the +* challenge. Anything else means it is still working on it. All three +* are rescue, so this never fails probe; only the drain does. +*/ + 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; + + rb =mxl862xx_sb_pdi_poll_drain (priv,MXL862XX_RESCUE_READY_TIMEOUT_MS ); +if (rb <0 ) +return rb; +if (rb ==MXL862XX_SB_PDI_READY ) +WRITE_ONCE (priv->rescue_ready, true); + +return MXL862XX_IN_RESCUE ; +} +/* MCUboot firmware image header */ struct mxl862xx_fw_hdr {__le32 image_type;
@@ -335,13 +698,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 */
@@ -523,6 +888,25 @@ 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, +DEVLINK_INFO_VERSION_GENERIC_FW , buf); +if (ret) +return ret; +return devlink_info_version_stored_put (req, +DEVLINK_INFO_VERSION_GENERIC_FW , buf); + } +/* A 0 part number means the CHIP ID read failed or the part is * unfused; omit it rather than publish a bogus "0000" that fwupd * would match firmware against -- it then falls back to the driver
@@ -596,6 +980,21 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,return ret; } +/* Refuse to flash while the background self-heal is still draining, and +* for good once it has given up on the loader. +*/ +if (READ_ONCE (priv->rescue_failed)) { +NL_SET_ERR_MSG_MOD (extack, +"download recovery failed, power cycle the switch" ); +return -EIO ; + } + +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).
@@ -615,9 +1014,13 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,return -ENOMEM ; } -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 so userspace
@@ -646,7 +1049,7 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds, priv->block_host = true;mutex_unlock (&priv->mdiodev->bus->mdio_lock); -set_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags); +mxl862xx_stop_work (priv);cancel_delayed_work_sync (&priv->stats_work);cancel_work_sync (&priv->crc_err_work);for (i =0 ; i < ds->num_ports; i++)
@@ -661,6 +1064,7 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,mutex_lock_nested (&priv->mdiodev->bus->mdio_lock,MDIO_MUTEX_NESTED ); priv->block_host = false; + priv->rescue_mode = false;mutex_unlock (&priv->mdiodev->bus->mdio_lock);/* Refresh the cached versions so the flash update only
drivers/net/dsa/mxl862xx/mxl862xx-fw.h +3 -0
@@ -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 +9 -0
@@ -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;
@@ -548,10 +554,13 @@ int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val)void mxl862xx_host_init (struct mxl862xx_priv *priv) { +mutex_init (&priv->heal_lock);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
@@ -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 +85 -21
172 lines not on this page · raw message
drivers/net/dsa/mxl862xx/mxl862xx.h +34 -0
@@ -4,7 +4,9 @@#define __MXL862XX_H#include <asm/byteorder.h> +#include <linux/bitops.h> #include <linux/mdio.h> +#include <linux/mutex.h> #include <linux/workqueue.h> #include <net/dsa.h>
@@ -14,6 +16,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 +333,18 @@ 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. Set from setup() before the +* switch is registered and cleared under the MDIO bus lock +* for the benefit of mxl862xx_api_wrap(); other readers +* only need it to be a stable single flag. +* @rescue_ready: (rescue_mode) loader is at a clean READY and will accept +* a flash; false while rescue_heal_work is draining +* @rescue_failed: (rescue_mode) the self-heal gave up; the loader needs a +* power cycle and no flash can be accepted +* @rescue_heal_work: background self-heal draining a wedged download to READY +* @heal_lock: serialises the self-heal's reprobe hand-off against +* teardown setting %MXL862XX_FLAG_WORK_STOPPED * @stats_work: periodic work item that polls RMON hardware counters * and accumulates them into 64-bit per-port stats */
@@ -334,6 +352,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,9 +368,24 @@ struct mxl862xx_priv {u16 vf_block_size;bool block_host;bool skip_teardown; +bool rescue_mode; +bool rescue_ready; +bool rescue_failed; +struct mutex heal_lock; struct delayed_work stats_work; }; +/* Stop the background workers. The flag is set under heal_lock, which the +* rescue self-heal also holds across its own flag test and the queueing of its +* reprobe, so no reprobe can slip past a teardown that has begun. +*/ +static inline void mxl862xx_stop_work (struct mxl862xx_priv *priv) +{ +mutex_lock (&priv->heal_lock); +set_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags); +mutex_unlock (&priv->heal_lock); +} +int mxl862xx_wait_ready (struct dsa_switch *ds);#endif /* __MXL862XX_H */
Signature · 1 line
2.55.0
Paolo Abeni
maintainerRe: [PATCH net-next v9 4/5] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
From: AI Reviewer <[email protected]>
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://netdev-ai.bots.linux.dev/sashiko/
---
--- Patch [4]: [PATCH net-next v9 4/5] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode --- commit c3cc712743add1015d17df863b9f273b60dce171
Author: Daniel Golle <[email protected]>
net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
This adds a clause-22 SB PDI probe at setup time that classifies whether the switch is running firmware or sitting in its MCUboot loader, registers the switch in a reduced "rescue mode" when only the loader answers, and adds a background work item that drains an interrupted download one byte at a time before devlink dev flash is allowed to recover the switch.
A few questions on the new detection and drain paths below.
Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +0
-0
[ ... ]
Quoted 8 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +4
-0@@ -124,6 +166,10 @@#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 +#define MXL862XX_DRAIN_POLL_MS 2000 +/* Covers the loader's END wait, verification and the reset into READY */ +#define MXL862XX_DRAIN_DONE_MS 15000
[ ... ]
Quoted 14 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +10
-0@@ -260,6 +306,323 @@ static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev)return rp; } +/* 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
[ ... ]
Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +4
-0+static int mxl862xx_sb_pdi_poll_drain (struct mxl862xx_priv *priv, +unsigned long timeout_ms) +{ +int val;
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 343
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +10
-0+ +read_poll_timeout (mxl862xx_smdio_read, val, + val <0 || (u16 )val ==MXL862XX_SB_PDI_READY || + (u16 )val ==0 , +50 , timeout_ms *1000 , false, + priv,MXL862XX_SB_PDI_STAT ); +if (val <0 ) +return val; +return (u16 )val; +}
[Severity: Medium]
[Finding: main-3-5]
[Sources: opus-5]
Is STAT == 0 unambiguous enough to act on here? The file header says the loader publishes 0 both to ask for the next slice and as the "image verified OK" verdict, and this patch encodes the latter as MXL862XX_SB_PDI_VERIFY_OK == 0. It is also the value the mailbox word holds after a reset, since mxl862xx_sb_pdi_reset() only writes CTRL/ADDR/DATA.
mxl862xx_sb_pdi_poll_drain() breaks on "(u16)val == 0" and returns 0 for all of those states, and mxl862xx_rescue_drain() returns early only on READY or a non-zero STAT:
stat = mxl862xx_sb_pdi_poll_drain(priv, MXL862XX_DRAIN_POLL_MS);
...
if (stat == MXL862XX_SB_PDI_READY)
return 0;
if (stat) {
ret = mxl862xx_rescue_drain_finish(priv, chunk);
...
}
/* Feed one zero byte; reset cleared the write latch. */
...
ret = mxl862xx_smdio_write(priv, MXL862XX_SB_PDI_STAT,
MXL862XX_DRAIN_CHUNK_BYTES);So when the last outstanding byte has just been consumed and the loader has left the receive loop publishing verdict 0, does this write another count with nothing outstanding? That is the case the MXL862XX_DRAIN_CHUNK_BYTES comment describes as underflowing the loader's counter "for ~2^32 more bytes (a state only a power cycle clears)", and it also contradicts the header rule that "a STAT write is a command only once the loader has left the loop".
The alternative outcome looks equally unpleasant: STAT stays latched at 1, which the next poll plus mxl862xx_rescue_drain_finish() read as MXL862XX_SB_PDI_VERIFY_BAD and turn into -EIO, latching priv->rescue_failed for a switch that actually finished cleanly.
The same single interpretation is applied in mxl862xx_rescue_mode_detect()'s STAT == 0 branch, with no liveness confirmation (unlike the READY path with its RDREG challenge), and its second call site in mxl862xx_setup() runs right after mxl862xx_reset() has software-reset the switch:
ret = mxl862xx_wait_ready(ds);
if (ret) {
/* the reset may only now have triggered rescue mode */
rescue = mxl862xx_rescue_mode_detect(priv);Can STAT read its reset value 0 there while the MCU is still booting, after which the driver writes a payload word plus a bogus count and queues an hours-long drain against a chip that is not in a receive loop at all?
Quoted 13 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +13
-0+ +/* The loader is not asking for a chunk: it may still be programming the last +* one, or the counter has reached zero and it is verifying the image and +* resetting into READY. Wait that out -- STAT cannot tell the two apart, and +* guessing would mean writing END into a live receive loop. Returns 0 once the +* loader has left the loop, -EAGAIN if it asks for another chunk after all. +*/ +static int mxl862xx_rescue_drain_finish (struct mxl862xx_priv *priv,u32 chunk) +{ +struct device *dev = &priv->mdiodev->dev; +int stat; + + stat =mxl862xx_sb_pdi_poll_drain (priv,MXL862XX_DRAIN_DONE_MS );
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 366
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +10
-0+if (stat <0 ) +return stat; +if (stat ==MXL862XX_SB_PDI_READY ) +return 0 ; +if (stat ==MXL862XX_SB_PDI_VERIFY_BAD ) { +dev_err (dev, +"flash: loader stuck after %u chunks, power cycle it\n" , + chunk); +return -EIO ; + }
[Severity: Medium]
[Finding: main-1-1]
[Sources: opus-5]
The new drain path and the existing flash path seem to disagree by about 8x on how long the same loader step may take, and on what a lingering STAT == 1 means.
mxl862xx_sb_pdi_flush_last() gives the count 120 s to disappear and treats a lingering 1 as the documented "image rejected" verdict:
ret = read_poll_timeout(mxl862xx_smdio_read, val,
val < 0 || (u16)val != (u16)data_written,
10000, MXL862XX_FW_WRITE_TIMEOUT_MS * 1000, ...);
...
if ((u16)val == MXL862XX_SB_PDI_VERIFY_BAD) { ... return -EBADMSG; }Here the budget is MXL862XX_DRAIN_POLL_MS (2 s) plus MXL862XX_DRAIN_DONE_MS (15 s), and the same value is declared a permanent wedge. Given that the drain writes STAT = MXL862XX_DRAIN_CHUNK_BYTES == 1, that 1 is also MXL862XX_SB_PDI_VERIFY_BAD, and that the file header states "the loader leaves the count in STAT while it programs the chunk, so a lingering count does not distinguish 'busy' from 'verdict'", can a loader that is merely slow to verify a multi-MiB image be misclassified here?
The verdict is then sticky for the rest of the binding: mxl862xx_rescue_heal_work_fn() does WRITE_ONCE(priv->rescue_failed, true), mxl862xx_devlink_flash_update() answers -EIO with "download recovery failed, power cycle the switch", and nothing ever clears rescue_failed. Would a longer budget matching MXL862XX_FW_WRITE_TIMEOUT_MS, or a retry rather than a latched flag, fit the protocol better?
[ ... ]
Quoted 21 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +21
-0+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; +struct mxl862xx_reprobe *ko; +int ret; + + ret =mxl862xx_rescue_drain (priv); +if (ret == -ECANCELED ) +return ; +if (ret) { +/* Nothing retries this, so say so: rescue_ready stays clear +* and devlink dev flash reports why it refuses. +*/ +dev_err (dev,"flash: download recovery failed: %pe\n" , +ERR_PTR (ret)); +WRITE_ONCE (priv->rescue_failed, true); +return ; + } +
[ ... ]
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 491
Quoted 12 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +12
-0+mutex_lock (&priv->heal_lock); +if (test_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags)) + goto out; +if (!try_module_get (THIS_MODULE )) + goto out; +get_device (dev); + ko =mxl862xx_reprobe_alloc (dev); +if (!ko) { +put_device (dev); +module_put (THIS_MODULE ); + goto out; + }
[Severity: Medium]
[Finding: main-3-6]
[Sources: opus-5]
After a successful drain, recovery is handed entirely to the queued reprobe and priv->rescue_ready is never set. What happens to the switch if try_module_get() fails or mxl862xx_reprobe_alloc() returns NULL?
Both take "goto out" with rescue_ready == false, rescue_failed == false and no log line, while the loader is sitting at a clean READY and would accept a flash. From then on devlink reports nothing:
mxl862xx_devlink_info_get()
if (!READ_ONCE(priv->rescue_ready)) return 0;
and every flash attempt is refused:
mxl862xx_devlink_flash_update()
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;
}Since nothing re-queues rescue_heal_work or re-runs mxl862xx_rescue_mode_detect() while the driver stays bound, does a single -ENOMEM here leave the switch unflashable until a manual unbind/rebind? Would setting rescue_ready on a successful drain before the reprobe is queued cover this?
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 496
Quoted 5 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +5
-0+queue_delayed_work (system_long_wq, &ko->work, +msecs_to_jiffies (MXL862XX_FW_REPROBE_DELAY_MS )); +out: +mutex_unlock (&priv->heal_lock); +}
[Severity: High]
[Finding: main-4-7]
[Sources: opus-5]
The reprobe queued here cannot be cancelled by anything in the driver:heal_lock only makes the WORK_STOPPED test and the queueing atomic against mxl862xx_stop_work(), and mxl862xx_host_shutdown() cancels rescue_heal_work itself but not the delayed work it may have queued. What happens if the heal work wins that race and drops heal_lock just before teardown sets the flag?
For unbind it re-binds a device that was deliberately unbound. For shutdown the sequence looks worse:
mxl862xx_shutdown()
dsa_switch_shutdown(ds); /* leaves ds linked in dst->ports */ mxl862xx_stop_work(priv); /* too late */ mxl862xx_host_shutdown(priv); /* cancels only rescue_heal_work */ ... dev_set_drvdata(&mdiodev->dev, NULL);
500 ms later the reprobe runs device_reprobe(), which detaches first:
mxl862xx_remove()
if (!ds) return; /* drvdata was cleared, so no dsa_unregister_switch() */
devres then frees the devm-allocated priv/ds while the DSA tree still holds dsa_ports whose dp->ds points at it, and the re-attach walks exactly those:
net/dsa/dsa.c:dsa_switch_find() {
list_for_each_entry(dp, &dst->ports, list) {
if (dp->ds->index != sw_index)
continue;
return dp->ds;
}
}Is that a use-after-free read of the freed dsa_switch, reached via mxl862xx_probe() -> dsa_register_switch() -> dsa_switch_parse_of() -> dsa_switch_find()? A failed dsa_register_switch() after mxl862xx_setup() queued the heal work looks able to lose the same race.
The uncancellable reprobe mechanism itself came in earlier in this series with "net: dsa: mxl862xx: add devlink flash_update and info_get"; what this patch adds is a second trigger that is not user-initiated and can fire tens of minutes after probe. Could the reprobe re-validate dev->driver before re-binding, or be made cancellable from teardown?
Quoted 2 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +2
-0+ +/* Detect MCUboot rescue mode over clause-22 SMDIO alone, so the caller can rule
[ ... ]
Quoted 4 lines
+ * Return:
MXL862XX_IN_RESCUE,MXL862XX_NOT_RESCUE,-ENODEVwhen nothing
+ * answers,-EOPNOTSUPPfor the flashless loop,-ENXIOfor a READY loader whose
+ * mailbox fails the challenge, or an SMDIO bus error.
+ */
[Severity: Low]
[Finding: main-2-4]
[Sources: opus-5]
This isn't a bug, but the documented return set does not quite match the code. The main way the challenge fails is the loader never re-arming STAT=READY, and then mxl862xx_sb_pdi_poll_stat() returns the read_poll_timeout() result:
rb = mxl862xx_sb_pdi_poll_stat(priv, MXL862XX_SB_PDI_READY, MXL862XX_RESCUE_READY_TIMEOUT_MS); ... if (rb < 0) return rb;
That propagates -ETIMEDOUT, which is neither -ENXIO nor an SMDIO bus error. -ENXIO is only reached when READY was seen but DATA still holds the marker.
The comment on mxl862xx_rescue_drain_finish() has the same mismatch: it documents only 0 and -EAGAIN while the body also returns -EIO and propagated SMDIO bus errors.
Quoted 19 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +19
-0+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. */ +WRITE_ONCE (priv->rescue_ready, false); + + ret =mxl862xx_sb_pdi_reset (priv); +if (ret <0 ) +return ret; + +/* 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 )
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 544
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +10
-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 ;
[Severity: Medium]
[Finding: main-1-0]
[Sources: opus-5]
The commit message says a running firmware "is left untouched":
"a running firmware, which answers with its own non-zero status word (0x0003 on the firmware tested) and is left untouched"
Does that hold? mxl862xx_sb_pdi_reset() writes CTRL/ADDR/DATA and the two scratch writes above land in the mailbox before STAT is read, so they reach a switch whose firmware is alive and well.
The second half of the same paragraph says -ENODEV covers only:
"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)"
But the read-back compare above also returns -ENODEV for a live switch whose SB PDI window is not at the offsets this file documents as "the OTP reset defaults and the only layout supported here", and mxl862xx_setup() turns that into a probe failure:
rescue = mxl862xx_rescue_mode_detect(priv);
if (rescue < 0) {
dev_err(ds->dev, "switch state detection failed: %pe\n",
ERR_PTR(rescue));
return rescue;
}So a board that bound and worked before this patch would now fail to probe. Should the presence probe only be consulted after the clause-45 path has failed, or should the commit message describe these two behaviours?
[ ... ]
Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +4
-0+/* 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 ) {
[ ... ]
drivers/net/dsa/mxl862xx/mxl862xx-fw.c static struct mxl862xx_reprobe *mxl862xx_reprobe_alloc(struct device *dev) · line 588
Quoted 6 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +6
-0+if (!rb && (u16 )dat !=MXL862XX_SB_PDI_RDREG_MARK ) { +WRITE_ONCE (priv->rescue_ready, true); +return MXL862XX_IN_RESCUE ; + } +return -ENXIO ; + }
[Severity: Medium]
[Finding: main-1-2]
[Sources: opus-5]
The commit message presents the classification as exhaustive:
"The first three enter rescue mode or normal operation, the last two fail probe cleanly with -EOPNOTSUPP and -ENODEV instead of a CRC-error storm."
Is there not a sixth outcome here? A loader idle in the console loop at STAT == MXL862XX_SB_PDI_READY that does not service the register-read challenge ends up at this -ENXIO (or at the raw poll error), and mxl862xx_setup() propagates it as a fatal probe error.
That is a switch genuinely in MCUboot rescue mode, the state this patch exists to recover, and the driver then refuses to bind, so no devlink instance is created and the documented devlink dev flash recovery is unreachable. Given that RDREG is an undocumented loader command whose reply the header describes as coming from "loader BSS, not a chip id", should a loader that ignores it still be treated as flashable rather than as a probe failure?
[ ... ]
Quoted 19 lines
drivers/net/dsa/mxl862xx/mxl862xx-fw.c +15
-0@@ -596,6 +980,21 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds,return ret; } +/* Refuse to flash while the background self-heal is still draining, and +* for good once it has given up on the loader. +*/ +if (READ_ONCE (priv->rescue_failed)) { +NL_SET_ERR_MSG_MOD (extack, +"download recovery failed, power cycle the switch" ); +return -EIO ; + } + +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 ; + } +
[ ... ]
Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx-host.c +0
-0
[ ... ]
Quoted 14 lines
drivers/net/dsa/mxl862xx/mxl862xx-host.c +3
-0@@ -548,10 +554,13 @@ int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val)void mxl862xx_host_init (struct mxl862xx_priv *priv) { +mutex_init (&priv->heal_lock);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); }
[ ... ]
Quoted 5 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +0
-0@@ -674,21 +674,66 @@ static int mxl862xx_setup(struct dsa_switch *ds)
[ ... ]
Quoted 45 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +45
-0+ rescue =mxl862xx_rescue_mode_detect (priv); +if (rescue <0 ) { +dev_err (ds->dev,"switch state detection failed: %pe\n" , +ERR_PTR (rescue)); +return rescue; + } + +if (rescue ==MXL862XX_NOT_RESCUE ) { + ret =mxl862xx_reset (priv); +if (ret) +return ret; + + ret =mxl862xx_wait_ready (ds); +if (ret) { +/* the reset may only now have triggered rescue mode */ + rescue =mxl862xx_rescue_mode_detect (priv); +if (rescue <0 ) { +dev_err (ds->dev, +"switch not responding after reset: %pe\n" , +ERR_PTR (rescue)); +return rescue; + } +if (rescue ==MXL862XX_NOT_RESCUE ) +return ret; + } + } + + priv->rescue_mode = rescue; + +if (priv->rescue_mode) { +if (priv->rescue_ready) { +dev_warn (ds->dev, +"switch in MCUboot rescue mode, use devlink to flash new firmware\n" ); + }else { +/* Drain the wedged download in the background so it +* never holds the devlink lock; info and flash become +* available once ready. +*/ +dev_warn (ds->dev, +"switch in MCUboot with an interrupted download, recovering in background\n" ); +queue_work (system_long_wq, &priv->rescue_heal_work); + } +return 0 ; + } +
[ ... ]
Quoted 9 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +1
-1@@ -2245,7 +2309,7 @@ static void mxl862xx_shutdown(struct mdio_device *mdiodev)dsa_switch_shutdown (ds); -set_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags); +mxl862xx_stop_work (priv);cancel_delayed_work_sync (&priv->stats_work);mxl862xx_host_shutdown (priv);
[ ... ]
Quoted 4 lines
drivers/net/dsa/mxl862xx/mxl862xx.h +0
-0
[ ... ]
drivers/net/dsa/mxl862xx/mxl862xx.h struct mxl862xx_priv { · line 388
Quoted 11 lines
drivers/net/dsa/mxl862xx/mxl862xx.h +11
-0+/* Stop the background workers. The flag is set under heal_lock, which the +* rescue self-heal also holds across its own flag test and the queueing of its +* reprobe, so no reprobe can slip past a teardown that has begun. +*/ +static inline void mxl862xx_stop_work (struct mxl862xx_priv *priv) +{ +mutex_lock (&priv->heal_lock); +set_bit (MXL862XX_FLAG_WORK_STOPPED , &priv->flags); +mutex_unlock (&priv->heal_lock); +} +
The comment says "no reprobe can slip past a teardown that has begun", but the mutex only orders the flag store against the heal work's flag test. If the heal work tested the flag first and queued the reprobe, does anything here stop that reprobe from running after teardown completes? See the question at mxl862xx_rescue_heal_work_fn() above.
Signature · 1 line
This is an AI-generated review.
Daniel Golle
maintainerRe: [PATCH net-next v9 4/5] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
Quoted 2 lines · Paolo Abeni
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible.
Thanks. I traced all six findings. One is a real bug, two become comment and commit message fixes, and the rest I believe the loader protocol answers.
For context, since several findings concern state classification: v9 was tested on hardware in all three states the detection has to tell apart, that is WSP firmware running, loader idle at READY, and a loader wedged in the payload receive loop after an interrupted download. Classification was correct in each case, and the wedged case drained back to READY and reprobed into a flashable rescue mode.
[main-4-7] the queued reprobe cannot be cancelled
Real, and I will fix it. dsa_switch_shutdown() only unlinks the netdevs, so dst->ports keeps entries whose dp->ds points at the devm allocated switch, and the tree stays on dsa_tree_list. mxl862xx_shutdown() then clears drvdata. A reprobe queued earlier fires 500 ms later and finds dev->driver still set, since shutdown does not unbind, so it detaches: mxl862xx_remove() returns early on the NULL drvdata, dsa_unregister_switch() never runs, devres frees priv and ds under the live tree, and the rebind walks those stale ports in dsa_switch_find().
Two corrections. The probe failure variant is already covered, because mxl862xx_probe() cancels the heal work when dsa_register_switch() fails and that path calls dsa_switch_release_ports(), leaving no stale ports. And the mechanism came in with patch 3: every user flash queues the same uncancellable reprobe. The realistic reproducer is "devlink dev flash, then reboot within 500 ms", so the fix belongs there.
The simple fixes do not hold. cancel_delayed_work_sync() from teardown deadlocks, as the work calls device_reprobe() which takes device_lock while device_shutdown() holds it across ->shutdown(). A drvdata or system_state test inside the work is check then act: it can pass, block on device_lock behind shutdown, and detach anyway. For v10 I plan to track the pending reprobe in priv under heal_lock and have teardown do an asynchronous cancel_delayed_work() with an ownership handoff, so whoever wins the cancel drops the module and device references. For a work item already executing when teardown starts, I will drop devm for priv and ds and free them in mxl862xx_remove(). The early return on cleared drvdata then leaks them on a dying system instead of freeing them under a live tree, dsa_switch_find() returns the old ds, parse fails with a clean -EEXIST, and the reprobe only logs that the device was left unbound.
[main-1-1] inconsistent timeouts for the same loader step
Agreed, fixed for v10. FW_WRITE_TIMEOUT_MS drops from 120 s to 60 s. DRAIN_DONE_MS becomes SB_PDI_VERIFY_MS, still 15 s, and now covers the verdict wait on both paths; flush_last() cannot see the boundary between programming the last slice and verifying, so it gets the sum of the write and verify budgets. DRAIN_POLL_MS becomes SB_PDI_STEP_MS at 2 s, and both detection waits use it too. RESCUE_READY_TIMEOUT_MS is gone. There is now one constant per loader operation.
I would keep the sticky verdict. With the budgets aligned, a STAT still holding 1 after the verify window means a genuinely wedged loader, usually a counter underflowed by an earlier oversized chunk, and only a power cycle clears that. I will document that power cycling the switch alone does not clear rescue_failed while the driver stays bound.
[main-3-5] is STAT == 0 unambiguous enough?
I do not think the ambiguous state exists. The loader leaves the receive loop exactly when r_remain reaches zero, so "in the loop, asking for the next chunk" and "out of the loop, publishing verdict 0" cannot overlap in time. Once it has left, a STAT write is a command rather than a decrement, so there is nothing to underflow. Verdict 0 ending a drain would also require the zero stuffed remains of an interrupted image to pass the checksum. What the hardware does at the end of a drain is the other branch: STAT lingers at 1 during verify, then the loader resets into READY, which is what mxl862xx_rescue_drain_finish() waits out.
The post reset case cannot arrive that early either. The second detection call runs only after mxl862xx_reset() and after mxl862xx_wait_ready() has spent its whole budget, an unconditional 2 s sleep plus polling to timeout, while MCUboot reaches its loop in milliseconds. It is also self correcting: a booting MCU publishes READY or a firmware status word, a freshly booted loader sits in the console loop where the 1-byte advance is not a count, and the drain converges through the "firmware booted while draining" branch. I will add a comment so the next reader need not re-derive this.
[main-3-6] alloc failure after a successful drain
The hole is real: a NULL from mxl862xx_reprobe_alloc() leaves both rescue_ready and rescue_failed clear and logs nothing, so flash answers "retry shortly" forever. I would rather not set rescue_ready before queueing the reprobe, though. That flag is what stops a user flash from starting while the reprobe is pending, given that the flash path's own guard is skip_teardown, which the heal path never sets, so setting it early would let a flash begin and then be detached mid transfer. v10 will log the failure and set rescue_failed instead. The try_module_get() leg needs nothing, as it only fails during rmmod.
[main-1-0] "left untouched" and the -ENODEV cases
Fair, the commit message overstates it. The presence probe does write the CTRL/ADDR/DATA scratch latches of a running firmware. It never writes STAT, and the writes are inert on the tested firmware, but I will reword it rather than claim the firmware is untouched.
A live switch with a relocated SB PDI window would indeed fail to probe now. Those offsets are the OTP reset defaults and no supported compatible moves them, so I would document the behaviour rather than consult the probe only after clause-45 has failed, which brings back the CRC error storm for genuinely absent switches. Happy to reconsider.
[main-1-2] the -ENXIO outcome
Deliberate, and I will say so in the commit message. The RDREG challenge is what separates a loader idle at READY from a running firmware whose status word happens to read 0xc55c. Treating an unresponsive challenge as flashable would sometimes classify healthy firmware as a flash target, which is worse than refusing to bind. Every loader I have tested services it.
[main-2-4] documented return values
Both mismatches are real and will be fixed. Detection can also return the -ETIMEDOUT from mxl862xx_sb_pdi_poll_stat() when READY is never re-armed, and drain_finish() also returns -EIO and SMDIO bus errors. I lean towards mapping the challenge timeout to -ENXIO so the documented set becomes true.
Cheers
Daniel
Daniel Golle
maintainerPATCH v9 5/5 net: dsa: mxl862xx: document devlink flash and info support
In reply to Daniel GolleDescribe 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.
---
v9: no changes, picked up Andrew's v5 Reviewed-by
v8:
- asic.id and asic.rev are omitted whenever the part number reads zero, not only in MCUboot rescue mode (found by Sashiko AI review)
- drop the claim that "0.0.0" marks a switch that never ran firmware; rescue mode always reports it (found by Sashiko AI review)
- document devlink dev flash as the signal that says whether a recovery is still running, including the -EIO it returns once the recovery has failed (found by Sashiko AI review)
v7: no changes
v6: no changes
v5: new patch, splitting the devlink documentation out of the flash
update and rescue mode recovery patches so each keeps to code (Jakub Kicinski asked for the documentation)
Documentation/networking/devlink/index.rst | 1 + Documentation/networking/devlink/mxl862xx.rst | 71 +++++++++++++++++++ MAINTAINERS | 1 + 3 files changed, 73 insertions(+) create mode 100644 Documentation/networking/devlink/mxl862xx.rst
Documentation/networking/devlink/index.rst +1 -0
@@ -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 +71 -0
71 lines not on this page · raw message
MAINTAINERS +1 -0
@@ -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