[RFC] net:phy:phylib: phy shares the same interrupt with mac

[RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

1)  Add a new definition PHY_MAC if phy shares the same
     interrupt with mac.
2)  Add do_phy_workqueue(), that mac can invoke this function
     in its ISR when link status changed or other conditions.

Does this seems reasonable?


---------------------------
 drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |    4 ++--
 include/linux/phy.h          |    3 +++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7670aac..c8009e9 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
    return IRQ_HANDLED;
 }

+/*
+ * do_phy_workqueue - PHY interrupt handler used by MAC
+ * @irq: interrupt line
+ * @phydev: phy_device pointer
+ *
+ * Description: When a PHY use the same interrupt with MAC,
+ * the handler is invoked by MAC, and schedules  a work task
+ * to clear the PHY's interrupt.
+ * This handler is invoked only in MAC's ISR.
+ */
+irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
+
+   BUG_ON(!in_interrupt());
+
+   if (PHY_HALTED == phydev->state)
+       return IRQ_NONE;
+
+   schedule_work(&phydev->phy_queue);
+   return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(do_phy_workqueue);
+
 /**
  * phy_enable_interrupts - Enable the interrupts from the PHY side
  * @phydev: target phy_device struct
@@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)

    INIT_WORK(&phydev->phy_queue, phy_change);

+   if (phydev->irq == PHY_MAC) {
+       err = phy_enable_interrupts(phydev);
+       return err;
+   }
+
    atomic_set(&phydev->irq_disable, 0);
    if (request_irq(phydev->irq, phy_interrupt,
                IRQF_SHARED,
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 993c52c..1857097 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
struct phy_device *phydev,

    phy_prepare_link(phydev, handler);
    phy_start_machine(phydev, NULL);
-   if (phydev->irq > 0)
+   if ((phydev->irq > 0) || (phydev->irq == PHY_MAC))
        phy_start_interrupts(phydev);

    return 0;
@@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
  */
 void phy_disconnect(struct phy_device *phydev)
 {
-   if (phydev->irq > 0)
+   if ((phydev->irq > 0) || (phydev->irq == PHY_MAC))
        phy_stop_interrupts(phydev);

    phy_stop_machine(phydev);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7da5fa8..155822c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -44,9 +44,11 @@
  * Set phydev->irq to PHY_POLL if interrupts are not supported,
  * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
  * the attached driver handles the interrupt
+ * Set to PHY_MAC if using the same interrupt with MAC
  */
 #define PHY_POLL       -1
 #define PHY_IGNORE_INTERRUPT   -2
+#define PHY_MAC        -3

 #define PHY_HAS_INTERRUPT  0x00000001
 #define PHY_HAS_MAGICANEG  0x00000002
@@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
struct ethtool_cmd *cmd);
 int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_mii_ioctl(struct phy_device *phydev,
        struct ifreq *ifr, int cmd);
+irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
 int phy_start_interrupts(struct phy_device *phydev);
 void phy_print_status(struct phy_device *phydev);
 void phy_device_free(struct phy_device *phydev);

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Florian Fainelli]

Hi,

Le 04/03/12 04:11, Jason Lin a écrit :
> 1)  Add a new definition PHY_MAC if phy shares the same
>       interrupt with mac.
> 2)  Add do_phy_workqueue(), that mac can invoke this function
>       in its ISR when link status changed or other conditions.
>
> Does this seems reasonable?

No, I think this is well handled by the PHY_IGNORE_INTERRUPT case 
(documented in Documentation/networking/phy.txt) by doing the following:

- distinguish between MAC and PHY interrupts in your MAC interrupt 
handler (most likely you have separate bits for PHY interrupts)
- once you see a PHY interrupt, call the appropriate PHY state machine 
callbacks to update the PHY state machine

>
>
> ---------------------------
>   drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
>   drivers/net/phy/phy_device.c |    4 ++--
>   include/linux/phy.h          |    3 +++
>   3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7670aac..c8009e9 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
>      return IRQ_HANDLED;
>   }
>
> +/*
> + * do_phy_workqueue - PHY interrupt handler used by MAC
> + * @irq: interrupt line
> + * @phydev: phy_device pointer
> + *
> + * Description: When a PHY use the same interrupt with MAC,
> + * the handler is invoked by MAC, and schedules  a work task
> + * to clear the PHY's interrupt.
> + * This handler is invoked only in MAC's ISR.
> + */
> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
> +
> +   BUG_ON(!in_interrupt());
> +
> +   if (PHY_HALTED == phydev->state)
> +       return IRQ_NONE;
> +
> +   schedule_work(&phydev->phy_queue);
> +   return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL(do_phy_workqueue);
> +
>   /**
>    * phy_enable_interrupts - Enable the interrupts from the PHY side
>    * @phydev: target phy_device struct
> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>
>      INIT_WORK(&phydev->phy_queue, phy_change);
>
> +   if (phydev->irq == PHY_MAC) {
> +       err = phy_enable_interrupts(phydev);
> +       return err;
> +   }
> +
>      atomic_set(&phydev->irq_disable, 0);
>      if (request_irq(phydev->irq, phy_interrupt,
>                  IRQF_SHARED,
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 993c52c..1857097 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
> struct phy_device *phydev,
>
>      phy_prepare_link(phydev, handler);
>      phy_start_machine(phydev, NULL);
> -   if (phydev->irq>  0)
> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>          phy_start_interrupts(phydev);
>
>      return 0;
> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>    */
>   void phy_disconnect(struct phy_device *phydev)
>   {
> -   if (phydev->irq>  0)
> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>          phy_stop_interrupts(phydev);
>
>      phy_stop_machine(phydev);
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 7da5fa8..155822c 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -44,9 +44,11 @@
>    * Set phydev->irq to PHY_POLL if interrupts are not supported,
>    * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>    * the attached driver handles the interrupt
> + * Set to PHY_MAC if using the same interrupt with MAC
>    */
>   #define PHY_POLL       -1
>   #define PHY_IGNORE_INTERRUPT   -2
> +#define PHY_MAC        -3
>
>   #define PHY_HAS_INTERRUPT  0x00000001
>   #define PHY_HAS_MAGICANEG  0x00000002
> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
> struct ethtool_cmd *cmd);
>   int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>   int phy_mii_ioctl(struct phy_device *phydev,
>          struct ifreq *ifr, int cmd);
> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>   int phy_start_interrupts(struct phy_device *phydev);
>   void phy_print_status(struct phy_device *phydev);
>   void phy_device_free(struct phy_device *phydev);
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 5470 bytes --]

But, I need to enable PHY's interrupt by setting PHY's registers.
And after producing a interrupt, there needs a workqueue
to ack the PHY's interrupt.
The phy_start_interrupts() can enable PHY's interrupt.
If I do the following:
1) set phydev->irq = PHY_IGNORE_INTERRUPT;
2) invoke phy_connect();
3) phy_conenct() -> phy_connect_direct() ->
    if (phydev->irq > 0)
         phy_start_interrupts(phydev);
4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
5) phy_start_interrupts() will connect to a callback function
    config_intr() of each PHY library.

For example, drivers/net/phy/marvell.c, marvell_config_intr()
Need to set register 0x12 to 0x6400 to enable corresponding interrupts.

