netdev - Re: [PATCH net-next v1 4/4] net: phy: mxl-gpy: disable interrupts on GPY215 by default

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y6G5phSGSPk+7Dgj@lunn.ch>
Date:   Tue, 20 Dec 2022 14:33:26 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Michael Walle <michael@...le.cc>
Cc:     Xu Liang <lxu@...linear.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v1 4/4] net: phy: mxl-gpy: disable interrupts on
 GPY215 by default

> > Yes, it is a valid point to do this check, but on its own i don't
> > think it is sufficient.
> 
> Care to elaborate a bit? E.g. what is the difference to the case
> the phy would have an interrupt described but no .config_intr()
> op.
> 
> > > > I think a better place for this test is in gpy_config_intr(), return
> > > > -EOPNOTSUPP. phy_enable_interrupts() failing should then cause
> > > > phy_request_interrupt() to use polling.
> > > 
> > > Which will then print a warning, which might be misleading.
> > > Or we disable the warning if -EOPNOTSUPP is returned?
> > 
> > Disabling the warning is the right thing to do.
> 
> There is more to this. .config_intr() is also used in
> phy_init_hw() and phy_drv_supports_irq(). The latter would
> still return true in our case. I'm not sure that is correct.
> 
> After trying your suggestion, I'm still in favor of somehow
> tell the phy core to force polling mode during probe() of the
> driver.

The problem is that the MAC can set the interrupt number after the PHY
probe has been called. e.g.

https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c#L524

The interrupt needs to be set by the time the PHY is connected to the
MAC, which is often in the MAC open method, much later than the PHY
probe.

     Andrew