| Message ID | 1234817631-7981-2-git-send-email-agust@denx.de |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Feb 16, 2009 at 2:53 PM, Anatolij Gustschin <agust@denx.de> wrote: > Marvell 88E1121R PHY device can be hardware-configured > to use shared interrupt pin for both PHY ports. This > patch adds support for shared PHY interrupts to the PHY > framework to enable support for such PHY configurations. Well, technically the phylib already supports shared interrupts (all of my systems have only one interrupt for all of the PHYs). This patch adds support for some PHYs which can report which device caused the interrupt. > > Signed-off-by: Anatolij Gustschin <agust@denx.de> > --- > drivers/net/phy/phy.c | 13 +++++++++++++ > drivers/net/phy/phy_device.c | 2 ++ > include/linux/phy.h | 4 ++++ > 3 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index df4e625..93d387e 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -657,9 +657,17 @@ EXPORT_SYMBOL(phy_stop_interrupts); > static void phy_change(struct work_struct *work) > { > int err; > + int src; > struct phy_device *phydev = > container_of(work, struct phy_device, phy_queue); > > + if (phydev->drv->flags & PHY_INTERRUPTS_SHARED > + && phydev->drv->interrupt_src) { > + src = phydev->drv->interrupt_src(phydev); > + if ((src == -1) || (src != phydev->addr)) > + goto ignore; > + } > + I don't agree with this approach to the problem. First, it would appear that phydev->drv->interrupt_src is redundant with PHY_INTERRUPTS_SHARED. So we probably don't need to add that bit. Next, the result of interrupt_src isn't being used to do anything but determine whether *this* PHY originated the interrupt. So a minor improvement would be to have a function ->did_interrupt(), which returns 1 if the PHY generated an interrupt, and 0 otherwise. For solving this problem, I'd prefer a solution that didn't appear to be so closely tied to the specific hardware you are using. The ->did_interrupt() method would allow any PHY with interrupts to report whether it believes it created the interrupt. It seems to me that this is just creating more overhead, though. There may be a way to consolidate the various interrupt-related functions, and create a new set which allows for this sort of "early-out". Another possibility to consider is some way that each "global" device could only read this register once per interrupt, and report that status through the phy_device struct(s). Maybe we need some sort of container for multi-PHY devices? Andy -- 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
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index df4e625..93d387e 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -657,9 +657,17 @@ EXPORT_SYMBOL(phy_stop_interrupts); static void phy_change(struct work_struct *work) { int err; + int src; struct phy_device *phydev = container_of(work, struct phy_device, phy_queue); + if (phydev->drv->flags & PHY_INTERRUPTS_SHARED + && phydev->drv->interrupt_src) { + src = phydev->drv->interrupt_src(phydev); + if ((src == -1) || (src != phydev->addr)) + goto ignore; + } + err = phy_disable_interrupts(phydev); if (err) @@ -688,6 +696,11 @@ static void phy_change(struct work_struct *work) return; +ignore: + atomic_dec(&phydev->irq_disable); + enable_irq(phydev->irq); + return; + irq_enable_err: disable_irq(phydev->irq); atomic_inc(&phydev->irq_disable); diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index e11b03b..dccc445 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -401,6 +401,8 @@ void phy_detach(struct phy_device *phydev) * real driver could be loaded */ if (phydev->dev.driver == &genphy_driver.driver) device_release_driver(&phydev->dev); + else if (phydev->drv->remove) + phydev->drv->remove(phydev); } EXPORT_SYMBOL(phy_detach); diff --git a/include/linux/phy.h b/include/linux/phy.h index 77c4ed6..a844c2a 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -49,6 +49,7 @@ #define PHY_HAS_INTERRUPT 0x00000001 #define PHY_HAS_MAGICANEG 0x00000002 +#define PHY_INTERRUPTS_SHARED 0x00000004 /* Interface Mode definitions */ typedef enum { @@ -389,6 +390,9 @@ struct phy_driver { /* Enables or disables interrupts */ int (*config_intr)(struct phy_device *phydev); + /* Returns interrupt source if interrupt pin is shared */ + int (*interrupt_src)(struct phy_device *phydev); + /* Clears up any memory if needed */ void (*remove)(struct phy_device *phydev);
Marvell 88E1121R PHY device can be hardware-configured to use shared interrupt pin for both PHY ports. This patch adds support for shared PHY interrupts to the PHY framework to enable support for such PHY configurations. Signed-off-by: Anatolij Gustschin <agust@denx.de> --- drivers/net/phy/phy.c | 13 +++++++++++++ drivers/net/phy/phy_device.c | 2 ++ include/linux/phy.h | 4 ++++ 3 files changed, 19 insertions(+), 0 deletions(-)