Any comments are appreciated.
Thanks.

ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> Œ‘µÀ£º
> Hi,
>
> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>
>> 1)  Add a new definition PHY_MAC if phy shares the same
>>      interrupt with mac.
>> 2)  Add do_phy_workqueue(), that mac can invoke this function
>>      in its ISR when link status changed or other conditions.
>>
>> Does this seems reasonable?
>
>
> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
> (documented in Documentation/networking/phy.txt) by doing the following:
>
> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
> (most likely you have separate bits for PHY interrupts)
> - once you see a PHY interrupt, call the appropriate PHY state machine
> callbacks to update the PHY state machine
>
>>
>>
>> ---------------------------
>>  drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
>>  drivers/net/phy/phy_device.c |    4 ++--
>>  include/linux/phy.h          |    3 +++
>>  3 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 7670aac..c8009e9 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>> *phy_dat)
>>     return IRQ_HANDLED;
>>  }
>>
>> +/*
>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>> + * @irq: interrupt line
>> + * @phydev: phy_device pointer
>> + *
>> + * Description: When a PHY use the same interrupt with MAC,
>> + * the handler is invoked by MAC, and schedules  a work task
>> + * to clear the PHY's interrupt.
>> + * This handler is invoked only in MAC's ISR.
>> + */
>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>> +
>> +   BUG_ON(!in_interrupt());
>> +
>> +   if (PHY_HALTED == phydev->state)
>> +       return IRQ_NONE;
>> +
>> +   schedule_work(&phydev->phy_queue);
>> +   return IRQ_HANDLED;
>> +}
>> +EXPORT_SYMBOL(do_phy_workqueue);
>> +
>>  /**
>>   * phy_enable_interrupts - Enable the interrupts from the PHY side
>>   * @phydev: target phy_device struct
>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>
>>     INIT_WORK(&phydev->phy_queue, phy_change);
>>
>> +   if (phydev->irq == PHY_MAC) {
>> +       err = phy_enable_interrupts(phydev);
>> +       return err;
>> +   }
>> +
>>     atomic_set(&phydev->irq_disable, 0);
>>     if (request_irq(phydev->irq, phy_interrupt,
>>                 IRQF_SHARED,
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 993c52c..1857097 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>> struct phy_device *phydev,
>>
>>     phy_prepare_link(phydev, handler);
>>     phy_start_machine(phydev, NULL);
>> -   if (phydev->irq>  0)
>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>         phy_start_interrupts(phydev);
>>
>>     return 0;
>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>   */
>>  void phy_disconnect(struct phy_device *phydev)
>>  {
>> -   if (phydev->irq>  0)
>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>         phy_stop_interrupts(phydev);
>>
>>     phy_stop_machine(phydev);
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 7da5fa8..155822c 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -44,9 +44,11 @@
>>   * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>   * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>>   * the attached driver handles the interrupt
>> + * Set to PHY_MAC if using the same interrupt with MAC
>>   */
>>  #define PHY_POLL       -1
>>  #define PHY_IGNORE_INTERRUPT   -2
>> +#define PHY_MAC        -3
>>
>>  #define PHY_HAS_INTERRUPT  0x00000001
>>  #define PHY_HAS_MAGICANEG  0x00000002
>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>> struct ethtool_cmd *cmd);
>>  int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>  int phy_mii_ioctl(struct phy_device *phydev,
>>         struct ifreq *ifr, int cmd);
>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>  int phy_start_interrupts(struct phy_device *phydev);
>>  void phy_print_status(struct phy_device *phydev);
>>  void phy_device_free(struct phy_device *phydev);
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 5877 bytes --]

Should phy_enable_interrupts() and phy_disable_interrupts() open
to MAC driver?
And MAC driver should initialize a workqueue to ack PHY's interrupt.
Is this a better way?

ÔÚ 2012Äê4ÔÂ5ÈÕÉÏÎç9:30£¬Jason Lin <kernel.jason@gmail.com> Œ‘µÀ£º
> But, I need to enable PHY's interrupt by setting PHY's registers.
> And after producing a interrupt, there needs a workqueue
> to ack the PHY's interrupt.
> The phy_start_interrupts() can enable PHY's interrupt.
> If I do the following:
> 1) set phydev->irq = PHY_IGNORE_INTERRUPT;
> 2) invoke phy_connect();
> 3) phy_conenct() -> phy_connect_direct() ->
>    if (phydev->irq > 0)
>         phy_start_interrupts(phydev);
> 4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
> 5) phy_start_interrupts() will connect to a callback function
>    config_intr() of each PHY library.
>
> For example, drivers/net/phy/marvell.c, marvell_config_intr()
> Need to set register 0x12 to 0x6400 to enable corresponding interrupts.
>
> Any comments are appreciated.
> Thanks.
>
> ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> Œ‘µÀ£º
>> Hi,
>>
>> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>>
>>> 1)  Add a new definition PHY_MAC if phy shares the same
>>>      interrupt with mac.
>>> 2)  Add do_phy_workqueue(), that mac can invoke this function
>>>      in its ISR when link status changed or other conditions.
>>>
>>> Does this seems reasonable?
>>
>>
>> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
>> (documented in Documentation/networking/phy.txt) by doing the following:
>>
>> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
>> (most likely you have separate bits for PHY interrupts)
>> - once you see a PHY interrupt, call the appropriate PHY state machine
>> callbacks to update the PHY state machine
>>
>>>
>>>
>>> ---------------------------
>>>  drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
>>>  drivers/net/phy/phy_device.c |    4 ++--
>>>  include/linux/phy.h          |    3 +++
>>>  3 files changed, 32 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 7670aac..c8009e9 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>>> *phy_dat)
>>>     return IRQ_HANDLED;
>>>  }
>>>
>>> +/*
>>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>>> + * @irq: interrupt line
>>> + * @phydev: phy_device pointer
>>> + *
>>> + * Description: When a PHY use the same interrupt with MAC,
>>> + * the handler is invoked by MAC, and schedules  a work task
>>> + * to clear the PHY's interrupt.
>>> + * This handler is invoked only in MAC's ISR.
>>> + */
>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>>> +
>>> +   BUG_ON(!in_interrupt());
>>> +
>>> +   if (PHY_HALTED == phydev->state)
>>> +       return IRQ_NONE;
>>> +
>>> +   schedule_work(&phydev->phy_queue);
>>> +   return IRQ_HANDLED;
>>> +}
>>> +EXPORT_SYMBOL(do_phy_workqueue);
>>> +
>>>  /**
>>>   * phy_enable_interrupts - Enable the interrupts from the PHY side
>>>   * @phydev: target phy_device struct
>>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>>
>>>     INIT_WORK(&phydev->phy_queue, phy_change);
>>>
>>> +   if (phydev->irq == PHY_MAC) {
>>> +       err = phy_enable_interrupts(phydev);
>>> +       return err;
>>> +   }
>>> +
>>>     atomic_set(&phydev->irq_disable, 0);
>>>     if (request_irq(phydev->irq, phy_interrupt,
>>>                 IRQF_SHARED,
>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>> index 993c52c..1857097 100644
>>> --- a/drivers/net/phy/phy_device.c
>>> +++ b/drivers/net/phy/phy_device.c
>>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>>> struct phy_device *phydev,
>>>
>>>     phy_prepare_link(phydev, handler);
>>>     phy_start_machine(phydev, NULL);
>>> -   if (phydev->irq>  0)
>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>         phy_start_interrupts(phydev);
>>>
>>>     return 0;
>>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>>   */
>>>  void phy_disconnect(struct phy_device *phydev)
>>>  {
>>> -   if (phydev->irq>  0)
>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>         phy_stop_interrupts(phydev);
>>>
>>>     phy_stop_machine(phydev);
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 7da5fa8..155822c 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -44,9 +44,11 @@
>>>   * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>>   * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>>>   * the attached driver handles the interrupt
>>> + * Set to PHY_MAC if using the same interrupt with MAC
>>>   */
>>>  #define PHY_POLL       -1
>>>  #define PHY_IGNORE_INTERRUPT   -2
>>> +#define PHY_MAC        -3
>>>
>>>  #define PHY_HAS_INTERRUPT  0x00000001
>>>  #define PHY_HAS_MAGICANEG  0x00000002
>>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>>> struct ethtool_cmd *cmd);
>>>  int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>>  int phy_mii_ioctl(struct phy_device *phydev,
>>>         struct ifreq *ifr, int cmd);
>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>>  int phy_start_interrupts(struct phy_device *phydev);
>>>  void phy_print_status(struct phy_device *phydev);
>>>  void phy_device_free(struct phy_device *phydev);
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 6778 bytes --]

Dear:
In include/linux/phy.h
There has some comments:
/*
 * Set phydev->irq to PHY_POLL if interrupts are not supported,
 * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
 * the attached driver handles the interrupt
 */
