[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CO1PR11MB50890C939694ABA5C4A75B05D6962@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Thu, 29 Aug 2024 17:50:50 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: Rosen Penev <rosenp@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"linux@...linux.org.uk" <linux@...linux.org.uk>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"o.rempel@...gutronix.de" <o.rempel@...gutronix.de>, "p.zabel@...gutronix.de"
<p.zabel@...gutronix.de>
Subject: RE: [PATCH net-next] net: ag71xx: disable napi interrupts during
probe
> -----Original Message-----
> From: Rosen Penev <rosenp@...il.com>
> Sent: Thursday, August 29, 2024 10:47 AM
> To: Keller, Jacob E <jacob.e.keller@...el.com>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; edumazet@...gle.com;
> kuba@...nel.org; pabeni@...hat.com; linux@...linux.org.uk; linux-
> kernel@...r.kernel.org; o.rempel@...gutronix.de; p.zabel@...gutronix.de
> Subject: Re: [PATCH net-next] net: ag71xx: disable napi interrupts during probe
>
> On Wed, Aug 28, 2024 at 2:05 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
> >
> >
> >
> > On 8/28/2024 1:41 PM, Rosen Penev wrote:
> > > From: Sven Eckelmann <sven@...fation.org>
> > >
> > > ag71xx_probe is registering ag71xx_interrupt as handler for gmac0/gmac1
> > > interrupts. The handler is trying to use napi_schedule to handle the
> > > processing of packets. But the netif_napi_add for this device is
> > > called a lot later in ag71xx_probe.
> > >
> > > It can therefore happen that a still running gmac0/gmac1 is triggering the
> > > interrupt handler with a bit from AG71XX_INT_POLL set in
> > > AG71XX_REG_INT_STATUS. The handler will then call napi_schedule and the
> > > napi code will crash the system because the ag->napi is not yet
> > > initialized.
> > >
> > > The gmcc0/gmac1 must be brought in a state in which it doesn't signal a
> > > AG71XX_INT_POLL related status bits as interrupt before registering the
> > > interrupt handler. ag71xx_hw_start will take care of re-initializing the
> > > AG71XX_REG_INT_ENABLE.
> > >
> > > Signed-off-by: Sven Eckelmann <sven@...fation.org>
> > > Signed-off-by: Rosen Penev <rosenp@...il.com>
> > > ---
> >
> > The description reads like a bug fix, so I would expect this to be
> > targeted to net and have a Fixes tag indicating what commit introduced
> > the issue, maybe:
> >
> > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> >
> > The change seems reasonable to me otherwise.
> OTOH there are currently no dual GMAC users upstream. Just single.
>
If that’s the case, updating the description to make that clear would help.
Powered by blists - more mailing lists