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

[Rosen Penev]

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?
>
>         Andrew