netdev - Re: [PATCH net-next 2/3] net: phy: add callback for custom interrupt handler to struct phy

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: <4a8eb515-472e-785e-619d-88ed1b2f8aa7@gmail.com>
Date:   Tue, 28 May 2019 12:37:25 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/3] net: phy: add callback for custom interrupt
 handler to struct phy_driver

On 5/27/19 12:36 PM, Heiner Kallweit wrote:
> On 27.05.2019 21:25, Florian Fainelli wrote:
>>
>>
>> On 5/27/2019 11:28 AM, Heiner Kallweit wrote:
>>> The phylib interrupt handler handles link change events only currently.
>>> However PHY drivers may want to use other interrupt sources too,
>>> e.g. to report temperature monitoring events. Therefore add a callback
>>> to struct phy_driver allowing PHY drivers to implement a custom
>>> interrupt handler.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>>> Suggested-by: Russell King - ARM Linux admin <linux@...linux.org.uk>
>>> ---
>>>  drivers/net/phy/phy.c | 9 +++++++--
>>>  include/linux/phy.h   | 3 +++
>>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 20955836c..8030d0a97 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -774,8 +774,13 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
>>>  	if (phydev->drv->did_interrupt && !phydev->drv->did_interrupt(phydev))
>>>  		return IRQ_NONE;
>>>  
>>> -	/* reschedule state queue work to run as soon as possible */
>>> -	phy_trigger_machine(phydev);
>>> +	if (phydev->drv->handle_interrupt) {
>>> +		if (phydev->drv->handle_interrupt(phydev))
>>
>> If Russell is okay with such a model where the PHY state machine still
>> manages the interrupts at large, and only calls a specific callback for
>> specific even handling, that's fine. We might have to allow PHY drivers
>> to let them specify what they want to get passed to
>> request_threaded_irq(), or leave it to them to do it.
>>
> This proposed easy model should be able to cover quite some use cases.
> One constraint may be that interrupts are disabled if phylib state
> machine isn't in a started state. Means most likely it's not able to
> cover e.g. the requirement to allow temperature warning interrupts if
> PHY is in state HALTED.
> 

There is possibly an user case where having interrupts might be useful,
imagine your system overheated and you brought down the PHY into
PHY_HALTED stated, you might want to be able to receive thermal trip
point crossing indicating that the low threshold has been crossed, which
means you could resume operating the link again.
-- 
Florian