| 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
| ||
|
Message-ID: <CAGCDX1kZo5f8_BpMgq75o0EoTRh9X=VbH2-rXDBYoKdCosO+VQ@mail.gmail.com>
Date: Fri, 6 Apr 2012 10:24:12 +0800
From: Jason Lin <kernel.jason@...il.com>
To: Florian Fainelli <florian@...nwrt.org>
Cc: netdev@...r.kernel.org
Subject: Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac
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@...il.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@...nwrt.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@...r.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists