Re: [PATCH net-next v4 0/4] net: stmmac: preserve and restore the installed EST schedule

[James Hilliard]

On Thu, Oct 1, 2026 at 1:49 AM Lorenzo Bianconi
<lorenzo.bianconi@xxxxxxxxxxxxxxxx> wrote:
>
> > Continue Lorenzo Bianconi's EST replay series with v4 of the unmerged work:
> > https://lore.kernel.org/r/20260902-stmmac-est-reapply-after-open-v3-0-e72a6df5a7ef@xxxxxxxxxxxxxxxx
>
> Hi James,
>
> My plan is to continue working on this series (the code is available in [0]),

I compared your b4/stmmac-est-reapply-after-open branch at
e7a2792034e0 with the EST v4 I posted. The first two patches
(embedding the EST state and passing an explicit enable argument)
are equivalent apart from the additional sign-off.

The substantive differences are:

1. Preserving the installed schedule on replacement failure

Both versions validate the gate list before modifying the installed
schedule, so both preserve it when an entry fails validation.

Your version then overwrites priv->est before programming hardware.
If EST programming or preemption mapping fails, it disables EST.
However, TAPRIO keeps its previous schedule when the driver rejects
the replacement, leaving the driver and qdisc with different state.

My version builds the complete replacement in a separate object and
publishes it only after programming and preemption mapping succeed.
On failure, it retains the old configuration and attempts to restore
it, advancing the old base time by whole cycles against the current
PHC time.

2. Restoring EST before starting DMA

Your version starts DMA inside stmmac_hw_setup(), before PTP setup
and EST replay.

Mine moves DMA startup into the open and resume paths, after those
steps succeed. This keeps DMA stopped if EST restoration fails. I
wouldn't claim the earlier ordering necessarily transmits an ungated
packet, but the later startup gives us a stronger initialization
invariant.

3. Serializing EST across a PHC time adjustment

Your adjtime path releases est_lock between disabling EST and
adjusting the clock, then reacquires it for replay. It also ignores
the disable, clock-adjustment and replay errors.

Mine holds est_lock across that entire sequence, so a concurrent
TAPRIO operation cannot replace or destroy the schedule partway
through it. It checks the errors and attempts replay even after a
failed clock adjustment, while preserving the first error.

4. PTP prerequisite version

My series uses the newer PTP error-propagation fix already merged
as 8181678a92f0. Your branch carries an earlier version below the EST
series. The merged version also tracks PTP enablement and handles
platforms without a configured PTP clock rate. This is your newer
upstream work, rather than an additional change in my EST patches.

One limitation of my version is that rollback remains best-effort:
if restoring the old schedule also fails, it logs the failure but
retains the old software configuration. Hardware can therefore
still differ from the cache. Neither version guarantees recovery
from that double failure.

I'd suggest retaining the candidate-schedule handling, later DMA
startup and adjtime serialization when consolidating the series.

> I am just waiting other patches to be merged upstream before reposting in order
> to avoid any conflict. I guess it is correct to publicly ask before posting
> other people patches.

I guess I had assumed since it had been nearly a month since your
last series that you just hadn't gotten around to spinning a new one
and figured if you were still working on it then my changes might be
helpful either way since I had reworked a few things while working
on a series depends on these changes.

Feel free to take back over and incorporate my changes into your
next revision if/as appropriate.

> Moreover, this is thecnically a fix so this seires should targed net tree.

I think the fixes you may have been waiting on are in net-next, which
is the reason I did not target the net tree.

>
> Regards,
> Lorenzo
>
> [0] https://github.com/LorenzoBianconi/net-next/tree/b4/stmmac-est-reapply-after-open
>
> >
> > The PTP initialization fix from that series is already in the base.
> > Carry the remaining embedded-state and explicit-enable preparations,
> > and restore the installed schedule after open and resume reset.
> >
> > Build TAPRIO replacements separately so rejected updates cannot
> > corrupt the installed schedule. Rebase the previous schedule against
> > the current PHC when rolling back a failed hardware update, and keep
> > DMA stopped until replay succeeds. These changes do not depend on
> > the later datapath recovery series.
> >
> > Changes in v4:
> > - Omit the merged PTP initialization patch and adapt the remaining
> >   changes to probe-time PTP locking and current TAPRIO error handling.
> > - Separate schedule validation/rollback from open and resume replay.
> > - Reuse the base-time helper for replacement, PHC adjustment and
> >   rollback; propagate errors while preserving the installed cache.
> > - Start DMA only after the installed schedule has been restored.
> > - Link to v3: https://lore.kernel.org/r/20260902-stmmac-est-reapply-after-open-v3-0-e72a6df5a7ef@xxxxxxxxxxxxxxxx
> >
> > Changes in v3:
> > - Guard priv->est.enable update using est_lock mutex.
> > - Embed stmmac_est in stmmac_priv struct.
> > - Rework locking in tc_taprio_configure().
> > - Fix possible divided by zero crash in tc_taprio_configure().
> > - Add missing reconfiguration during stmmac_resume().
> > - Honor ptp error code in __stmmac_open() and stmmac_resume().
> > - Link to v2: https://lore.kernel.org/r/20260829-stmmac-est-reapply-after-open-v2-1-5e5ccb185e92@xxxxxxxxxxxxxxxx
> >
> > Changes in v2:
> > - Rename stmmac_est_reconfigure() in stmmac_setup_est().
> > - Rely on stmmac_setup_est() in tc_taprio_configure().
> > - Link to v1: https://lore.kernel.org/r/20260825-stmmac-est-reapply-after-open-v1-1-dfa80735e0a1@xxxxxxxxxxxxxxxx
> >
> > Assisted-by: Codex:gpt-6-astra
> > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> >
> > ---
> > James Hilliard (1):
> >       net: stmmac: preserve the installed EST schedule on errors
> >
> > Lorenzo Bianconi (3):
> >       net: stmmac: embed struct stmmac_est in stmmac_priv struct
> >       net: stmmac: pass the desired EST enable state to est_configure()
> >       net: stmmac: restore EST before starting DMA on open and resume
> >
> >  drivers/net/ethernet/stmicro/stmmac/hwif.h        |   2 +-
> >  drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   2 +-
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_est.c  |  47 ++++++-
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_est.h  |  13 ++
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  38 ++++--
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c  |  59 ++++-----
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c   | 142 +++++++++++-----------
> >  7 files changed, 176 insertions(+), 127 deletions(-)
> > ---
> > base-commit: 1631d79ae57dce2c5f88ad278307028638a8b4d9
> > change-id: 20260824-stmmac-est-reapply-after-open-181d70a15eb6
> >
> > Best regards,
> > --
> > James Hilliard <james.hilliard1@xxxxxxxxx>
> >