Re: [net-next,v2,2/3] net: ethernet: adi: Add ADIN1110 support

[Andrew Lunn]

On Wed, Jul 27, 2022 at 04:26:12PM +0300, alexandru.tachici@xxxxxxxxxx wrote:
> > > +static irqreturn_t adin1110_irq(int irq, void *p)
> > > +{
> > > +	struct adin1110_priv *priv = p;
> > > +	u32 status1;
> > > +	u32 val;
> > > +	int ret;
> > > +	int i;
> > > +
> > > +	mutex_lock(&priv->lock);
> > 
> > The MDIO bus operations are using the same lock. MDIO can be quite
> > slow. Do you really need mutual exclusion between MDIO and interrupts?
> > What exactly is this lock protecting?
> > 
> >   Andrew
> 
> Hi Andrew,
> 
> Thanks for all the help here.
> 
> With this lock I am mainly protecting SPI read/writes. The hardware doesn't expose the MDIO pins.
> In order to read/write a PHY reg, there has to be a SPI read/write to the device, the same
> line where the MAC is programmed and ethernet frames are sent/received, not very efficient I know.

Have you profiled adin1110_mdio_read()?

You could hold the mutex for the "write the clause 22 read command",
and then release it. And then take the mutex in
adin1110_read_mdio_acc(). That will allow for example the interrupt
handler to jump in between polls, etc.

If all you are protecting is SPI read/writes, i wonder if you even
need this mutex, the SPI core has one as well.

	Andrew