Florian, you are right.
I have to set phydev->irq to PHY_IGNORE_INTERRUPT.
This seems to work if one MAC driver is stick to specific PHY driver.
So one can use phywrite() and phyread() to control the PHY.
In my case, MAC driver handles the interrupt, but not to stick to the
specific PHY driver. So MAC driver needs a set of general functions
to set PHY's registers to enable and ack PHY's interrupts.
Dose it make sense?

ÔÚ 2012Äê4ÔÂ6ÈÕÉÏÎç10:24£¬Jason Lin <kernel.jason@gmail.com> Œ‘µÀ£º
> Should phy_enable_interrupts() and phy_disable_interrupts() open
> to MAC driver?
> And MAC driver should initialize a workqueue to ack PHY's interrupt.
> Is this a better way?
>
> ÔÚ 2012Äê4ÔÂ5ÈÕÉÏÎç9:30£¬Jason Lin <kernel.jason@gmail.com> Œ‘µÀ£º
>> But, I need to enable PHY's interrupt by setting PHY's registers.
>> And after producing a interrupt, there needs a workqueue
>> to ack the PHY's interrupt.
>> The phy_start_interrupts() can enable PHY's interrupt.
>> If I do the following:
>> 1) set phydev->irq = PHY_IGNORE_INTERRUPT;
>> 2) invoke phy_connect();
>> 3) phy_conenct() -> phy_connect_direct() ->
>>    if (phydev->irq > 0)
>>         phy_start_interrupts(phydev);
>> 4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
>> 5) phy_start_interrupts() will connect to a callback function
>>    config_intr() of each PHY library.
>>
>> For example, drivers/net/phy/marvell.c, marvell_config_intr()
>> Need to set register 0x12 to 0x6400 to enable corresponding interrupts.
>>
>> Any comments are appreciated.
>> Thanks.
>>
>> ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> Œ‘µÀ£º
>>> Hi,
>>>
>>> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>>>
>>>> 1)  Add a new definition PHY_MAC if phy shares the same
>>>>      interrupt with mac.
>>>> 2)  Add do_phy_workqueue(), that mac can invoke this function
>>>>      in its ISR when link status changed or other conditions.
>>>>
>>>> Does this seems reasonable?
>>>
>>>
>>> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
>>> (documented in Documentation/networking/phy.txt) by doing the following:
>>>
>>> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
>>> (most likely you have separate bits for PHY interrupts)
>>> - once you see a PHY interrupt, call the appropriate PHY state machine
>>> callbacks to update the PHY state machine
>>>
>>>>
>>>>
>>>> ---------------------------
>>>>  drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
>>>>  drivers/net/phy/phy_device.c |    4 ++--
>>>>  include/linux/phy.h          |    3 +++
>>>>  3 files changed, 32 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index 7670aac..c8009e9 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>>>> *phy_dat)
>>>>     return IRQ_HANDLED;
>>>>  }
>>>>
>>>> +/*
>>>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>>>> + * @irq: interrupt line
>>>> + * @phydev: phy_device pointer
>>>> + *
>>>> + * Description: When a PHY use the same interrupt with MAC,
>>>> + * the handler is invoked by MAC, and schedules  a work task
>>>> + * to clear the PHY's interrupt.
>>>> + * This handler is invoked only in MAC's ISR.
>>>> + */
>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>>>> +
>>>> +   BUG_ON(!in_interrupt());
>>>> +
>>>> +   if (PHY_HALTED == phydev->state)
>>>> +       return IRQ_NONE;
>>>> +
>>>> +   schedule_work(&phydev->phy_queue);
>>>> +   return IRQ_HANDLED;
>>>> +}
>>>> +EXPORT_SYMBOL(do_phy_workqueue);
>>>> +
>>>>  /**
>>>>   * phy_enable_interrupts - Enable the interrupts from the PHY side
>>>>   * @phydev: target phy_device struct
>>>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>>>
>>>>     INIT_WORK(&phydev->phy_queue, phy_change);
>>>>
>>>> +   if (phydev->irq == PHY_MAC) {
>>>> +       err = phy_enable_interrupts(phydev);
>>>> +       return err;
>>>> +   }
>>>> +
>>>>     atomic_set(&phydev->irq_disable, 0);
>>>>     if (request_irq(phydev->irq, phy_interrupt,
>>>>                 IRQF_SHARED,
>>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>>> index 993c52c..1857097 100644
>>>> --- a/drivers/net/phy/phy_device.c
>>>> +++ b/drivers/net/phy/phy_device.c
>>>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>>>> struct phy_device *phydev,
>>>>
>>>>     phy_prepare_link(phydev, handler);
>>>>     phy_start_machine(phydev, NULL);
>>>> -   if (phydev->irq>  0)
>>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>>         phy_start_interrupts(phydev);
>>>>
>>>>     return 0;
>>>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>>>   */
>>>>  void phy_disconnect(struct phy_device *phydev)
>>>>  {
>>>> -   if (phydev->irq>  0)
>>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>>         phy_stop_interrupts(phydev);
>>>>
>>>>     phy_stop_machine(phydev);
>>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>>> index 7da5fa8..155822c 100644
>>>> --- a/include/linux/phy.h
>>>> +++ b/include/linux/phy.h
>>>> @@ -44,9 +44,11 @@
>>>>   * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>>>   * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>>>>   * the attached driver handles the interrupt
>>>> + * Set to PHY_MAC if using the same interrupt with MAC
>>>>   */
>>>>  #define PHY_POLL       -1
>>>>  #define PHY_IGNORE_INTERRUPT   -2
>>>> +#define PHY_MAC        -3
>>>>
>>>>  #define PHY_HAS_INTERRUPT  0x00000001
>>>>  #define PHY_HAS_MAGICANEG  0x00000002
>>>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>>>> struct ethtool_cmd *cmd);
>>>>  int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>>>  int phy_mii_ioctl(struct phy_device *phydev,
>>>>         struct ifreq *ifr, int cmd);
>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>>>  int phy_start_interrupts(struct phy_device *phydev);
>>>>  void phy_print_status(struct phy_device *phydev);
>>>>  void phy_device_free(struct phy_device *phydev);
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=GB2312, Size: 7855 bytes --]

Dear :
I describe my situation again.
In my hardware design, the interrupt (INT) pin of phy is connected to
the INT input pin of MAC.
In other words, one of triggering interrupt condition of MAC is
related to phy's interrupt.
So the phy will share the same "IRQ pin" with MAC.
As described above, the best solution is the INT pin of phy is
connected to architecture independently.
But, the hardware architecture cannot modify easily.
So I think
1. We can assign dummy IRQ number (which is a empty IRQ number but
large than zero) to phy.
phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
2. Change to do the soft IRQ in phy's ISR.
    To schedule workqueue in this soft IRQ.
