Re: [PATCH net] net: ibm: emac: fix MR0 write in emac_rx_enable()

[Andrew Lunn]

On Sat, Nov 07, 2026 at 04:07:04PM -0800, Rosen Penev wrote:
> On Sat, Nov 7, 2026 at 9:35 AM Andrew Lunn <andrew@xxxxxxx> wrote:
> >
> > On Thu, Nov 05, 2026 at 03:14:17PM -0800, Rosen Penev wrote:
> > > When an asynchronous RX disable is still in progress, emac_rx_enable()
> > > waits for MR0[RXI] with
> > >
> > >       while (!(r = in_be32(&p->mr0) & EMAC_MR0_RXI) && n)
> > >
> > > which assigns only the RXI bit to r, as & binds tighter than =. The
> > > following write of r | EMAC_MR0_RXE then clears every other MR0 bit,
> > > including TXE, and TX stays stopped until the watchdog resets the EMAC.
> > >
> > > This path is taken when RX is restarted after an RX descriptor error.
> > >
> > > Tested on a Cisco Meraki MX60.
> > >
> > > Fixes: 1d3bb996481e ("Device tree aware EMAC driver")
> > > Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> > > ---
> > >  drivers/net/ethernet/ibm/emac/core.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> > > index 640618c0a09e..b676b286e339 100644
> > > --- a/drivers/net/ethernet/ibm/emac/core.c
> > > +++ b/drivers/net/ethernet/ibm/emac/core.c
> > > @@ -249,7 +249,7 @@ static void emac_rx_enable(struct emac_instance *dev)
> > >               if (unlikely(!(r & EMAC_MR0_RXI))) {
> > >                       /* Wait if previous async disable is still in progress */
> > >                       int n = dev->stop_timeout;
> > > -                     while (!(r = in_be32(&p->mr0) & EMAC_MR0_RXI) && n) {
> > > +                     while (!((r = in_be32(&p->mr0)) & EMAC_MR0_RXI) && n) {
> > >                               udelay(1);
> > >                               --n;
> >
> > This sort of code should really make use of iopoll.h
> I agree. Is it appropriate for net?

Yes, that is fine.

     Andrew