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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 27 Jul 2022 15:26:27 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     alexandru.tachici@...log.com
Cc:     d.michailidis@...gible.com, davem@...emloft.net,
        devicetree@...r.kernel.org, edumazet@...gle.com,
        geert+renesas@...der.be, geert@...ux-m68k.org,
        gerhard@...leder-embedded.com, joel@....id.au,
        krzysztof.kozlowski+dt@...aro.org, kuba@...nel.org,
        l.stelmach@...sung.com, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, robh+dt@...nel.org, stefan.wahren@...e.com,
        stephen@...workplumber.org, wellslutw@...il.com
Subject: Re: [net-next,v2,2/3] net: ethernet: adi: Add ADIN1110 support

On Wed, Jul 27, 2022 at 04:26:12PM +0300, alexandru.tachici@...log.com 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