In this way, the MAC can schedule phy's soft IRQ to do phy's work
queue (to do ack phy's interrupt ... etc).

Dose it make sense?

2012/4/6 Jason Lin <kernel.jason@gmail.com>:
> Dear:
> In include/linux/phy.h
> There has some comments:
> /*
>  * Set phydev->irq to PHY_POLL if interrupts are not supported,
>  * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>  * the attached driver handles the interrupt
>  */
> Florian, you are right.
> I have to set phydev->irq to PHY_IGNORE_INTERRUPT.
> This seems to work if one MAC driver is stick to specific PHY driver.
> So one can use phywrite() and phyread() to control the PHY.
> In my case, MAC driver handles the interrupt, but not to stick to the
> specific PHY driver. So MAC driver needs a set of general functions
> to set PHY's registers to enable and ack PHY's interrupts.
> Dose it make sense?
>
> ÔÚ 2012Äê4ÔÂ6ÈÕÉÏÎç10:24£¬Jason Lin <kernel.jason@gmail.com> Œ‘µÀ£º
>> Should phy_enable_interrupts() and phy_disable_interrupts() open
>> to MAC driver?
>> And MAC driver should initialize a workqueue to ack PHY's interrupt.
>> Is this a better way?
>>
>> ÔÚ 2012Äê4ÔÂ5ÈÕÉÏÎç9:30£¬Jason Lin <kernel.jason@gmail.com> Œ‘µÀ£º
>>> But, I need to enable PHY's interrupt by setting PHY's registers.
>>> And after producing a interrupt, there needs a workqueue
>>> to ack the PHY's interrupt.
>>> The phy_start_interrupts() can enable PHY's interrupt.
>>> If I do the following:
>>> 1) set phydev->irq = PHY_IGNORE_INTERRUPT;
>>> 2) invoke phy_connect();
>>> 3) phy_conenct() -> phy_connect_direct() ->
>>>    if (phydev->irq > 0)
>>>         phy_start_interrupts(phydev);
>>> 4) PHY_IGNORE_INTERRUPT = -2, it will not enable PHY's interrupt.
>>> 5) phy_start_interrupts() will connect to a callback function
>>>    config_intr() of each PHY library.
>>>
>>> For example, drivers/net/phy/marvell.c, marvell_config_intr()
>>> Need to set register 0x12 to 0x6400 to enable corresponding interrupts.
>>>
>>> Any comments are appreciated.
>>> Thanks.
>>>
>>> ÔÚ 2012Äê4ÔÂ3ÈÕÏÂÎç4:49£¬Florian Fainelli <florian@openwrt.org> Œ‘µÀ£º
>>>> Hi,
>>>>
>>>> Le 04/03/12 04:11, Jason Lin a ¨¦crit :
>>>>
>>>>> 1)  Add a new definition PHY_MAC if phy shares the same
>>>>>      interrupt with mac.
>>>>> 2)  Add do_phy_workqueue(), that mac can invoke this function
>>>>>      in its ISR when link status changed or other conditions.
>>>>>
>>>>> Does this seems reasonable?
>>>>
>>>>
>>>> No, I think this is well handled by the PHY_IGNORE_INTERRUPT case
>>>> (documented in Documentation/networking/phy.txt) by doing the following:
>>>>
>>>> - distinguish between MAC and PHY interrupts in your MAC interrupt handler
>>>> (most likely you have separate bits for PHY interrupts)
>>>> - once you see a PHY interrupt, call the appropriate PHY state machine
>>>> callbacks to update the PHY state machine
>>>>
>>>>>
>>>>>
>>>>> ---------------------------
>>>>>  drivers/net/phy/phy.c        |   27 +++++++++++++++++++++++++++
>>>>>  drivers/net/phy/phy_device.c |    4 ++--
>>>>>  include/linux/phy.h          |    3 +++
>>>>>  3 files changed, 32 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>>> index 7670aac..c8009e9 100644
>>>>> --- a/drivers/net/phy/phy.c
>>>>> +++ b/drivers/net/phy/phy.c
>>>>> @@ -527,6 +527,28 @@ static irqreturn_t phy_interrupt(int irq, void
>>>>> *phy_dat)
>>>>>     return IRQ_HANDLED;
>>>>>  }
>>>>>
>>>>> +/*
>>>>> + * do_phy_workqueue - PHY interrupt handler used by MAC
>>>>> + * @irq: interrupt line
>>>>> + * @phydev: phy_device pointer
>>>>> + *
>>>>> + * Description: When a PHY use the same interrupt with MAC,
>>>>> + * the handler is invoked by MAC, and schedules  a work task
>>>>> + * to clear the PHY's interrupt.
>>>>> + * This handler is invoked only in MAC's ISR.
>>>>> + */
>>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev) {
>>>>> +
>>>>> +   BUG_ON(!in_interrupt());
>>>>> +
>>>>> +   if (PHY_HALTED == phydev->state)
>>>>> +       return IRQ_NONE;
>>>>> +
>>>>> +   schedule_work(&phydev->phy_queue);
>>>>> +   return IRQ_HANDLED;
>>>>> +}
>>>>> +EXPORT_SYMBOL(do_phy_workqueue);
>>>>> +
>>>>>  /**
>>>>>   * phy_enable_interrupts - Enable the interrupts from the PHY side
>>>>>   * @phydev: target phy_device struct
>>>>> @@ -589,6 +611,11 @@ int phy_start_interrupts(struct phy_device *phydev)
>>>>>
>>>>>     INIT_WORK(&phydev->phy_queue, phy_change);
>>>>>
>>>>> +   if (phydev->irq == PHY_MAC) {
>>>>> +       err = phy_enable_interrupts(phydev);
>>>>> +       return err;
>>>>> +   }
>>>>> +
>>>>>     atomic_set(&phydev->irq_disable, 0);
>>>>>     if (request_irq(phydev->irq, phy_interrupt,
>>>>>                 IRQF_SHARED,
>>>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>>>> index 993c52c..1857097 100644
>>>>> --- a/drivers/net/phy/phy_device.c
>>>>> +++ b/drivers/net/phy/phy_device.c
>>>>> @@ -345,7 +345,7 @@ int phy_connect_direct(struct net_device *dev,
>>>>> struct phy_device *phydev,
>>>>>
>>>>>     phy_prepare_link(phydev, handler);
>>>>>     phy_start_machine(phydev, NULL);
>>>>> -   if (phydev->irq>  0)
>>>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>>>         phy_start_interrupts(phydev);
>>>>>
>>>>>     return 0;
>>>>> @@ -399,7 +399,7 @@ EXPORT_SYMBOL(phy_connect);
>>>>>   */
>>>>>  void phy_disconnect(struct phy_device *phydev)
>>>>>  {
>>>>> -   if (phydev->irq>  0)
>>>>> +   if ((phydev->irq>  0) || (phydev->irq == PHY_MAC))
>>>>>         phy_stop_interrupts(phydev);
>>>>>
>>>>>     phy_stop_machine(phydev);
>>>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>>>> index 7da5fa8..155822c 100644
>>>>> --- a/include/linux/phy.h
>>>>> +++ b/include/linux/phy.h
>>>>> @@ -44,9 +44,11 @@
>>>>>   * Set phydev->irq to PHY_POLL if interrupts are not supported,
>>>>>   * or not desired for this PHY.  Set to PHY_IGNORE_INTERRUPT if
>>>>>   * the attached driver handles the interrupt
>>>>> + * Set to PHY_MAC if using the same interrupt with MAC
>>>>>   */
>>>>>  #define PHY_POLL       -1
>>>>>  #define PHY_IGNORE_INTERRUPT   -2
>>>>> +#define PHY_MAC        -3
>>>>>
>>>>>  #define PHY_HAS_INTERRUPT  0x00000001
>>>>>  #define PHY_HAS_MAGICANEG  0x00000002
>>>>> @@ -510,6 +512,7 @@ int phy_ethtool_sset(struct phy_device *phydev,
>>>>> struct ethtool_cmd *cmd);
>>>>>  int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>>>>>  int phy_mii_ioctl(struct phy_device *phydev,
>>>>>         struct ifreq *ifr, int cmd);
>>>>> +irqreturn_t do_phy_workqueue(int irq, struct phy_device *phydev);
>>>>>  int phy_start_interrupts(struct phy_device *phydev);
>>>>>  void phy_print_status(struct phy_device *phydev);
>>>>>  void phy_device_free(struct phy_device *phydev);
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Andy Fleming]

