* Re: [PATCH net-next v17 5/6] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
@ 2026-09-25 5:34 netdev-bot+sashiko
0 siblings, 0 replies; 2+ messages in thread
From: netdev-bot+sashiko @ 2026-09-25 5:34 UTC (permalink / raw)
To: daniel
Cc: jiri, davem, edumazet, kuba, pabeni, horms, corbet, skhan,
rdunlap, gregkh, rafael, dakr, andrew, olteanv, linux, netdev,
linux-doc, linux-kernel, driver-core
Thank you for your contribution! Sashiko AI review found 10 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 3 · Low: 6
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] mxl862xx_setup() now runs mxl862xx_rescue_mode_detect(priv,
false) on every probe, before any clause-45 access.
- [Medium] mxl862xx_rescue_mode_detect(priv, true) can never return
MXL862XX_NOT_RESCUE.
- [Medium] mxl862xx_rescue_drain() starts chunk at 0 on every run (line
379).
- [Medium] In rescue mode the CPU port and conduit are set up, so DSA's
conduit ethtool override stays installed.
- [Low] At probe time, rescue_failed can only be set by the START/ACK
branch of mxl862xx_rescue_mode_detect().
- [Low] The patch made mxl862xx_port_mdb_add() also return 0 on -ENODEV
when READ_ONCE(priv->rescue_mode) is set.
- [Low] The @flags kernel-doc in struct mxl862xx_priv still says
MXL862XX_FLAG_WORK_STOPPED "is set before cancelling stats_work to…
- [Low] The file header says the loader leaves the host's byte count in
STAT while it programs a chunk.
- [Low] In rescue mode, mxl862xx_devlink_info_get() reports the zeroed
fw_version cache ("0.0.0") with devlink_info_version_stored_put() as…
- [Low] mxl862xx_setup_rescue() queues rescue_heal_work on
system_long_wq, which is not freezable.
--
net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode
At setup, this patch detects the MCUboot loader over clause-22 SB PDI,
before any clause-45 access. If the loader is found, the switch is
registered with only its CPU port so devlink can reflash it, and devlink
dev info reports 0.0.0. A background work item drains an interrupted
download one byte at a time and then schedules a re-probe.
> diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-fw.c b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c
> index 7a506d110c8da..0761132120b1a 100644
> --- a/drivers/net/dsa/mxl862xx/mxl862xx-fw.c
> +++ b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c
[ ... ]
> @@ -218,6 +268,379 @@ static void mxl862xx_flash_notify(struct devlink *dl, const char *status,
[ ... ]
> +static int mxl862xx_sb_pdi_poll_drain(struct mxl862xx_priv *priv,
> + unsigned long sleep_us,
> + unsigned long timeout_ms)
> +{
> + int val;
> +
> + read_poll_timeout(mxl862xx_smdio_read, val,
> + val < 0 || (u16)val == MXL862XX_SB_PDI_READY ||
> + (u16)val == 0 ||
> + test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags),
> + sleep_us, timeout_ms * 1000, false,
> + priv, MXL862XX_SB_PDI_STAT);
> + if (val < 0)
> + return val;
> + if (test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags))
> + return -ECANCELED;
> + return (u16)val;
> +}
[Severity: Low]
This isn't a bug, but the @flags kernel-doc in struct mxl862xx_priv still
says MXL862XX_FLAG_WORK_STOPPED "is set before cancelling stats_work to
prevent rescheduling during teardown".
The flag now has a second job. It cancels the drain here and in
mxl862xx_rescue_drain(), and it also skips the reprobe hand-off in
mxl862xx_rescue_heal_work_fn(). Unbind and shutdown rely on it to stop a
long drain.
Should the kernel-doc mention this as well?
[ ... ]
> +static int mxl862xx_rescue_drain_finish(struct mxl862xx_priv *priv, u32 chunk)
> +{
[ ... ]
> + if (stat) {
> + if (!chunk) {
> + dev_err(dev,
> + "flash: loader still busy after the erase window\n");
> + return -EIO;
> + }
> + /* 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;
> + }
[Severity: Medium]
Can the "firmware booted while draining" branch be reached when
chunk == 0?
mxl862xx_rescue_drain() starts with chunk = 0 on every run. It does not
count the 1-byte slice-advance that mxl862xx_rescue_mode_detect() already
sent, or the settle wait.
Suppose WSP firmware boots before the worker feeds its first byte. That
can happen if detect's advance completed the image, or on the settle path
after mxl862xx_wait_ready() failed. Wouldn't this then poll for 300 s,
return -EIO and set rescue_failed?
After that there is no reprobe, and rescue_mode stays true, so
mxl862xx_api_wrap() returns -ENODEV for every command. In rescue mode,
mxl862xx_port_setup() also returned before mxl862xx_port_state(false) and
the single-port bridge setup.
Would the running firmware then forward traffic with its default setup
while the driver treats the switch as an inert loader, until someone
rebinds it by hand?
[ ... ]
> + 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;
[Severity: Low]
The file header says the loader keeps the host's byte count in STAT while
it programs a chunk. For the final slice, mxl862xx_sb_pdi_flush_last()
writes data_written, which can be any value from 1 to 65520. The values
0xc33c, 0xc55c, 0xf48f and 0xf490 all fall in that range.
Can detection misread STAT if probe runs while an interrupted host's
final chunk of one of these sizes is still being programmed or verified?
These checks run before any settle wait:
0xc33c: probe fails with -EOPNOTSUPP
0xc55c: the RDREG challenge goes to a loader outside its console loop,
and probe fails with -ENXIO
0xf48f/0xf490: rescue_failed is set, although the loader would finish
on its own
[ ... ]
> + if ((u16)stat == MXL862XX_SB_PDI_START ||
> + (u16)stat == MXL862XX_SB_PDI_START + 1) {
> + WRITE_ONCE(priv->rescue_failed, true);
> + return MXL862XX_IN_RESCUE;
> + }
[Severity: Low]
At probe time this is the only place that sets rescue_failed, and no
drain has run yet. rescue_heal_work is only queued after the
rescue_failed check in mxl862xx_setup_rescue().
Even so, mxl862xx_setup_rescue() logs "download recovery gave up", and
mxl862xx_devlink_flash_update() returns the extack "download recovery
failed".
Could these messages say that the download handshake is stuck, rather
than pointing at a recovery that never ran?
[ ... ]
> @@ -486,6 +911,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 (READ_ONCE(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);
> + }
[Severity: Low]
Documentation/networking/devlink/devlink-info.rst defines the stored group
as the versions in device flash. It also says a device that cannot keep
those versions accurate must not report them.
The file header says MCUboot is also entered through the sticky rescue
bit, so the flash may still hold a valid image with a real version.
Is it intended to report stored 0.0.0 here? That value means "ready to
flash" rather than describing what is in flash.
[ ... ]
> diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-host.c b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
> index 4b3956a518cfe..694c22d2dd094 100644
> --- a/drivers/net/dsa/mxl862xx/mxl862xx-host.c
> +++ b/drivers/net/dsa/mxl862xx/mxl862xx-host.c
[ ... ]
> @@ -347,6 +348,11 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data,
> goto out;
> }
>
> + if (priv->rescue_mode) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
[Severity: Medium]
In rescue mode the CPU port and the conduit are still set up, so DSA's
conduit ethtool override stays in place. ETHTOOL_GSTATS needs no
CAP_NET_ADMIN, so any local user can reach this path:
ethtool -S <conduit>
dsa_conduit_get_ethtool_stats()
dsa_conduit_append_port_stats()
mxl862xx_get_ethtool_stats(ds, cpu_port)
mxl862xx_read_rmon()
mxl862xx_api_wrap() returns -ENODEV here
mxl862xx_get_ethtool_stats() then does this, with no ratelimit:
ret = mxl862xx_read_rmon(ds, port, &cnt);
if (ret) {
dev_err(ds->dev, "failed to read RMON stats on port %d\n", port);
return;
}
Rescue mode can last indefinitely: a stuck handshake, a long drain, or a
loader waiting for a flash. Can an unprivileged user flood the kernel log
this way?
port_enable/disable, stp_state_set and mdb_add/del now have rescue_mode
guards. Should the stats callbacks get one too?
[ ... ]
> diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c
> index 33a7cdb8edd31..911114579f100 100644
> --- a/drivers/net/dsa/mxl862xx/mxl862xx.c
> +++ b/drivers/net/dsa/mxl862xx/mxl862xx.c
> @@ -667,27 +667,85 @@ static void mxl862xx_free_bridge(struct dsa_switch *ds,
> priv->bridges[bridge->num] = 0;
> }
>
> +static void mxl862xx_setup_rescue(struct dsa_switch *ds)
> +{
[ ... ]
> + /* 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);
> +}
[Severity: Low]
system_long_wq is not freezable. The driver also has no PM callback that
sets MXL862XX_FLAG_WORK_STOPPED or cancels this work.
Can a drain that runs for tens of minutes keep accessing the MDIO bus
while the bus controller suspends? Any bus error in that window makes
mxl862xx_rescue_heal_work_fn() set rescue_failed with no retry. After
that, devlink flash returns -EIO until the driver is rebound.
device_schedule_reprobe() uses system_freezable_wq. Would a freezable
workqueue fit better here as well?
> static int mxl862xx_setup(struct dsa_switch *ds)
> {
> struct mxl862xx_priv *priv = ds->priv;
> int n_user_ports = 0, max_vlans;
> int ingress_finals, vid_rules;
> struct dsa_port *dp;
> - int ret, i;
> + int ret, i, rescue;
>
> - ret = mxl862xx_reset(priv);
> - if (ret)
> - return ret;
> + /* Detect the loader over SB PDI first: it needs no firmware, unlike the
> + * C45 API (mxl862xx_reset/wait_ready), which spends its whole 10 s
> + * window on a mailbox nobody answers. Touch C45 only once rescue is
> + * ruled out.
> + */
> + rescue = mxl862xx_rescue_mode_detect(priv, false);
> + if (rescue < 0) {
> + dev_err(ds->dev, "switch state detection failed: %pe\n",
> + ERR_PTR(rescue));
> + return rescue;
> + }
[Severity: High]
Can this break probe on switches that worked before this patch?
Previously the driver only used clause-45 accesses, so a switch with
working firmware probed even when SB PDI was unusable. Now any negative
return from mxl862xx_rescue_mode_detect() fails setup before
mxl862xx_reset() or mxl862xx_wait_ready() is tried.
These cases return an error here:
- The host MDIO bus has no clause-22 ops. In that case
mxl862xx_smdio_write() -> __mdiobus_write() returns -EOPNOTSUPP because
bus->write is NULL. For example, mvmdio in BUS_TYPE_XSMI mode sets only
read_c45/write_c45.
- The ADDR/DATA scratch readback does not match. The kernel-doc says this
includes "one whose SB PDI window is not at the offsets above". The file
header says the 0xe1xx offsets are only the OTP reset defaults.
- STAT reads 0xc33c (-EOPNOTSUPP), or it reads 0xc55c and the marker is
left untouched (-ENXIO).
Every normal boot also writes to the SB PDI mailbox now. If STAT reads 0,
detect sends a 1-byte download chunk and returns MXL862XX_IN_RESCUE
whatever follows. That relies on 0 being "the one value a running
firmware never holds". Is that guaranteed?
Would it be safer to fall back to the clause-45 path when detection
fails, instead of failing probe?
The commit message only says "Detect the loader over SB PDI at setup,
ahead of any clause-45 access". The v10 and v16 changelog notes say the
message was extended to cover the scratch writes to a running firmware,
the -ENODEV/-ENXIO outcomes and the handshake outcome. That text is not
in the final commit message. Could it be added back?
> - ret = mxl862xx_wait_ready(ds);
> - if (ret)
> - return ret;
> + 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, true);
> + 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;
[Severity: Medium]
Can mxl862xx_rescue_mode_detect(priv, true) ever return
MXL862XX_NOT_RESCUE? With settle set, each branch returns either an error
or MXL862XX_IN_RESCUE:
DL_READY: -EOPNOTSUPP
READY: IN_RESCUE or -ENXIO
START/ACK: IN_RESCUE
other non-zero: the !settle return is skipped. The poll then returns
IN_RESCUE for READY or non-zero, or falls into the
STAT == 0 path, which also returns IN_RESCUE
If so, this check is dead code, and the -ETIMEDOUT from
mxl862xx_wait_ready() is always dropped.
Take a firmware that is present but fails mxl862xx_wait_ready(): for
example, it boots slower than the timeout, reports iv_major == 0, or
fails CFGGET. It holds a stable non-zero STAT word. Wouldn't it be
classified as rescue, bound with all user ports unused, and have
rescue_heal_work queued?
The drain then calls mxl862xx_rescue_drain_finish(priv, 0), waits 300 s,
returns -EIO and sets rescue_failed. Before this patch, probe simply
failed with -ETIMEDOUT.
> + }
> + }
[ ... ]
> @@ -1603,7 +1677,8 @@ static int mxl862xx_port_mdb_add(struct dsa_switch *ds, int port,
> * rebuilds the configuration. See mxl862xx_port_mdb_del().
> */
> if ((ret == -EBUSY && priv->block_host) ||
> - (ret == -ENODEV && priv->skip_teardown))
> + (ret == -ENODEV &&
> + (priv->skip_teardown || READ_ONCE(priv->rescue_mode))))
> return 0;
> if (ret)
> return ret;
[Severity: Low]
This isn't a bug, but the comment above still mentions only the flash
cases ("A flash blocks the API (-EBUSY) and its teardown drops the MAC
table (-ENODEV)"). The comment in mxl862xx_port_mdb_del() was updated to
say "its teardown or MCUboot drops the MAC table". Should this comment
be updated to match?
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1790130482.git.daniel%40makrotopia.org
^ permalink raw reply [flat|nested] 2+ messages in thread* [PATCH net-next v17 0/6] net: dsa: mxl862xx: devlink flash and rescue @ 2026-09-23 2:33 Daniel Golle 2026-09-23 2:36 ` [PATCH net-next v17 5/6] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode Daniel Golle 0 siblings, 1 reply; 2+ messages in thread From: Daniel Golle @ 2026-09-23 2:33 UTC (permalink / raw) To: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet, Shuah Khan, Randy Dunlap, Daniel Golle, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Andrew Lunn, Vladimir Oltean, Russell King, netdev, linux-doc, linux-kernel, driver-core This series adds "devlink dev flash" and "devlink dev info" support to the MaxLinear MxL862xx DSA driver, and makes a switch stuck in its MCUboot loader recoverable through the same path. The switch is flashed over the loader's clause-22 SMDIO download interface after the firmware API has rebooted it into MCUboot, and the driver reinitialises through a deferred detach and re-probe once the new image runs. A switch found in MCUboot at probe registers in a reduced rescue mode with firmware version 0.0.0, so the same flash flow recovers it; an interrupted download is drained in the background first. The deferred re-probe comes from a new driver-core helper, device_schedule_reprobe(), which the bound driver calls without a module reference; it also replaces the open-coded self-reprobe that iwlwifi, hci_h5 and btintel_pcie carry, whose work function frees its own module text from under a racing rmmod. fwupd's devlink plugin carries the matching quirks [17]. Patch 3 is a driver-core change and patch 4 does not link without it, so the series needs a driver-core ack before net-next can take it. Tested on an MxL86252C switch of the BananaPi R4 Pro 8X: an upgrade through fwupd; an unbind and a reboot issued while a flash was running, which wait out the transfer and announce the wait; a host crash and a power cut mid-transfer, recovered by the background drain and by the rescue path on the next boot; and an unbind and a reboot issued while a background drain was running, which abort the drain at once and let it resume and complete on the rebind or reboot. Changes since v16 [21]: - patch 3: drop the abort_if_blocked flag and the bool return of __device_release_driver(), leaving that function unchanged. The flag left the device-links state half torn down when it fired and did not cover the consumers unbound in the same window; the shutdown-versus- release window it targeted is pre-existing to every unbind path (driver_detach(), unbind_store(), the consumer recursion), so the re-probe, which is device_reprobe() deferred, shares it rather than introducing it, and the kernel-doc now says so (found by Sashiko AI review) - patch 3: record the bound driver's name beside its pointer and compare both, so a freed struct device_driver address the allocator later hands to a different driver is not mistaken for the original binding; this closes the recorded-identity race without a module reference (found by Sashiko AI review) - patch 4: wait out a flash in flight in .remove() as ->shutdown() already did, before dsa_unregister_switch() frees the user netdevs. The instance lock a flash holds does not cover that free, which the teardown reaches first, so an unbind racing a flash could touch a freed netdev; both paths now take the lock up front and announce the wait with dev_info() so the up-to-a-minute pause is not mistaken for a hang (found by Sashiko AI review) - patch 4: report success for a firmware read blocked by a flash (-EBUSY), not only the teardown -ENODEV, in port_mdb_add() and port_mdb_del(), so an MDB change racing a flash does not fail and leave the entry linked; clear the SB PDI ADDR and DATA latches with 0 rather than a CTRL mode value that only happens to be 0; log that a rebind is needed if the re-probe cannot be scheduled after the new firmware is already running; correct the block_host/skip_teardown kernel-doc to the policy the code implements (all found by Sashiko AI review) - patch 5: bypass the firmware-version gate in mxl862xx_phylink_get_caps() in rescue mode, so a SerDes CPU port does not get an empty supported_interfaces mask and fail phylink_create(), matching the mac_select_pcs() bypass; the @rescue_failed kernel-doc and the setup log message no longer prescribe a power cycle for every cause, whose per-cause remedy is in the documentation (found by Sashiko AI review) - Andrew's Reviewed-by is kept on patches 1, 2 and 6, unchanged this round - Jakub asked whether the mode transitions could be driven from userspace through devlink reload [22]. The deferred re-probe is kept: DSA has no reload plumbing, and adding it would need a new DSA-core path to reinitialise a switch while its devlink instance stays alive, whereas the re-probe reuses the existing unbind/register path, and the helper it uses is wanted regardless to replace the open-coded self-reprobe in iwlwifi, hci_h5 and btintel_pcie - patch 5: the interrupted-download drain's chunk == 0 error, which the review reads as mis-reporting a firmware that booted mid-drain, was reproduced on hardware and does not arise. The loader verifies the received image by CRC, so the zero-filled drain never reconstructs a bootable image; it returns the loader to its ready state instead, and the chunk == 0 error only fires for a genuinely wedged loader, where it is correct (found by Sashiko AI review) - the review's remaining findings are not acted on: patch 1 stays on the unconditional shared ops table as Andrew asked in v9; the SB PDI register offsets and the status words patch 5 classifies the switch by are a fixed hardware property configured by MCUboot and a stable contract the firmware release QA gates enforce, probe detection being part of the gate, so neither a relocated window nor a status value that breaks classification is a configuration that ships; and the background drain deliberately stays off a freezable workqueue, a suspend or bus error there being a rare event on an exceptional path that a rebind recovers - the changes to patches 3 to 5 were written with an LLM coding assistant working from the Sashiko findings and a local review, and reviewed by hand; the Assisted-by tags on patches 3 to 5 record this Changes since v15 [20]: - patch 1: the commit message no longer puts a notification pair around a missing firmware file; the core fails that before the pair, which wraps the call into the trampoline only - patch 3: take no lock in the caller's context and drop the parent snapshot, refusing buses that need the parent lock instead. The caller-context device lock inverted against the devlink instance lock, which a flash holds across the call, and against the synchronous cancel of the rescue self-heal work; a pinned parent can also be freed by device_move(). Only usb_bus_type sets need_parent_lock, and the conversions posted separately re-probe PCI and serdev devices, so none of them is refused. Patches 4 and 5 need no change of their own for either inversion (all found by Sashiko AI review) - patch 3: abandon the release when probing gets blocked while __device_release_driver() has the locks dropped to unbind busy consumer links, closing the one window where a ->remove() could follow a ->shutdown(). Earlier versions called it pre-existing, which it is, but a deferred re-probe is the one unbind that may be abandoned, so it takes a flag the other callers do not (found by Sashiko AI review) - patch 3: the commit message now describes what this patch changes rather than bugs in the drivers it does not convert (found by Sashiko AI review) - patch 3: the commit message no longer claims the detach is synchronised with device_shutdown() in every case. Probing is blocked only once wait_for_device_probe() has returned, so a re-probe already past the check detaches the device, which then runs ->remove() in place of ->shutdown() - patch 4: a firmware write blocked by a running flash reports success instead of -EBUSY. Tearing a bridge down over a flash, as a reboot does, made port_vlan_del() fail; the bridge then leaves the VLAN on its list and __vlan_group_free() warns and frees the group with the entries still linked - patch 4: ->shutdown() now waits for a flash in flight and refuses one requested after it, so a reboot cannot cut the image in half. The devlink core already serialises .remove() through the instance lock; ->shutdown() does not go through devlink and takes that lock itself - patch 4: the -EBUSY extack of a pending reprobe states a fact rather than advising a retry, and the comment on mxl862xx_read_chip_id() drops the rescue-mode cache that only patch 5 creates - patch 4: select CRC32, which nothing else selects for the image checksum validation (found by Sashiko AI review) - patch 5: treat a byte count outliving the settle step as a busy loader and wait out an erase a dead session left running before draining, so a host that died during the loader's erase no longer fails probe with the -ETIMEDOUT this series exists to avoid; drop the loader's clean ready state with the cached identity when a flash fails; state facts only in the extack of a failed recovery (all found by Sashiko AI review) - patch 5: the commit message and the comments no longer say the clause-45 API floods the log with CRC errors when no firmware answers. The mailbox commands run into their timeouts instead, and testing the stuck-in-MCUboot paths produces no such message; what probing SB PDI first saves is the ten seconds of polling and the -ETIMEDOUT that ends probe - patch 5: move the rescue branch of mxl862xx_setup() into a function of its own, which brings its messages back inside 100 columns; state a fact in the -EBUSY extack of a running recovery; trim the drain comment, whose protocol detail is in the file header, and order its declarations - patch 6: document that the ports come back down, which recovery failure needs a power cycle and which a rebind, and what a re-probe that cannot be scheduled leaves behind (found by Sashiko AI review) - patch 6: document that a reboot waits for a running flash, that a flash requested after it is refused, and that a bus error ends the download recovery for good; title-case the "Flash Update" heading like the other devlink driver documents - Andrew's Reviewed-by is kept on patches 1, 2 and 6: patch 1 gained commit message text only and patch 6 the documentation sentences above. It is dropped on patch 4, which gained the shutdown serialisation - patch 3 has drawn no driver-core reply here or in its three standalone postings [11][14]. The helper is no longer an RFC: Hans de Goede reviewed and tested it there [15][16], and the conversions of the open-coded users in iwlwifi, hci_h5 and btintel_pcie follow once this series is merged - the remaining findings of that review are not acted on: patch 1 is asked once more to install .flash_update only for drivers implementing the callback, as v4 did, and stays unconditional as Andrew asked in v9; the empty supported_interfaces a rescue-mode probe leaves for the quad-mode sub-interfaces can only reach phylink through a CPU port on one of them, which the chip does not support; and the driver pointer patch 3 records could in theory match a different driver loaded at the same address within the delay, at the cost of one spurious re-probe - the changes to patches 1 and 3 to 6, the commit message of patch 1 included, were written with an LLM coding assistant working from the Sashiko findings and from a local review of the posted series, and reviewed by hand; the Assisted-by tags on patches 3 to 6 record this Changes since v14 [19]: - patch 3: skip the detach while probing is blocked, which device_shutdown() does before its walk reaches any device, instead of a per-device flag set only once the walk arrives; validate the device and snapshot the parent, its locking requirement and the bound driver under the device lock; keep -EPROBE_DEFER out of the re-probe error path so it cannot overwrite a deferred probe reason (all found by Sashiko AI review) - patch 4: treat the closing END write as advisory, since the loader has verified the image by then, and admit only the flash task's own firmware reads past block_host instead of every read from every context (found by Sashiko AI review) - patch 5: classify a status register left in the download handshake as a loader needing a power cycle rather than as running firmware, give the loader one step to publish its next state before ruling it out after a failed clause-45 wait, abort the drain polls as soon as teardown asks for it instead of stalling unbind for up to 17 s, and pair the rescue_mode accesses with WRITE_ONCE()/READ_ONCE() (all found by Sashiko AI review) - patch 6: asic.rev comes from the CHIP ID registers as well, and -EIO means the driver gave up on the recovery, which may need a rebind rather than a power cycle (found by Sashiko AI review) - Andrew's Reviewed-by is kept on patches 1, 2, 4 and 6; the changes to 4 and 6 are the two small ones above and a documentation reword - the same review asks again whether patch 1 should install .flash_update only for drivers implementing the callback, as v4 did; it stays unconditional as Andrew asked in v9. Its remaining findings are not acted on either: the pre-existing window in __device_release_driver() where a ->shutdown() can interleave with a release, which every unbind path shares; the recorded driver pointer, which an unbind and rebind within the delay can match again at the cost of one spurious re-probe; and the get_stats64() re-arm race in remove(), which predates this series and is fixed separately for net - the changes to patches 3 to 6 were written with an LLM coding assistant working from the Sashiko findings and reviewed by hand; the Assisted-by tags on those patches record this Changes since v13 [18]: - patch 5: initialise the SerDes state once mxl862xx_wait_ready() has cached the firmware version, still before the rescue-mode early return, so PCS setup can depend on the running firmware - picked up Andrew's Reviewed-by on patches 1 and 4; the one on patch 5 is not carried as that patch changed - the Sashiko review of v13 repeats the ABA finding on patch 3 dismissed in v13 and marks the two __device_release_driver() windows and the get_stats64() re-arm race as pre-existing; no change - the change to patch 5 was written with an LLM coding assistant working from a report against a downstream tree and reviewed by hand; the Assisted-by tag on that patch records this Changes since v12 [13]: - v12 went out just as net-next closed for the 7.3 merge window. The helper of patch 3 was then posted on its own, with conversions of the existing open-coded users in iwlwifi, hci_h5 and btintel_pcie, most recently as v3 [14], which Greg's patch bot deferred past the merge window; Hans de Goede reviewed and tested the helper and the hci_h5 conversion there on RTL8723BS hardware [15][16]. The Sashiko review of that posting found the same issues in the helper as the review of v12 did, so patch 3 here supersedes the helper patch of that series; once this series is merged, the conversions follow as patches for bluetooth-next and wireless-next. On the userspace side, fwupd's devlink plugin has meanwhile gained the quirks for these switches [17] - patch 3: queue the re-probe on system_freezable_wq, so one pending across system suspend runs after resume instead of detaching a suspended device or racing its late suspend callbacks; record at scheduling time whether the parent needs locking instead of reading dev->bus, which may be gone with its module once the device was unregistered; let __device_release_driver() report whether it released the driver, so an administrative unbind that wins the race inside the device links loop is not undone by the re-attach (all found by Sashiko AI review); use dev_err_probe() for the re-probe error path (Hans de Goede) - patch 4: stop the stats poll with disable_delayed_work_sync() in the flash path, so a racing get_stats64() re-arm is a no-op, and drop the early return in the work function; the v12 reordering of remove() is gone as well, since the get_stats64() race it addressed predates this series and needs a fix of its own (found by Sashiko AI review) - two further findings of the same review are not acted on: the recorded driver pointer could in theory match a different driver loaded at the same address within the delay, which would cost that driver one spurious detach and re-probe; and the final put_device() from the work could call a release() whose module was unloaded in the meantime, which is the same hazard every asynchronous device reference in the core carries, the async probe helper included - the changes to patches 3 and 4 were written with an LLM coding assistant working from the Sashiko findings and reviewed by hand; the Assisted-by tags on those two patches record this Changes since v11 [12]: - patch 3: pin the parent device across the deferred re-probe and take the parent lock across device_attach() on buses that need it; a reference on the child alone left a freed dev->parent dereferenced under __device_driver_lock() (found by Sashiko AI review) - patch 4: cancel the stats poll after dsa_unregister_switch() so a racing get_stats64() cannot re-arm it against freed priv; and keep the host blocked for writes across the post-flash readiness poll, letting only the flash path's own reads reach the new firmware (found by Sashiko AI review) Changes since v10 [10]: - new patch 3: driver core: add device_schedule_reprobe(), as posted in the RFC [11], used to schedule the post-flash and post-drain re-probe with device_schedule_reprobe() instead of a driver-owned work item. - the dsa_switch allocation returns to devres. Keeping it out of devres only defused the remaining check-vs-detach window, which the core helper closes outright - scheduling the re-probe is now the one step that can fail after the switch was flashed, since the helper allocates its work item internally and v10's allocate-up-front dance is no longer possible. An -ENOMEM there is a system-wide condition no driver-level message or recovery improves, so flash_update just returns it (unbind and rebind reinitialises the driver), and a drain whose hand-off fails still marks recovery failed so devlink does not keep promising a retry Changes since v9 [9]: - Harmonised the SB PDI timeouts. The verify wait is now one constant shared by the flash and drain paths (15 s), the 1-byte mailbox step is another (2 s) used by the drain and by both detection waits, and the per-slice write budget drops from 120 s to 60 s. The last-slice flush, which cannot see the boundary between programming and verifying, gets the sum of the two. - The post-flash and post-drain reprobe no longer detaches a device that has been shut down or unbound, and the dsa_switch is allocated outside devres so a lost race cannot leave dsa_switch_find() reading freed memory. This was a live bug in v9's patch 3 as well, reachable by rebooting within 500 ms of a flash. - A failed reprobe hand-off after a successful drain now logs and fails flashing outright, instead of leaving devlink answering "retry shortly" for good. Dropped heal_lock and mxl862xx_stop_work() with it: the lock only made the flag test and the queueing atomic, which is not the guarantee the comment claimed, and the reprobe's own check is what actually decides now. - A loader that publishes READY but never services the register-read challenge now reports -ENXIO rather than propagating -ETIMEDOUT, and the documented return sets of mxl862xx_rescue_mode_detect() and mxl862xx_rescue_drain_finish() match what the code returns. - Commit message for patch 4 no longer claims a running firmware is "left untouched" (the presence probe writes two mailbox scratch registers, inert to a firmware that does not read them), says that an SB PDI window away from the OTP reset offsets also yields -ENODEV, and describes the -ENXIO outcome above. - Commented why STAT == 0 during a drain is unambiguous: by the r_remain == 0 rule the loader cannot be both inside the receive loop asking for a chunk and publishing a verdict. - Documented that a switch power cycled on its own needs the driver unbound and rebound before a failed recovery is re-examined. 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/ak0J-HgzMRea53om@makrotopia.org/ [2] https://lore.kernel.org/all/cover.1783988826.git.daniel@makrotopia.org/ [3] https://lore.kernel.org/all/cover.1784513694.git.daniel@makrotopia.org/ [4] https://lore.kernel.org/all/cover.1784665017.git.daniel@makrotopia.org/ [5] https://lore.kernel.org/all/cover.1784945329.git.daniel@makrotopia.org/ [6] https://lore.kernel.org/all/cover.1785119999.git.daniel@makrotopia.org/ [7] https://lore.kernel.org/all/cover.1785274610.git.daniel@makrotopia.org/ [8] https://lore.kernel.org/all/cover.1785389905.git.daniel@makrotopia.org/ [9] https://lore.kernel.org/all/cover.1785728574.git.daniel@makrotopia.org/ [10] https://lore.kernel.org/all/cover.1786294649.git.daniel@makrotopia.org/ [11] https://lore.kernel.org/all/anpxFdwNxk0XwPjQ@makrotopia.org/ [12] https://lore.kernel.org/all/cover.1786773971.git.daniel@makrotopia.org/ [13] https://lore.kernel.org/all/cover.1786922210.git.daniel@makrotopia.org/ [14] https://lore.kernel.org/all/cover.1787281239.git.daniel@makrotopia.org/ [15] https://lore.kernel.org/all/c461462f-de0b-43e8-ac9e-541013f5f8da@oss.qualcomm.com/ [16] https://lore.kernel.org/all/7ffe0c2e-0742-488a-ab6c-1dc2fabc049c@oss.qualcomm.com/ [17] https://github.com/fwupd/fwupd/commit/e50c9e5ab39d31242e664efbbf441fd46d15a0cd [18] https://lore.kernel.org/all/cover.1788783126.git.daniel@makrotopia.org/ [19] https://lore.kernel.org/all/cover.1788976064.git.daniel@makrotopia.org/ [20] https://lore.kernel.org/all/cover.1789175618.git.daniel@makrotopia.org/ [21] https://lore.kernel.org/all/cover.1789477568.git.daniel@makrotopia.org/ [22] https://lore.kernel.org/all/aq5QDmuQnqQTudca@makrotopia.org/ Daniel Golle (6): net: dsa: add devlink flash_update callback to dsa_switch_ops net: dsa: mxl862xx: add SMDIO clause-22 register access driver core: add device_schedule_reprobe() 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 | 91 ++ MAINTAINERS | 1 + drivers/base/dd.c | 115 ++ drivers/net/dsa/mxl862xx/Kconfig | 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 | 1145 +++++++++++++++++ drivers/net/dsa/mxl862xx/mxl862xx-fw.h | 21 + drivers/net/dsa/mxl862xx/mxl862xx-host.c | 68 + drivers/net/dsa/mxl862xx/mxl862xx-host.h | 2 + drivers/net/dsa/mxl862xx/mxl862xx-phylink.c | 9 +- drivers/net/dsa/mxl862xx/mxl862xx.c | 180 ++- drivers/net/dsa/mxl862xx/mxl862xx.h | 46 + include/linux/device.h | 2 + include/net/dsa.h | 3 + net/dsa/devlink.c | 13 + 18 files changed, 1697 insertions(+), 15 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: 944ae66642b726bd6b25ae71b1e9ff88a0e0bdb0 prerequisite-patch-id: 0000000000000000000000000000000000000000 -- 2.55.0 ^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH net-next v17 5/6] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode 2026-09-23 2:33 [PATCH net-next v17 0/6] net: dsa: mxl862xx: devlink flash and rescue Daniel Golle @ 2026-09-23 2:36 ` Daniel Golle 0 siblings, 0 replies; 2+ messages in thread From: Daniel Golle @ 2026-09-23 2:36 UTC (permalink / raw) To: Jiri Pirko, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet, Shuah Khan, Randy Dunlap, Daniel Golle, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Andrew Lunn, Vladimir Oltean, Russell King, netdev, linux-doc, linux-kernel, driver-core A broken or interrupted firmware image, or the sticky rescue bit, keeps the switch in its MCUboot loader, which exposes only the clause-22 SMDIO download interface. The clause-45 firmware API never comes up there, so an ordinary probe would spend its whole timeout on a mailbox nothing answers before failing. Detect the loader over SB PDI at setup, ahead of any clause-45 access, and in rescue mode register the switch without user interfaces so devlink stays available to reflash it: the user ports fail setup and the DSA core re-registers them as unused, while the CPU port comes up on its fixed link. devlink dev info then reports the firmware version as "0.0.0", which no released firmware carries, so fwupd offers every release as an upgrade and recovers the switch through the regular flash flow. An interrupted download can leave the loader wedged mid-payload with an outstanding byte count. A background work item off the devlink flash path drains it back to a clean ready state by feeding that count one byte at a time, since a larger step could underflow the loader's counter and wedge it until a power cycle, then re-probes so the probe-time detection reclassifies the switch. Until it finishes, devlink dev info reports no version and devlink dev flash returns -EBUSY. Assisted-by: LLM Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- v17: - bypass the firmware-version gate in mxl862xx_phylink_get_caps() in rescue mode, so a SerDes CPU port does not get an empty supported_interfaces mask and fail phylink_create(); the sibling mac_select_pcs() bypass was already there (found by Sashiko AI review) - extend the port_mdb_add()/port_mdb_del() flash-window tolerance to rescue mode as well (found by Sashiko AI review) - the @rescue_failed kernel-doc and the mxl862xx_setup_rescue() log message no longer prescribe a power cycle for every cause; the per-cause remedy is in the documentation (found by Sashiko AI review) v16: - drop the claim that the clause-45 API floods the log with CRC errors when no firmware answers, here and in the comments. The mailbox commands run into their timeouts instead, and testing the stuck-in-MCUboot paths produces no such message. What probing SB PDI first saves is the ten seconds of polling and the -ETIMEDOUT that ends probe - move the rescue-mode branch of mxl862xx_setup() into a function of its own, which brings its messages back inside 100 columns - state a fact in the -EBUSY extack of a running recovery, dropping the retry advice - trim the comment on mxl862xx_rescue_drain(), whose protocol detail is in the file header already, and order its declarations longest line first - treat a byte count that outlives the settle step as a busy loader rather than a running firmware, and wait out an erase from the dead session before draining, so a host that died during the loader's erase no longer fails probe with the -ETIMEDOUT this patch exists to avoid (found by Sashiko AI review) - poll the status register every 10 ms rather than every 50 us for the waits now measured in minutes - drop the loader's clean ready state along with the cached identity when a flash fails, so devlink dev info stops reporting 0.0.0, which means "ready to accept an image", for a loader left mid transfer (found by Sashiko AI review) - state facts in the extack for a failed recovery; the remedies, which differ per cause, are in the documentation (found by Sashiko AI review) - name -ECANCELED in the documented return sets that can produce it, and the opening handshake among the detection outcomes in the commit message (found by Sashiko AI review) v15: - classify a status register left in the download handshake as a loader needing a power cycle, rather than as a running firmware, which made probe fail with the CRC-error storm this patch avoids (found by Sashiko AI review) - give the loader one step to publish its next state when detection runs after a failed clause-45 wait, so the count of a chunk it is still programming is not read as a firmware status word (found by Sashiko AI review) - abort the drain polls as soon as teardown asks for it, instead of holding up unbind and shutdown for up to 17 s (found by Sashiko AI review) - pair the rescue_mode accesses with WRITE_ONCE()/READ_ONCE(), like the sibling rescue flags (found by Sashiko AI review) v14: - initialise the SerDes state once mxl862xx_wait_ready() has cached the firmware version, still before the rescue-mode early return, so PCS setup can depend on the running firmware v13: no changes v12: no changes v11: - schedule the post-drain re-probe with device_schedule_reprobe() too, instead of a driver-owned work item - a drain whose re-probe hand-off fails still marks recovery failed, so devlink does not keep promising a retry v10: - share the SB PDI timeouts with the flash path: one constant for the verify wait (15 s) and one for a single 1-byte mailbox step (2 s), the latter also replacing the separate detection timeout. The last-slice flush gets the sum of the write and verify budgets, since it cannot see the boundary between programming and verifying (found by Sashiko AI review) - report a reprobe hand-off that cannot be set up after a successful drain, instead of leaving devlink answering "retry shortly" for good for a loader sitting at a clean READY (found by Sashiko AI review) - drop heal_lock and mxl862xx_stop_work() with it: making the flag test and the queueing atomic was never the guarantee its comment claimed, and the reprobe now decides for itself whether it may still run (found by Sashiko AI review) - return -ENXIO rather than a propagated -ETIMEDOUT when the loader never re-arms READY for the register-read challenge, and correct the documented return sets of mxl862xx_rescue_mode_detect() and mxl862xx_rescue_drain_finish() (found by Sashiko AI review) - explain why STAT == 0 during a drain is unambiguous: by the r_remain == 0 rule the loader cannot be both inside the receive loop asking for a chunk and publishing a verdict (found by Sashiko AI review) - commit message: a running firmware is not "left untouched", the presence probe writes two mailbox scratch registers which are inert to it; an SB PDI window away from the OTP reset offsets also yields -ENODEV; and describe the -ENXIO outcome for a READY loader that never services the challenge (found by Sashiko AI review) 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 | 491 +++++++++++++++++++- drivers/net/dsa/mxl862xx/mxl862xx-fw.h | 3 + drivers/net/dsa/mxl862xx/mxl862xx-host.c | 8 + drivers/net/dsa/mxl862xx/mxl862xx-phylink.c | 9 +- drivers/net/dsa/mxl862xx/mxl862xx.c | 111 ++++- drivers/net/dsa/mxl862xx/mxl862xx.h | 21 + 6 files changed, 613 insertions(+), 30 deletions(-) diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-fw.c b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c index 7a506d110c8d..0761132120b1 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx-fw.c +++ b/drivers/net/dsa/mxl862xx/mxl862xx-fw.c @@ -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,13 @@ #define MXL862XX_FW_WRITE_TIMEOUT_MS 60000 #define MXL862XX_FW_REBOOT_DELAY_MS 5000 #define MXL862XX_FW_REPROBE_DELAY_MS 500 +/* One loader mailbox step: program a 1-byte chunk or service a command */ +#define MXL862XX_SB_PDI_STEP_MS 2000 +/* Covers the loader's END wait, verification and the reset into READY */ +#define MXL862XX_SB_PDI_VERIFY_MS 15000 +/* STAT poll intervals: a mailbox step is quick, an erase is not */ +#define MXL862XX_SB_PDI_POLL_US 50 +#define MXL862XX_SB_PDI_SLOW_POLL_US 10000 static int mxl862xx_sb_pdi_reset(struct mxl862xx_priv *priv) { @@ -192,7 +241,8 @@ static int mxl862xx_sb_pdi_flush_last(struct mxl862xx_priv *priv, ret = read_poll_timeout(mxl862xx_smdio_read, val, val < 0 || (u16)val != (u16)data_written, - 10000, MXL862XX_FW_WRITE_TIMEOUT_MS * 1000, + 10000, (MXL862XX_FW_WRITE_TIMEOUT_MS + + MXL862XX_SB_PDI_VERIFY_MS) * 1000, false, priv, MXL862XX_SB_PDI_STAT); if (val < 0) return val; @@ -218,6 +268,379 @@ static void mxl862xx_flash_notify(struct devlink *dl, const char *status, devlink_flash_update_status_notify(dl, status, NULL, done, total); } +/* Byte-count of each chunk fed to the loader during drain. It MUST be 1: the + * loader only lets us observe "counter == 0", never "counter < step", so any + * step > 1 can subtract past zero, underflow the 32-bit counter and wedge the + * loader for ~2^32 more bytes (a state only a power cycle clears). Stepping by + * 1 walks the counter through every value and is guaranteed to land on zero + * whatever its (possibly odd) start. A 1-byte chunk is a path the loader + * already handles: the normal transfer ends with a single trailing byte for + * odd-sized images (see Step 6). + */ +#define MXL862XX_DRAIN_CHUNK_BYTES 1 + +/* 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, or + * -ECANCELED once teardown asks the caller to stop. On timeout the value 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. + * + * STAT 0 is unambiguous here even though it is also the "image verified" + * verdict: by the r_remain == 0 rule the loader leaves the receive loop the + * moment the counter reaches zero, so it is never both inside the loop asking + * for a chunk and publishing a verdict. Once it has left, the next STAT write + * is a command rather than a count, so feeding one more chunk after a verdict + * cannot underflow anything either. + */ +static int mxl862xx_sb_pdi_poll_drain(struct mxl862xx_priv *priv, + unsigned long sleep_us, + unsigned long timeout_ms) +{ + int val; + + read_poll_timeout(mxl862xx_smdio_read, val, + val < 0 || (u16)val == MXL862XX_SB_PDI_READY || + (u16)val == 0 || + test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags), + sleep_us, timeout_ms * 1000, false, + priv, MXL862XX_SB_PDI_STAT); + if (val < 0) + return val; + if (test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags)) + return -ECANCELED; + 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. Before the first chunk it may also still be erasing + * for the session that died, which no verify window covers. Wait that out -- + * STAT cannot tell the cases apart, and guessing would mean writing END into a + * live receive loop. + * + * Return: 0 once the loader has left the loop, -EAGAIN if it asks for another + * chunk after all, -EIO for a loader still holding the count when the window + * expires, -ECANCELED on teardown, or an SMDIO bus error. + */ +static int mxl862xx_rescue_drain_finish(struct mxl862xx_priv *priv, u32 chunk) +{ + struct device *dev = &priv->mdiodev->dev; + int stat; + + if (chunk) + stat = mxl862xx_sb_pdi_poll_drain(priv, MXL862XX_SB_PDI_POLL_US, + MXL862XX_SB_PDI_VERIFY_MS); + else + stat = mxl862xx_sb_pdi_poll_drain(priv, + MXL862XX_SB_PDI_SLOW_POLL_US, + MXL862XX_FW_ERASE_TIMEOUT_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) { + if (!chunk) { + dev_err(dev, + "flash: loader still busy after the erase window\n"); + return -EIO; + } + /* 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; +} + +/* Walk the loader's receive counter to zero (see the header): the image size + * died with the host, and a chunk larger than what is outstanding underflows + * the counter, so only single bytes are safe. Tens of minutes for a multi-MiB + * remainder. Returns 0 once the loader has left the receive loop, <0 on error; + * a counter an earlier oversized chunk underflowed needs a power cycle. + */ +static int mxl862xx_rescue_drain(struct mxl862xx_priv *priv) +{ + /* Bound: twice the loader's 16 MiB image cap, one byte per chunk. */ + u32 max_chunks = 2u * (16u << 20) / MXL862XX_DRAIN_CHUNK_BYTES; + struct device *dev = &priv->mdiodev->dev; + 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_SB_PDI_POLL_US, + MXL862XX_SB_PDI_STEP_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; + 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 core + * skips the re-probe on its own if the device is unbound first; the + * flag test only avoids scheduling one certain to be skipped. A + * failed hand-off leaves nothing to reclassify the switch, so mark + * recovery failed rather than promise a retry that cannot succeed. + */ + if (test_bit(MXL862XX_FLAG_WORK_STOPPED, &priv->flags)) + return; + + if (device_schedule_reprobe(dev, MXL862XX_FW_REPROBE_DELAY_MS)) + WRITE_ONCE(priv->rescue_failed, true); +} + +/* Detect MCUboot rescue mode over clause-22 SMDIO alone, so the caller can rule + * the loader out before any C45 API request (which only runs into its timeouts + * 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. + * - 0xf48f/0xf490: a download handshake nobody can finish; rescue, but only + * a power cycle gets the loader out of it. + * - other non-zero: running firmware, left unpoked. With @settle the loader + * gets one step to publish 0 or READY first, and a count outliving that is + * a busy loader, for a caller which has ruled a running firmware out. + * - 0: wedged receive loop; the 1-byte slice-advance then says whether it + * still needs draining or has just finished. + * + * The scratch write reaches a running firmware too, but lands in mailbox + * registers it does not read, so it is inert there. + * + * Return: MXL862XX_IN_RESCUE, MXL862XX_NOT_RESCUE, -ENODEV when the scratch + * write does not latch, which is a switch that does not answer at all or one + * whose SB PDI window is not at the offsets above, -EOPNOTSUPP for the + * flashless loop, -ENXIO for a READY loader whose mailbox fails the challenge, + * -ECANCELED when teardown interrupts a poll, or an SMDIO bus error. + */ +int mxl862xx_rescue_mode_detect(struct mxl862xx_priv *priv, bool settle) +{ + 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_SB_PDI_STEP_MS); + dat = mxl862xx_smdio_read(priv, MXL862XX_SB_PDI_DATA); + ret = mxl862xx_sb_pdi_reset(priv); + /* Never re-arming READY fails the challenge like any other + * unserviced command; report it as such rather than as a bus + * timeout the bus never saw. + */ + if (rb == -ETIMEDOUT) + rb = -ENXIO; + if (rb < 0) + return rb; + if (dat < 0) + return dat; + if (ret < 0) + return ret; + if ((u16)dat != MXL862XX_SB_PDI_RDREG_MARK) { + WRITE_ONCE(priv->rescue_ready, true); + return MXL862XX_IN_RESCUE; + } + /* READY but the marker is untouched, so nothing is servicing the + * mailbox. A firmware publishing 0xc55c as its status word looks + * exactly like this, and flashing one would be far worse than + * refusing to bind, so treat it as unusable. + */ + return -ENXIO; + } + + /* The download handshake lives in STAT too and outlives a mailbox + * reset, so an aborted transfer leaves the loader waiting for a + * header no later session can supply: only a power cycle clears it. + */ + if ((u16)stat == MXL862XX_SB_PDI_START || + (u16)stat == MXL862XX_SB_PDI_START + 1) { + WRITE_ONCE(priv->rescue_failed, true); + return MXL862XX_IN_RESCUE; + } + + /* Any other non-zero value is a running firmware, not a loader -- but + * the loader also holds the count of a chunk it is programming or + * erasing for, so let a caller that has ruled the firmware out wait a + * step for the next state; a count outliving that is that busy loader. + */ + if (stat) { + if (!settle) + return MXL862XX_NOT_RESCUE; + + stat = mxl862xx_sb_pdi_poll_drain(priv, MXL862XX_SB_PDI_POLL_US, + MXL862XX_SB_PDI_STEP_MS); + if (stat < 0) + return stat; + if (stat == MXL862XX_SB_PDI_READY) { + WRITE_ONCE(priv->rescue_ready, true); + return MXL862XX_IN_RESCUE; + } + if (stat) + return MXL862XX_IN_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_SB_PDI_POLL_US, + MXL862XX_SB_PDI_STEP_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; @@ -293,13 +716,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 (!READ_ONCE(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 */ @@ -486,6 +911,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 (READ_ONCE(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 @@ -564,9 +1008,27 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds, return ret; } - dev_info(ds->dev, "flash: running firmware %u.%u.%u\n", - priv->fw_version.major, priv->fw_version.minor, - priv->fw_version.revision); + /* 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"); + return -EIO; + } + + if (READ_ONCE(priv->rescue_mode) && !READ_ONCE(priv->rescue_ready)) { + NL_SET_ERR_MSG_MOD(extack, + "switch is recovering an interrupted download"); + return -EBUSY; + } + + if (READ_ONCE(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 @@ -613,6 +1075,7 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds, * readiness poll below read the freshly booted firmware. */ priv->flash_owner = current; + WRITE_ONCE(priv->rescue_mode, false); mutex_unlock(&priv->mdiodev->bus->mdio_lock); /* Refresh the cached versions so the flash update only @@ -629,11 +1092,13 @@ int mxl862xx_devlink_flash_update(struct dsa_switch *ds, if (ret) { /* The switch is in MCUboot with erased or partly written flash; * drop the cached identity so devlink dev info stops reporting - * the pre-flash version until the reprobe re-reads the truth. + * the pre-flash version until the reprobe re-reads the truth, + * and with it the loader's clean READY state. */ memset(&priv->fw_version, 0, sizeof(priv->fw_version)); priv->asic_id = 0; priv->asic_rev = 0; + WRITE_ONCE(priv->rescue_ready, false); } mutex_lock_nested(&priv->mdiodev->bus->mdio_lock, MDIO_MUTEX_NESTED); diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-fw.h b/drivers/net/dsa/mxl862xx/mxl862xx-fw.h index 15ed3a46bcfe..02e5a627e947 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx-fw.h +++ b/drivers/net/dsa/mxl862xx/mxl862xx-fw.h @@ -6,7 +6,10 @@ #include <net/dsa.h> struct mxl862xx_priv; +struct work_struct; +int mxl862xx_rescue_mode_detect(struct mxl862xx_priv *priv, bool settle); +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); diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-host.c b/drivers/net/dsa/mxl862xx/mxl862xx-host.c index 4b3956a518cf..694c22d2dd09 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx-host.c +++ b/drivers/net/dsa/mxl862xx/mxl862xx-host.c @@ -17,6 +17,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) @@ -347,6 +348,11 @@ int mxl862xx_api_wrap(struct mxl862xx_priv *priv, u16 cmd, void *_data, goto out; } + if (priv->rescue_mode) { + ret = -ENODEV; + goto out; + } + /* During the post-flash readiness poll block_host stays set, but the * flash path's own firmware version reads must reach the new image; * host writes stay blocked so stale resource IDs cannot corrupt it. @@ -558,9 +564,11 @@ int mxl862xx_smdio_write(struct mxl862xx_priv *priv, u32 addr, u16 val) void mxl862xx_host_init(struct mxl862xx_priv *priv) { INIT_WORK(&priv->crc_err_work, mxl862xx_crc_err_work_fn); + INIT_WORK(&priv->rescue_heal_work, mxl862xx_rescue_heal_work_fn); } void mxl862xx_host_shutdown(struct mxl862xx_priv *priv) { cancel_work_sync(&priv->crc_err_work); + cancel_work_sync(&priv->rescue_heal_work); } diff --git a/drivers/net/dsa/mxl862xx/mxl862xx-phylink.c b/drivers/net/dsa/mxl862xx/mxl862xx-phylink.c index b689652aa9b9..df77bbf108d5 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx-phylink.c +++ b/drivers/net/dsa/mxl862xx/mxl862xx-phylink.c @@ -47,7 +47,12 @@ void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, fallthrough; case 10 ... 12: case 14 ... 16: - if (!MXL862XX_FW_VER_MIN(priv, 1, 0, 84)) + /* Rescue mode has no firmware version, so bypass the gate and + * advertise the full set; a CPU port on a quad sub-interface + * would otherwise get an empty mask and fail phylink_create(). + */ + if (!READ_ONCE(priv->rescue_mode) && + !MXL862XX_FW_VER_MIN(priv, 1, 0, 84)) break; __set_bit(PHY_INTERFACE_MODE_QSGMII, config->supported_interfaces); __set_bit(PHY_INTERFACE_MODE_10G_QXGMII, config->supported_interfaces); @@ -406,6 +411,8 @@ mxl862xx_phylink_mac_select_pcs(struct phylink_config *config, switch (port) { case 9 ... 16: + if (READ_ONCE(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"); diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.c b/drivers/net/dsa/mxl862xx/mxl862xx.c index 33a7cdb8edd3..911114579f10 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx.c +++ b/drivers/net/dsa/mxl862xx/mxl862xx.c @@ -667,27 +667,85 @@ static void mxl862xx_free_bridge(struct dsa_switch *ds, priv->bridges[bridge->num] = 0; } +static void mxl862xx_setup_rescue(struct dsa_switch *ds) +{ + struct mxl862xx_priv *priv = ds->priv; + + if (priv->rescue_ready) { + dev_warn(ds->dev, + "switch in MCUboot rescue mode, use devlink to flash new firmware\n"); + return; + } + + if (priv->rescue_failed) { + dev_warn(ds->dev, + "switch in MCUboot, download recovery gave up; see Documentation/networking/devlink/mxl862xx.rst\n"); + return; + } + + /* 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); +} + static int mxl862xx_setup(struct dsa_switch *ds) { struct mxl862xx_priv *priv = ds->priv; int n_user_ports = 0, max_vlans; int ingress_finals, vid_rules; struct dsa_port *dp; - int ret, i; + int ret, i, rescue; - ret = mxl862xx_reset(priv); - if (ret) - return ret; + /* Detect the loader over SB PDI first: it needs no firmware, unlike the + * C45 API (mxl862xx_reset/wait_ready), which spends its whole 10 s + * window on a mailbox nobody answers. Touch C45 only once rescue is + * ruled out. + */ + rescue = mxl862xx_rescue_mode_detect(priv, false); + if (rescue < 0) { + dev_err(ds->dev, "switch state detection failed: %pe\n", + ERR_PTR(rescue)); + return rescue; + } - ret = mxl862xx_wait_ready(ds); - if (ret) - return ret; + 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, true); + 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; + /* Software-only SerDes state, needed before anything can reach phylink, + * including a rescue-mode flash clearing rescue_mode ahead of reprobe. + */ mutex_init(&priv->serdes_lock); for (i = 0; i < ARRAY_SIZE(priv->serdes_ports); i++) mxl862xx_setup_pcs(priv, &priv->serdes_ports[i], i + MXL862XX_FIRST_SERDES_PORT); + if (priv->rescue_mode) { + mxl862xx_setup_rescue(ds); + return 0; + } + /* Calculate Extended VLAN block sizes. * With VLAN Filter handling VID membership checks: * Ingress: only final catchall rules (PVID insertion, 802.1Q @@ -778,11 +836,21 @@ static int mxl862xx_port_state(struct dsa_switch *ds, int port, bool enable) static int mxl862xx_port_enable(struct dsa_switch *ds, int port, struct phy_device *phydev) { + struct mxl862xx_priv *priv = ds->priv; + + if (READ_ONCE(priv->rescue_mode)) + return 0; + return mxl862xx_port_state(ds, port, true); } static void mxl862xx_port_disable(struct dsa_switch *ds, int port) { + struct mxl862xx_priv *priv = ds->priv; + + if (READ_ONCE(priv->rescue_mode)) + return; + if (mxl862xx_port_state(ds, port, false)) dev_err(ds->dev, "failed to disable port %d\n", port); } @@ -1400,6 +1468,17 @@ static int mxl862xx_port_setup(struct dsa_switch *ds, int port) bool is_cpu_port = dsa_port_is_cpu(dp); int ret; + if (dsa_port_is_dsa(dp)) { + dev_err(ds->dev, "port %d: DSA links not supported\n", port); + return -EOPNOTSUPP; + } + + /* DSA reinits failed user ports as unused; shared ports must + * succeed for the tree to register. + */ + if (READ_ONCE(priv->rescue_mode)) + return dsa_port_is_user(dp) ? -ENODEV : 0; + ret = mxl862xx_port_state(ds, port, false); if (ret) return ret; @@ -1409,11 +1488,6 @@ static int mxl862xx_port_setup(struct dsa_switch *ds, int port) if (dsa_port_is_unused(dp)) return 0; - if (dsa_port_is_dsa(dp)) { - dev_err(ds->dev, "port %d: DSA links not supported\n", port); - return -EOPNOTSUPP; - } - ret = mxl862xx_configure_sp_tag_proto(ds, port, is_cpu_port); if (ret) return ret; @@ -1603,7 +1677,8 @@ static int mxl862xx_port_mdb_add(struct dsa_switch *ds, int port, * rebuilds the configuration. See mxl862xx_port_mdb_del(). */ if ((ret == -EBUSY && priv->block_host) || - (ret == -ENODEV && priv->skip_teardown)) + (ret == -ENODEV && + (priv->skip_teardown || READ_ONCE(priv->rescue_mode)))) return 0; if (ret) return ret; @@ -1642,12 +1717,13 @@ static int mxl862xx_port_mdb_del(struct dsa_switch *ds, int port, ether_addr_copy(qparam.mac, mdb->addr); ret = MXL862XX_API_READ(priv, MXL862XX_MAC_TABLEENTRYQUERY, qparam); - /* A flash blocks the API (-EBUSY) and its teardown drops the MAC - * table (-ENODEV); a delete then has nothing to do. Outside these, + /* A flash blocks the API (-EBUSY); its teardown or MCUboot drops the + * MAC table (-ENODEV); a delete then has nothing to do. Outside these, * both are bus errors and must be reported. */ if ((ret == -EBUSY && priv->block_host) || - (ret == -ENODEV && priv->skip_teardown)) + (ret == -ENODEV && + (priv->skip_teardown || READ_ONCE(priv->rescue_mode)))) return 0; if (ret) return ret; @@ -1705,6 +1781,9 @@ static void mxl862xx_port_stp_state_set(struct dsa_switch *ds, int port, struct mxl862xx_priv *priv = ds->priv; int ret; + if (READ_ONCE(priv->rescue_mode)) + return; + switch (state) { case BR_STATE_DISABLED: param.port_state = cpu_to_le32(MXL862XX_STP_PORT_STATE_DISABLE); diff --git a/drivers/net/dsa/mxl862xx/mxl862xx.h b/drivers/net/dsa/mxl862xx/mxl862xx.h index 054d0d35d3a5..70cab20a216a 100644 --- a/drivers/net/dsa/mxl862xx/mxl862xx.h +++ b/drivers/net/dsa/mxl862xx/mxl862xx.h @@ -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 @@ -336,6 +342,17 @@ struct mxl862xx_fw_version { * @shutting_down: set under the devlink instance lock once ->shutdown() * or .remove() has begun, so no flash starts while the * switch is going away + * @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 with WRITE_ONCE() under + * the MDIO bus lock for the benefit of mxl862xx_api_wrap(); + * readers outside that lock use READ_ONCE(). + * @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 loader cannot accept a flash; the + * remedy depends on the cause and is described in + * Documentation/networking/devlink/mxl862xx.rst + * @rescue_heal_work: background self-heal draining a wedged download to READY * @stats_work: periodic work item that polls RMON hardware counters * and accumulates them into 64-bit per-port stats */ @@ -343,6 +360,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; @@ -360,6 +378,9 @@ struct mxl862xx_priv { bool block_host; bool skip_teardown; bool shutting_down; + bool rescue_mode; + bool rescue_ready; + bool rescue_failed; struct delayed_work stats_work; }; -- 2.55.0 ^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-25 5:34 UTC | newest] Thread overview: 2+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-25 5:34 [PATCH net-next v17 5/6] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode netdev-bot+sashiko -- strict thread matches above, loose matches on Subject: below -- 2026-09-23 2:33 [PATCH net-next v17 0/6] net: dsa: mxl862xx: devlink flash and rescue Daniel Golle 2026-09-23 2:36 ` [PATCH net-next v17 5/6] net: dsa: mxl862xx: recover switch stuck in MCUboot rescue mode Daniel Golle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox