Re: [PATCH net-next v3 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt

From: Florian Fainelli
Date: Fri Nov 09 2018 - 15:33:42 EST


On 11/9/18 12:22 PM, Heiner Kallweit wrote:
> On 09.11.2018 21:13, Andrew Lunn wrote:
>> Hi Heiner
>>
>>> +static bool phy_drv_supports_irq(struct phy_driver *phydrv)
>>> +{
>>> + return phydrv->config_intr || phydrv->ack_interrupt;
>>> +}
>>
>> Should this be && not || ? I thought both needed to be provided for
>> interrupts to work.
>>
>> Andrew
>>
> I've seen at least one driver which configures interrupts in
> config_init and doesn't define a config_intr callback
> (ack_interrupt callback is there)

That driver should probably be fixed, while it most likely does not make
any significant difference during probe/connect, since config_init() and
config_intr() are virtually happening at the same time, this is not
necessarily true when disconnecting from the PHY where we really want
config_intr() to effectively disable the interrupts and not leaving
something enabled that would now become unmaskable, because no more
driver attached.

> Intention of this check is not to ensure that the driver defines
> everything to make interrupts work. All it states:
> If at least one of the irq-related callbacks is defined, then
> we interpret this as indicator that the PHY supports interrupts.

I agree with Andrew here, that this should be an AND here, both
callbacks must be implemented for interrupts to work correctly.
--
Florian