On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> wrote:
> Dear :
> I describe my situation again.
> In my hardware design, the interrupt (INT) pin of phy is connected to
> the INT input pin of MAC.
> In other words, one of triggering interrupt condition of MAC is
> related to phy's interrupt.
> So the phy will share the same "IRQ pin" with MAC.
> As described above, the best solution is the INT pin of phy is
> connected to architecture independently.
> But, the hardware architecture cannot modify easily.
> So I think
> 1. We can assign dummy IRQ number (which is a empty IRQ number but
> large than zero) to phy.
> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
> 2. Change to do the soft IRQ in phy's ISR.
>     To schedule workqueue in this soft IRQ.
> In this way, the MAC can schedule phy's soft IRQ to do phy's work
> queue (to do ack phy's interrupt ... etc).
>
> Dose it make sense?
>

If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
to manage PHY interrupts, then we need to fix PHY Lib to support this
correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
want to start defining interrupt numbers which may conflict with real
numbers, and have to be constantly re-defined by various systems to
avoid that.

Your notion of making the phy_enable_interrupts() and
phy_disable_interrupts() code available to the MAC drivers sounds like
the right approach to me. But you might want to implement your own
version. The biggest problem with PHY interrupts is that masking them
requires an MDIO transaction, which is *slow*. If you can mask the
interrupt by writing a bit in a memory-mapped register, it's far
better to do it that way, at interrupt time, and *then* schedule the
PHY code to be run from a work queue. However, your driver probably
already *does* have a workqueue of some sort. If so, what you should
probably do is implement a new version of phy_change(), that doesn't
have to deal with the weird PHY interrupt masking issues. Something
like this:

mydriver_phy_change(struct mydriver_priv *priv)
{
        int err;
        struct phy_device *phydev = priv->phydev;

        if (phydev->drv->did_interrupt &&
            !phydev->drv->did_interrupt(phydev))
                goto ignore;

        err = phy_disable_interrupts(phydev);

        if (err)
                goto phy_err;

        mutex_lock(&phydev->lock);
        if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
                phydev->state = PHY_CHANGELINK;
        mutex_unlock(&phydev->lock);

        /* Reenable interrupts */
        if (PHY_HALTED != phydev->state)
                err = phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);

        if (err)
                goto irq_enable_err;

        /* reschedule state queue work to run as soon as possible */
        cancel_delayed_work_sync(&phydev->state_queue);
        schedule_delayed_work(&phydev->state_queue, 0);

        return;

ignore:
        return;

irq_enable_err:
phy_err:
        phy_error(phydev);
}


Of course, now I re-read your question, and I'm not sure I've
interpreted it correctly. Are you saying your MAC has control of the
PHY interrupt (ie - the interrupt sets a bit in some event register in
your MAC, and you can MASK/ACK it from your driver), or that the PHY
shares the same interrupt pin?

If it's the second one, you don't need to do anything, but allow your
MAC driver to register its interrupt as a shared interrupt, and allow
the PHY to manage its own interrupts, as usual.

Andy

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

Thank for your reply.
My case is the second one.
change request_irq with IRQF_SHARED, and assign the same MAC irq number to phy
(mii_bus->irq[i] = mac_irq)
This way can fix my issue.

Thanks again.

2012/7/13 Andy Fleming <afleming@gmail.com>:
> On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> wrote:
>> Dear :
>> I describe my situation again.
>> In my hardware design, the interrupt (INT) pin of phy is connected to
>> the INT input pin of MAC.
>> In other words, one of triggering interrupt condition of MAC is
>> related to phy's interrupt.
>> So the phy will share the same "IRQ pin" with MAC.
>> As described above, the best solution is the INT pin of phy is
>> connected to architecture independently.
>> But, the hardware architecture cannot modify easily.
>> So I think
>> 1. We can assign dummy IRQ number (which is a empty IRQ number but
>> large than zero) to phy.
>> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
>> 2. Change to do the soft IRQ in phy's ISR.
>>     To schedule workqueue in this soft IRQ.
>> In this way, the MAC can schedule phy's soft IRQ to do phy's work
>> queue (to do ack phy's interrupt ... etc).
>>
>> Dose it make sense?
>>
>
> If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
> to manage PHY interrupts, then we need to fix PHY Lib to support this
> correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
> want to start defining interrupt numbers which may conflict with real
> numbers, and have to be constantly re-defined by various systems to
> avoid that.
>
> Your notion of making the phy_enable_interrupts() and
> phy_disable_interrupts() code available to the MAC drivers sounds like
> the right approach to me. But you might want to implement your own
> version. The biggest problem with PHY interrupts is that masking them
> requires an MDIO transaction, which is *slow*. If you can mask the
> interrupt by writing a bit in a memory-mapped register, it's far
> better to do it that way, at interrupt time, and *then* schedule the
> PHY code to be run from a work queue. However, your driver probably
> already *does* have a workqueue of some sort. If so, what you should
> probably do is implement a new version of phy_change(), that doesn't
> have to deal with the weird PHY interrupt masking issues. Something
> like this:
>
> mydriver_phy_change(struct mydriver_priv *priv)
> {
>         int err;
>         struct phy_device *phydev = priv->phydev;
>
>         if (phydev->drv->did_interrupt &&
>             !phydev->drv->did_interrupt(phydev))
>                 goto ignore;
>
>         err = phy_disable_interrupts(phydev);
>
>         if (err)
>                 goto phy_err;
>
>         mutex_lock(&phydev->lock);
>         if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
>                 phydev->state = PHY_CHANGELINK;
>         mutex_unlock(&phydev->lock);
>
>         /* Reenable interrupts */
>         if (PHY_HALTED != phydev->state)
>                 err = phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
>
>         if (err)
>                 goto irq_enable_err;
>
>         /* reschedule state queue work to run as soon as possible */
>         cancel_delayed_work_sync(&phydev->state_queue);
>         schedule_delayed_work(&phydev->state_queue, 0);
>
>         return;
>
> ignore:
>         return;
>
> irq_enable_err:
> phy_err:
>         phy_error(phydev);
> }
>
>
> Of course, now I re-read your question, and I'm not sure I've
> interpreted it correctly. Are you saying your MAC has control of the
> PHY interrupt (ie - the interrupt sets a bit in some event register in
> your MAC, and you can MASK/ACK it from your driver), or that the PHY
> shares the same interrupt pin?
>
> If it's the second one, you don't need to do anything, but allow your
> MAC driver to register its interrupt as a shared interrupt, and allow
> the PHY to manage its own interrupts, as usual.
>
> Andy

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

Dear:
But I think there have some performance issues.
Because the phy should execute its own ISR only when the interrupt is
issued by phy.
This will cause some unnecessary MDIO access.

By the way, PHY_DUMMY_IRQ is defined in the platform definition file.
One should get PHY_DUMMY_IRQ by platform_get_irq function.
So the driver will look like this:

/*** platform related file ***************/
static struct resource mac_0_resources[] = {
    {
        .start  = MAC_PA_BASE,
        .end    = MAC_PA_BASE + SZ_4K - 1,
        .flags  = IORESOURCE_MEM,
    }, {
        .start  = IRQ_MAC,
        .flags  = IORESOURCE_IRQ,
    }, {
        .start  = IRQ_PHY,
        .flags  = IORESOURCE_IRQ,
    },
};


/*** driver file ***************/
mydriver_probe(struct platform_device *pdev)
{
    int mac_irq, phy_irq, i;

    mac_irq = platform_get_irq(pdev, 0);
    if (mac_irq < 0)
        return mac_irq;

    phy_irq = platform_get_irq(pdev, 1);
    if (phy_irq < 0)
        phy_irq = PHY_POLL;

    for (i = 0; i < PHY_MAX_ADDR; i++)
        priv->mii_bus->irq[i] = phy_irq;
}

mydriver_napi_poll(struct napi_struct *napi, int budget)
{
    struct mydriver_priv *priv = = container_of(napi, struct
mydriver_priv, napi);
    struct phy_device *phydev = priv->phydev;
    unsigned int status = get_int_status();

#ifdef CONFIG_MAC_CONTROL_PHY_INT
    if (status & PHY_INT) {
        tasklet_schedule(&phydev->phy_task);
    }
#endif

}

/**** phy lib modification ***************/
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 3cbda08..482f7e5 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -416,7 +416,7 @@ out_unlock:
 }
 EXPORT_SYMBOL(phy_start_aneg);

-
+static void phy_tasklet(unsigned long data);
 static void phy_change(struct work_struct *work);

 /**
@@ -523,10 +523,12 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
     * context, so we need to disable the irq here.  A work
     * queue will write the PHY to disable and clear the
     * interrupt, and then reenable the irq line. */
-   disable_irq_nosync(irq);
+/* disable_irq_nosync(irq);
    atomic_inc(&phydev->irq_disable);

-   schedule_work(&phydev->phy_queue);
+   schedule_work(&phydev->phy_queue);*/
+
+   tasklet_schedule(&phydev->phy_task);

    return IRQ_HANDLED;
 }
