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  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]
Date:   Fri, 16 Dec 2022 10:46:08 +0100
From:   Michael Walle <>
To:     Andrew Lunn <>
Cc:     Xu Liang <>,
        Heiner Kallweit <>,
        Russell King <>,
        "David S . Miller" <>,
        Eric Dumazet <>,
        Jakub Kicinski <>,
        Paolo Abeni <>,
        Rob Herring <>,
        Krzysztof Kozlowski <>,,,
Subject: Re: [PATCH net-next v1 4/4] net: phy: mxl-gpy: disable interrupts on
 GPY215 by default

Am 2022-12-03 21:36, schrieb Andrew Lunn:
>> > > @@ -290,6 +291,10 @@ static int gpy_probe(struct phy_device *phydev)
>> > >  	phydev->priv = priv;
>> > >  	mutex_init(&priv->mbox_lock);
>> > >
>> > > +	if (gpy_has_broken_mdint(phydev) &&
>> > > +	    !device_property_present(dev,
>> > > "maxlinear,use-broken-interrupts"))
>> > > +		phydev->irq = PHY_POLL;
>> > > +
>> >
>> > I'm not sure of ordering here. It could be phydev->irq is set after
>> > probe. The IRQ is requested as part of phy_connect_direct(), which is
>> > much later.
>> I've did it that way, because phy_probe() also sets phydev->irq = 
>> in some cases and the phy driver .probe() is called right after it.
> 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()

>> > 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 same way it's done if there is no .config_intr().

It's not like we'd need change the mode after probe during
runtime. Also with your proposed changed the attachment print
is wrong/misleading as it still prints the original irq instead


Powered by blists - more mailing lists