netdev - Re: [PATCH] net: phy: Support for non-HW interrupt devices

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: <20160108004238.GD4389@lunn.ch>
Date:	Fri, 8 Jan 2016 01:42:38 +0100
From:	Andrew Lunn <andrew@...n.ch>
To:	Woojung.Huh@...rochip.com
Cc:	f.fainelli@...il.com, netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH] net: phy: Support for non-HW interrupt devices

On Thu, Jan 07, 2016 at 11:14:19PM +0000, Woojung.Huh@...rochip.com wrote:
> This patch introduces PHY_PSEUDO_INTERRUPT and routines to
> handle pseudo (not-real-hardware) interrupt such as USB interrupt pipe for
> USB-to-Ethernet device.
> 
> Unless having real hardware interrupt handler registered by request_irq(),
> phy_state_machine() can't avoid calling phy_read_status()
> to monitor link changes. This especially prevents USB-to-Ethernet device
> from going to auto suspend to save power.

Hi Woojung

Do you have an example PHY driver making use of this. At the moment i
don't see how it works.

There is possibly a cleaner way to do this. I've some unpublished work
where i added interrupt support for the Marvell switches. These
switches can have embedded PHYs, and the interrupts from these PHYs go
into the switchers interrupt controller. From that one line comes out
and is connected to a GPIO line. To support this, i've implemented a
Linux chained interrupt driver within the switch driver. PHY
interrupts then become normal Linux interrupts which the PHY library
can make use of.

So, maybe your USB driver should implemented a Linux interrupt
controller. USB interrupt pipe transactions result in the interrupt
controller triggering the normal Linux interrupt mechanism, and so no
need to change PHYLIB.

In fact, USB interrupt transaction to Linux interrupt sounds generic
enough that it might of been implemented already somewhere.

       Andrew

> 
> Signed-off-by: Woojung Huh <woojung.huh@...rochip.com>
> ---
>  drivers/net/phy/phy.c | 31 +++++++++++++++++++++++++++++--
>  include/linux/phy.h   |  6 ++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 47cd306..8f678e9 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -588,6 +588,25 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
>  }
>  
>  /**
> + * phy_pseudo_interrupt - PHY pseudo interrupt handler
> + * @phydev: target phy_device struct
> + *
> + * Description: This is for phy pseudo interrupt such as
> + * USB interrupt pipe for USB-to-Ethernet devices.
> + * When a PHY pseudo interrupt occurs, i.e, USB interrupt pipe completion,
> + * the handler schedules a work task to clear the interrupt.
> + */
> +void phy_pseudo_interrupt(struct phy_device *phydev)
> +{
> +	if (phydev->state != PHY_HALTED) {
> +		atomic_inc(&phydev->irq_disable);
> +
> +		queue_work(system_power_efficient_wq, &phydev->phy_queue);
> +	}
> +}
> +EXPORT_SYMBOL(phy_pseudo_interrupt);
> +
> +/**
>   * phy_enable_interrupts - Enable the interrupts from the PHY side
>   * @phydev: target phy_device struct
>   */
> @@ -640,12 +659,20 @@ phy_err:
>  int phy_start_interrupts(struct phy_device *phydev)
>  {
>  	atomic_set(&phydev->irq_disable, 0);
> -	if (request_irq(phydev->irq, phy_interrupt, 0, "phy_interrupt",
> -			phydev) < 0) {
> +
> +	/* phydev->irq is bigger than zero when real H/W interrupt.
> +	 * This avoids calling request_irq when pseudo interrupt such as
> +	 * USB interrupt pipe for USB-to-Ethernet device.
> +	 */
> +	if ((phydev->irq > 0) &&
> +	    (request_irq(phydev->irq, phy_interrupt, 0, "phy_interrupt",
> +			 phydev) < 0)) {
>  		pr_warn("%s: Can't get IRQ %d (PHY)\n",
>  			phydev->bus->name, phydev->irq);
>  		phydev->irq = PHY_POLL;
>  		return 0;
> +	} else if (phydev->irq == PHY_PSEUDO_INTERRUPT) {
> +		phy_pseudo_interrupt(phydev);
>  	}
>  
>  	return phy_enable_interrupts(phydev);
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 05fde31..a68e690 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -55,6 +55,12 @@
>  #define PHY_POLL		-1
>  #define PHY_IGNORE_INTERRUPT	-2
>  
> +/*
> + * Set to PHY_PSEUDO_INTERRUPT when the attached driver uses
> + * not real hardware interrupt such as USB interrupt pipe.
> + */
> +#define PHY_PSEUDO_INTERRUPT	-100
> +
>  #define PHY_HAS_INTERRUPT	0x00000001
>  #define PHY_HAS_MAGICANEG	0x00000002
>  #define PHY_IS_INTERNAL		0x00000004
> -- 
> 2.1.4