@@ -591,6 +593,7 @@ int phy_start_interrupts(struct phy_device *phydev)
 {
    int err = 0;

+   tasklet_init(&phydev->phy_task, phy_tasklet, (unsigned long)phydev);
    INIT_WORK(&phydev->phy_queue, phy_change);

    atomic_set(&phydev->irq_disable, 0);
@@ -633,6 +636,7 @@ int phy_stop_interrupts(struct phy_device *phydev)
     * possibly pending and take care of the matter below.
     */
    cancel_work_sync(&phydev->phy_queue);
+   tasklet_kill(&phydev->phy_task);
    /*
     * If work indeed has been cancelled, disable_irq() will have
     * been left unbalanced from phy_interrupt() and enable_irq()
@@ -645,7 +649,13 @@ int phy_stop_interrupts(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_stop_interrupts);


-
+static void phy_tasklet(unsigned long data)
+{
+   struct phy_device *phydev = (struct phy_device *)data;
+   disable_irq_nosync(phydev->irq);
+   atomic_inc(&phydev->irq_disable);
+   schedule_work(&phydev->phy_queue);
+}
 /**
  * phy_change - Scheduled by the phy_interrupt/timer to handle PHY changes
  * @work: work_struct that describes the work to be done
diff --git a/include/linux/phy.h b/include/linux/phy.h
index c599f7e..04203a7 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -330,6 +330,7 @@ struct phy_device {
    /* Interrupt and Polling infrastructure */
    struct work_struct phy_queue;
    struct delayed_work state_queue;
+   struct tasklet_struct phy_task;
    atomic_t irq_disable;

    struct mutex lock;

/*** end modification **********************************/

how about this modification?
F.Y.I.
Thanks.

2012/7/13 Jason Lin <kernel.jason@gmail.com>:
> Thank for your reply.
> My case is the second one.
> change request_irq with IRQF_SHARED, and assign the same MAC irq number to phy
> (mii_bus->irq[i] = mac_irq)
> This way can fix my issue.
>
> Thanks again.
>
> 2012/7/13 Andy Fleming <afleming@gmail.com>:
>> On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> wrote:
>>> Dear :
>>> I describe my situation again.
>>> In my hardware design, the interrupt (INT) pin of phy is connected to
>>> the INT input pin of MAC.
>>> In other words, one of triggering interrupt condition of MAC is
>>> related to phy's interrupt.
>>> So the phy will share the same "IRQ pin" with MAC.
>>> As described above, the best solution is the INT pin of phy is
>>> connected to architecture independently.
>>> But, the hardware architecture cannot modify easily.
>>> So I think
>>> 1. We can assign dummy IRQ number (which is a empty IRQ number but
>>> large than zero) to phy.
>>> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
>>> 2. Change to do the soft IRQ in phy's ISR.
>>>     To schedule workqueue in this soft IRQ.
>>> In this way, the MAC can schedule phy's soft IRQ to do phy's work
>>> queue (to do ack phy's interrupt ... etc).
>>>
>>> Dose it make sense?
>>>
>>
>> If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
>> to manage PHY interrupts, then we need to fix PHY Lib to support this
>> correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
>> want to start defining interrupt numbers which may conflict with real
>> numbers, and have to be constantly re-defined by various systems to
>> avoid that.
>>
>> Your notion of making the phy_enable_interrupts() and
>> phy_disable_interrupts() code available to the MAC drivers sounds like
>> the right approach to me. But you might want to implement your own
>> version. The biggest problem with PHY interrupts is that masking them
>> requires an MDIO transaction, which is *slow*. If you can mask the
>> interrupt by writing a bit in a memory-mapped register, it's far
>> better to do it that way, at interrupt time, and *then* schedule the
>> PHY code to be run from a work queue. However, your driver probably
>> already *does* have a workqueue of some sort. If so, what you should
>> probably do is implement a new version of phy_change(), that doesn't
>> have to deal with the weird PHY interrupt masking issues. Something
>> like this:
>>
>> mydriver_phy_change(struct mydriver_priv *priv)
>> {
>>         int err;
>>         struct phy_device *phydev = priv->phydev;
>>
>>         if (phydev->drv->did_interrupt &&
>>             !phydev->drv->did_interrupt(phydev))
>>                 goto ignore;
>>
>>         err = phy_disable_interrupts(phydev);
>>
>>         if (err)
>>                 goto phy_err;
>>
>>         mutex_lock(&phydev->lock);
>>         if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
>>                 phydev->state = PHY_CHANGELINK;
>>         mutex_unlock(&phydev->lock);
>>
>>         /* Reenable interrupts */
>>         if (PHY_HALTED != phydev->state)
>>                 err = phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
>>
>>         if (err)
>>                 goto irq_enable_err;
>>
>>         /* reschedule state queue work to run as soon as possible */
>>         cancel_delayed_work_sync(&phydev->state_queue);
>>         schedule_delayed_work(&phydev->state_queue, 0);
>>
>>         return;
>>
>> ignore:
>>         return;
>>
>> irq_enable_err:
>> phy_err:
>>         phy_error(phydev);
>> }
>>
>>
>> Of course, now I re-read your question, and I'm not sure I've
>> interpreted it correctly. Are you saying your MAC has control of the
>> PHY interrupt (ie - the interrupt sets a bit in some event register in
>> your MAC, and you can MASK/ACK it from your driver), or that the PHY
>> shares the same interrupt pin?
>>
>> If it's the second one, you don't need to do anything, but allow your
>> MAC driver to register its interrupt as a shared interrupt, and allow
>> the PHY to manage its own interrupts, as usual.
>>
>> Andy

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Jason Lin]

Any comments will be appreciated.
Thanks.

2012/7/13 Jason Lin <kernel.jason@gmail.com>:
> Dear:
> But I think there have some performance issues.
> Because the phy should execute its own ISR only when the interrupt is
> issued by phy.
> This will cause some unnecessary MDIO access.
>
> By the way, PHY_DUMMY_IRQ is defined in the platform definition file.
> One should get PHY_DUMMY_IRQ by platform_get_irq function.
> So the driver will look like this:
>
> /*** platform related file ***************/
> static struct resource mac_0_resources[] = {
>     {
>         .start  = MAC_PA_BASE,
>         .end    = MAC_PA_BASE + SZ_4K - 1,
>         .flags  = IORESOURCE_MEM,
>     }, {
>         .start  = IRQ_MAC,
>         .flags  = IORESOURCE_IRQ,
>     }, {
>         .start  = IRQ_PHY,
>         .flags  = IORESOURCE_IRQ,
>     },
> };
>
>
> /*** driver file ***************/
> mydriver_probe(struct platform_device *pdev)
> {
>     int mac_irq, phy_irq, i;
>
>     mac_irq = platform_get_irq(pdev, 0);
>     if (mac_irq < 0)
>         return mac_irq;
>
>     phy_irq = platform_get_irq(pdev, 1);
>     if (phy_irq < 0)
>         phy_irq = PHY_POLL;
>
>     for (i = 0; i < PHY_MAX_ADDR; i++)
>         priv->mii_bus->irq[i] = phy_irq;
> }
>
> mydriver_napi_poll(struct napi_struct *napi, int budget)
> {
>     struct mydriver_priv *priv = = container_of(napi, struct
> mydriver_priv, napi);
>     struct phy_device *phydev = priv->phydev;
>     unsigned int status = get_int_status();
>
> #ifdef CONFIG_MAC_CONTROL_PHY_INT
>     if (status & PHY_INT) {
>         tasklet_schedule(&phydev->phy_task);
>     }
> #endif
>
> }
>
> /**** phy lib modification ***************/
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 3cbda08..482f7e5 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -416,7 +416,7 @@ out_unlock:
>  }
>  EXPORT_SYMBOL(phy_start_aneg);
>
> -
> +static void phy_tasklet(unsigned long data);
>  static void phy_change(struct work_struct *work);
>
>  /**
> @@ -523,10 +523,12 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
>      * context, so we need to disable the irq here.  A work
>      * queue will write the PHY to disable and clear the
>      * interrupt, and then reenable the irq line. */
> -   disable_irq_nosync(irq);
> +/* disable_irq_nosync(irq);
>     atomic_inc(&phydev->irq_disable);
>
> -   schedule_work(&phydev->phy_queue);
> +   schedule_work(&phydev->phy_queue);*/
> +
> +   tasklet_schedule(&phydev->phy_task);
>
>     return IRQ_HANDLED;
>  }
> @@ -591,6 +593,7 @@ int phy_start_interrupts(struct phy_device *phydev)
>  {
>     int err = 0;
>
> +   tasklet_init(&phydev->phy_task, phy_tasklet, (unsigned long)phydev);
>     INIT_WORK(&phydev->phy_queue, phy_change);
>
>     atomic_set(&phydev->irq_disable, 0);
> @@ -633,6 +636,7 @@ int phy_stop_interrupts(struct phy_device *phydev)
>      * possibly pending and take care of the matter below.
>      */
>     cancel_work_sync(&phydev->phy_queue);
> +   tasklet_kill(&phydev->phy_task);
>     /*
>      * If work indeed has been cancelled, disable_irq() will have
>      * been left unbalanced from phy_interrupt() and enable_irq()
> @@ -645,7 +649,13 @@ int phy_stop_interrupts(struct phy_device *phydev)
>  }
>  EXPORT_SYMBOL(phy_stop_interrupts);
>
>
> -
> +static void phy_tasklet(unsigned long data)
> +{
> +   struct phy_device *phydev = (struct phy_device *)data;
> +   disable_irq_nosync(phydev->irq);
> +   atomic_inc(&phydev->irq_disable);
> +   schedule_work(&phydev->phy_queue);
> +}
>  /**
>   * phy_change - Scheduled by the phy_interrupt/timer to handle PHY changes
>   * @work: work_struct that describes the work to be done
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index c599f7e..04203a7 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -330,6 +330,7 @@ struct phy_device {
>     /* Interrupt and Polling infrastructure */
>     struct work_struct phy_queue;
>     struct delayed_work state_queue;
> +   struct tasklet_struct phy_task;
>     atomic_t irq_disable;
>
>     struct mutex lock;
>
> /*** end modification **********************************/
>
> how about this modification?
> F.Y.I.
> Thanks.
>
> 2012/7/13 Jason Lin <kernel.jason@gmail.com>:
>> Thank for your reply.
>> My case is the second one.
>> change request_irq with IRQF_SHARED, and assign the same MAC irq number to phy
>> (mii_bus->irq[i] = mac_irq)
>> This way can fix my issue.
>>
>> Thanks again.
>>
>> 2012/7/13 Andy Fleming <afleming@gmail.com>:
>>> On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> wrote:
>>>> Dear :
>>>> I describe my situation again.
>>>> In my hardware design, the interrupt (INT) pin of phy is connected to
>>>> the INT input pin of MAC.
>>>> In other words, one of triggering interrupt condition of MAC is
>>>> related to phy's interrupt.
>>>> So the phy will share the same "IRQ pin" with MAC.
>>>> As described above, the best solution is the INT pin of phy is
>>>> connected to architecture independently.
>>>> But, the hardware architecture cannot modify easily.
>>>> So I think
>>>> 1. We can assign dummy IRQ number (which is a empty IRQ number but
>>>> large than zero) to phy.
>>>> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
>>>> 2. Change to do the soft IRQ in phy's ISR.
>>>>     To schedule workqueue in this soft IRQ.
>>>> In this way, the MAC can schedule phy's soft IRQ to do phy's work
>>>> queue (to do ack phy's interrupt ... etc).
>>>>
>>>> Dose it make sense?
>>>>
>>>
>>> If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
>>> to manage PHY interrupts, then we need to fix PHY Lib to support this
>>> correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
>>> want to start defining interrupt numbers which may conflict with real
>>> numbers, and have to be constantly re-defined by various systems to
>>> avoid that.
>>>
>>> Your notion of making the phy_enable_interrupts() and
>>> phy_disable_interrupts() code available to the MAC drivers sounds like
>>> the right approach to me. But you might want to implement your own
>>> version. The biggest problem with PHY interrupts is that masking them
>>> requires an MDIO transaction, which is *slow*. If you can mask the
>>> interrupt by writing a bit in a memory-mapped register, it's far
>>> better to do it that way, at interrupt time, and *then* schedule the
>>> PHY code to be run from a work queue. However, your driver probably
>>> already *does* have a workqueue of some sort. If so, what you should
>>> probably do is implement a new version of phy_change(), that doesn't
>>> have to deal with the weird PHY interrupt masking issues. Something
>>> like this:
>>>
>>> mydriver_phy_change(struct mydriver_priv *priv)
>>> {
>>>         int err;
>>>         struct phy_device *phydev = priv->phydev;
>>>
>>>         if (phydev->drv->did_interrupt &&
>>>             !phydev->drv->did_interrupt(phydev))
>>>                 goto ignore;
>>>
>>>         err = phy_disable_interrupts(phydev);
>>>
>>>         if (err)
>>>                 goto phy_err;
>>>
>>>         mutex_lock(&phydev->lock);
>>>         if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
>>>                 phydev->state = PHY_CHANGELINK;
>>>         mutex_unlock(&phydev->lock);
>>>
>>>         /* Reenable interrupts */
>>>         if (PHY_HALTED != phydev->state)
>>>                 err = phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
>>>
>>>         if (err)
>>>                 goto irq_enable_err;
>>>
>>>         /* reschedule state queue work to run as soon as possible */
>>>         cancel_delayed_work_sync(&phydev->state_queue);
>>>         schedule_delayed_work(&phydev->state_queue, 0);
>>>
>>>         return;
>>>
>>> ignore:
>>>         return;
>>>
>>> irq_enable_err:
>>> phy_err:
>>>         phy_error(phydev);
>>> }
>>>
>>>
>>> Of course, now I re-read your question, and I'm not sure I've
>>> interpreted it correctly. Are you saying your MAC has control of the
>>> PHY interrupt (ie - the interrupt sets a bit in some event register in
>>> your MAC, and you can MASK/ACK it from your driver), or that the PHY
>>> shares the same interrupt pin?
>>>
>>> If it's the second one, you don't need to do anything, but allow your
>>> MAC driver to register its interrupt as a shared interrupt, and allow
>>> the PHY to manage its own interrupts, as usual.
>>>
>>> Andy

Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac

[Florian Fainelli]

Hi,

On Tuesday 24 July 2012 09:58:53 Jason Lin wrote:
> Any comments will be appreciated.
> Thanks.

If you phy shares thee interrupt line with the MAC, then proceed with the 
following:

- set your PHY address irq to PHY_IGNORE_INTERRUPT
- in your MAC interrupt handler, differentiate the PHY interrupt status bit 
from the other status bits
- call the adjust_link function that you have defined for phylib in your 
interrupt handler and make sure that nothing sleeps inside this adjust_link 
callback.

> 
> 2012/7/13 Jason Lin <kernel.jason@gmail.com>:
> > Dear:
> > But I think there have some performance issues.
> > Because the phy should execute its own ISR only when the interrupt is
> > issued by phy.
> > This will cause some unnecessary MDIO access.
> >
> > By the way, PHY_DUMMY_IRQ is defined in the platform definition file.
> > One should get PHY_DUMMY_IRQ by platform_get_irq function.
> > So the driver will look like this:
> >
> > /*** platform related file ***************/
> > static struct resource mac_0_resources[] = {
> >     {
> >         .start  = MAC_PA_BASE,
> >         .end    = MAC_PA_BASE + SZ_4K - 1,
> >         .flags  = IORESOURCE_MEM,
> >     }, {
> >         .start  = IRQ_MAC,
> >         .flags  = IORESOURCE_IRQ,
> >     }, {
> >         .start  = IRQ_PHY,
> >         .flags  = IORESOURCE_IRQ,
> >     },
> > };
> >
> >
> > /*** driver file ***************/
> > mydriver_probe(struct platform_device *pdev)
> > {
> >     int mac_irq, phy_irq, i;
> >
> >     mac_irq = platform_get_irq(pdev, 0);
> >     if (mac_irq < 0)
> >         return mac_irq;
> >
> >     phy_irq = platform_get_irq(pdev, 1);
> >     if (phy_irq < 0)
> >         phy_irq = PHY_POLL;
> >
> >     for (i = 0; i < PHY_MAX_ADDR; i++)
> >         priv->mii_bus->irq[i] = phy_irq;
> > }
> >
> > mydriver_napi_poll(struct napi_struct *napi, int budget)
> > {
> >     struct mydriver_priv *priv = = container_of(napi, struct
> > mydriver_priv, napi);
> >     struct phy_device *phydev = priv->phydev;
> >     unsigned int status = get_int_status();
> >
> > #ifdef CONFIG_MAC_CONTROL_PHY_INT
> >     if (status & PHY_INT) {
> >         tasklet_schedule(&phydev->phy_task);
> >     }
> > #endif
> >
> > }
> >
> > /**** phy lib modification ***************/
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 3cbda08..482f7e5 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -416,7 +416,7 @@ out_unlock:
> >  }
> >  EXPORT_SYMBOL(phy_start_aneg);
> >
> > -
> > +static void phy_tasklet(unsigned long data);
> >  static void phy_change(struct work_struct *work);
> >
> >  /**
> > @@ -523,10 +523,12 @@ static irqreturn_t phy_interrupt(int irq, void 
*phy_dat)
> >      * context, so we need to disable the irq here.  A work
> >      * queue will write the PHY to disable and clear the
> >      * interrupt, and then reenable the irq line. */
> > -   disable_irq_nosync(irq);
> > +/* disable_irq_nosync(irq);
> >     atomic_inc(&phydev->irq_disable);
> >
> > -   schedule_work(&phydev->phy_queue);
> > +   schedule_work(&phydev->phy_queue);*/
> > +
> > +   tasklet_schedule(&phydev->phy_task);
> >
> >     return IRQ_HANDLED;
> >  }
> > @@ -591,6 +593,7 @@ int phy_start_interrupts(struct phy_device *phydev)
> >  {
> >     int err = 0;
> >
> > +   tasklet_init(&phydev->phy_task, phy_tasklet, (unsigned long)phydev);
> >     INIT_WORK(&phydev->phy_queue, phy_change);
> >
> >     atomic_set(&phydev->irq_disable, 0);
> > @@ -633,6 +636,7 @@ int phy_stop_interrupts(struct phy_device *phydev)
> >      * possibly pending and take care of the matter below.
> >      */
> >     cancel_work_sync(&phydev->phy_queue);
> > +   tasklet_kill(&phydev->phy_task);
> >     /*
> >      * If work indeed has been cancelled, disable_irq() will have
> >      * been left unbalanced from phy_interrupt() and enable_irq()
> > @@ -645,7 +649,13 @@ int phy_stop_interrupts(struct phy_device *phydev)
> >  }
> >  EXPORT_SYMBOL(phy_stop_interrupts);
> >
> >
> > -
> > +static void phy_tasklet(unsigned long data)
> > +{
> > +   struct phy_device *phydev = (struct phy_device *)data;
> > +   disable_irq_nosync(phydev->irq);
> > +   atomic_inc(&phydev->irq_disable);
> > +   schedule_work(&phydev->phy_queue);
> > +}
> >  /**
> >   * phy_change - Scheduled by the phy_interrupt/timer to handle PHY 
changes
> >   * @work: work_struct that describes the work to be done
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index c599f7e..04203a7 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -330,6 +330,7 @@ struct phy_device {
> >     /* Interrupt and Polling infrastructure */
> >     struct work_struct phy_queue;
> >     struct delayed_work state_queue;
> > +   struct tasklet_struct phy_task;
> >     atomic_t irq_disable;
> >
> >     struct mutex lock;
> >
> > /*** end modification **********************************/
> >
> > how about this modification?
> > F.Y.I.
> > Thanks.
> >
> > 2012/7/13 Jason Lin <kernel.jason@gmail.com>:
> >> Thank for your reply.
> >> My case is the second one.
> >> change request_irq with IRQF_SHARED, and assign the same MAC irq number 
to phy
> >> (mii_bus->irq[i] = mac_irq)
> >> This way can fix my issue.
> >>
> >> Thanks again.
> >>
> >> 2012/7/13 Andy Fleming <afleming@gmail.com>:
> >>> On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> 
wrote:
> >>>> Dear :
> >>>> I describe my situation again.
> >>>> In my hardware design, the interrupt (INT) pin of phy is connected to
> >>>> the INT input pin of MAC.
> >>>> In other words, one of triggering interrupt condition of MAC is
> >>>> related to phy's interrupt.
> >>>> So the phy will share the same "IRQ pin" with MAC.
> >>>> As described above, the best solution is the INT pin of phy is
> >>>> connected to architecture independently.
> >>>> But, the hardware architecture cannot modify easily.
> >>>> So I think
> >>>> 1. We can assign dummy IRQ number (which is a empty IRQ number but
> >>>> large than zero) to phy.
> >>>> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
> >>>> 2. Change to do the soft IRQ in phy's ISR.
> >>>>     To schedule workqueue in this soft IRQ.
> >>>> In this way, the MAC can schedule phy's soft IRQ to do phy's work
> >>>> queue (to do ack phy's interrupt ... etc).
> >>>>
> >>>> Dose it make sense?
> >>>>
> >>>
> >>> If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
> >>> to manage PHY interrupts, then we need to fix PHY Lib to support this
> >>> correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
> >>> want to start defining interrupt numbers which may conflict with real
> >>> numbers, and have to be constantly re-defined by various systems to
> >>> avoid that.
> >>>
> >>> Your notion of making the phy_enable_interrupts() and
> >>> phy_disable_interrupts() code available to the MAC drivers sounds like
> >>> the right approach to me. But you might want to implement your own
> >>> version. The biggest problem with PHY interrupts is that masking them
> >>> requires an MDIO transaction, which is *slow*. If you can mask the
> >>> interrupt by writing a bit in a memory-mapped register, it's far
> >>> better to do it that way, at interrupt time, and *then* schedule the
> >>> PHY code to be run from a work queue. However, your driver probably
> >>> already *does* have a workqueue of some sort. If so, what you should
> >>> probably do is implement a new version of phy_change(), that doesn't
> >>> have to deal with the weird PHY interrupt masking issues. Something
> >>> like this:
> >>>
> >>> mydriver_phy_change(struct mydriver_priv *priv)
> >>> {
> >>>         int err;
> >>>         struct phy_device *phydev = priv->phydev;
> >>>
> >>>         if (phydev->drv->did_interrupt &&
> >>>             !phydev->drv->did_interrupt(phydev))
> >>>                 goto ignore;
> >>>
> >>>         err = phy_disable_interrupts(phydev);
> >>>
> >>>         if (err)
> >>>                 goto phy_err;
> >>>
> >>>         mutex_lock(&phydev->lock);
> >>>         if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev-
>state))
> >>>                 phydev->state = PHY_CHANGELINK;
> >>>         mutex_unlock(&phydev->lock);
> >>>
> >>>         /* Reenable interrupts */
> >>>         if (PHY_HALTED != phydev->state)
> >>>                 err = phy_config_interrupt(phydev, 
PHY_INTERRUPT_ENABLED);
> >>>
> >>>         if (err)
> >>>                 goto irq_enable_err;
> >>>
> >>>         /* reschedule state queue work to run as soon as possible */
> >>>         cancel_delayed_work_sync(&phydev->state_queue);
> >>>         schedule_delayed_work(&phydev->state_queue, 0);
> >>>
> >>>         return;
> >>>
> >>> ignore:
> >>>         return;
> >>>
> >>> irq_enable_err:
> >>> phy_err:
> >>>         phy_error(phydev);
> >>> }
> >>>
> >>>
> >>> Of course, now I re-read your question, and I'm not sure I've
> >>> interpreted it correctly. Are you saying your MAC has control of the
> >>> PHY interrupt (ie - the interrupt sets a bit in some event register in
> >>> your MAC, and you can MASK/ACK it from your driver), or that the PHY
> >>> shares the same interrupt pin?
> >>>
> >>> If it's the second one, you don't need to do anything, but allow your
> >>> MAC driver to register its interrupt as a shared interrupt, and allow
> >>> the PHY to manage its own interrupts, as usual.
> >>>
> >>> Andy