2007-06-30 11:47:47

by Török Edwin

[permalink] [raw]
Subject: [PATCH] b44: power down PHY when interface down

When the interface is down (or driver removed), the BroadCom 44xx card remains
powered on, and both its MAC and PHY is using up power.
This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
is halted, and does a partial chip reset turns off the activity LEDs too.

Applies to 2.6.22-rc6, or current git head.

Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using powertop).

Signed-off-by: Torok Edwin <[email protected]>
---
b44.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index 879a2ff..00d0f57 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -113,6 +113,8 @@ static void b44_init_rings(struct b44 *);
#define B44_FULL_RESET 1
#define B44_FULL_RESET_SKIP_PHY 2
#define B44_PARTIAL_RESET 3
+#define B44_CHIP_RESET_FULL 4
+#define B44_CHIP_RESET_PARTIAL 5

static void b44_init_hw(struct b44 *, int);

@@ -1283,7 +1285,7 @@ static void b44_clear_stats(struct b44 *bp)
}

/* bp->lock is held. */
-static void b44_chip_reset(struct b44 *bp)
+static void b44_chip_reset(struct b44 *bp, int reset_kind)
{
if (ssb_is_core_up(bp)) {
bw32(bp, B44_RCV_LAZY, 0);
@@ -1307,6 +1309,10 @@ static void b44_chip_reset(struct b44 *bp)

b44_clear_stats(bp);

+ if (reset_kind == B44_CHIP_RESET_PARTIAL)
+ return;/* don't enable PHY if we are doing a partial reset
+ , we are probably going to power down */
+
/* Make PHY accessible. */
bw32(bp, B44_MDIO_CTRL, (MDIO_CTRL_PREAMBLE |
(0x0d & MDIO_CTRL_MAXF_MASK)));
@@ -1332,7 +1338,14 @@ static void b44_chip_reset(struct b44 *bp)
static void b44_halt(struct b44 *bp)
{
b44_disable_ints(bp);
- b44_chip_reset(bp);
+ /* reset PHY */
+ b44_phy_reset(bp);
+ /* power down PHY */
+ printk(KERN_INFO PFX "%s: powering down PHY\n", bp->dev->name);
+ bw32(bp, B44_MAC_CTRL, MAC_CTRL_PHY_PDOWN);
+ /* now reset the chip, but without enabling the MAC&PHY
+ * part of it. This has to be done _after_ we shut down the PHY */
+ b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
}

/* bp->lock is held. */
@@ -1376,7 +1389,7 @@ static void b44_init_hw(struct b44 *bp, int reset_kind)
{
u32 val;

- b44_chip_reset(bp);
+ b44_chip_reset(bp, B44_CHIP_RESET_FULL);
if (reset_kind == B44_FULL_RESET) {
b44_phy_reset(bp);
b44_setup_phy(bp);
@@ -1430,7 +1443,7 @@ static int b44_open(struct net_device *dev)

err = request_irq(dev->irq, b44_interrupt, IRQF_SHARED, dev->name, dev);
if (unlikely(err < 0)) {
- b44_chip_reset(bp);
+ b44_chip_reset(bp, B44_CHIP_RESET_PARTIAL);
b44_free_rings(bp);
b44_free_consistent(bp);
goto out;
@@ -2250,7 +2263,7 @@ static int __devinit b44_init_one(struct pci_dev *pdev,
/* Chip reset provides power to the b44 MAC & PCI cores, which
* is necessary for MAC register access.
*/
- b44_chip_reset(bp);
+ b44_chip_reset(bp, B44_CHIP_RESET_FULL);

printk(KERN_INFO "%s: Broadcom 4400 10/100BaseT Ethernet ", dev->name);
for (i = 0; i < 6; i++)
@@ -2284,6 +2297,7 @@ static void __devexit b44_remove_one(struct pci_dev *pdev)
free_netdev(dev);
pci_release_regions(pdev);
pci_disable_device(pdev);
+ pci_set_power_state(pdev, PCI_D3hot);
pci_set_drvdata(pdev, NULL);
}

@@ -2312,6 +2326,7 @@ static int b44_suspend(struct pci_dev *pdev,
pm_message_t state)
b44_setup_wol(bp);
}
pci_disable_device(pdev);
+ pci_set_power_state(pdev, PCI_D3hot);
return 0;
}


2007-06-30 12:06:16

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sat, Jun 30, 2007 at 02:47:35PM +0300, T?r?k Edvin wrote:
> When the interface is down (or driver removed), the BroadCom 44xx card
> remains
> powered on, and both its MAC and PHY is using up power.
> This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> is halted, and does a partial chip reset turns off the activity LEDs too.

Do you still get link beat detection when the phy is powered down?

--
Matthew Garrett | [email protected]

2007-06-30 14:46:58

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

Matthew Garrett wrote:
> On Sat, Jun 30, 2007 at 02:47:35PM +0300, T?r?k Edvin wrote:
>> When the interface is down (or driver removed), the BroadCom 44xx card
>> remains
>> powered on, and both its MAC and PHY is using up power.
>> This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
>> is halted, and does a partial chip reset turns off the activity LEDs too.
>
> Do you still get link beat detection when the phy is powered down?
>
does that matter?
If the interface is down, nic drivers aren't expected to detect
link... if userspace wants to find link status it should have the
interface up.

2007-06-30 15:19:58

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sat, Jun 30, 2007 at 07:44:59AM -0700, Arjan van de Ven wrote:
> Matthew Garrett wrote:
> >Do you still get link beat detection when the phy is powered down?
> >
> does that matter?
> If the interface is down, nic drivers aren't expected to detect
> link... if userspace wants to find link status it should have the
> interface up.

If that's the policy, why do we leave this up to individual drivers?
Right now most of them let you make the query regardless of interface
state.

I'd agree that there's a need for a state where we power down as much as
possible (even at the cost of functionality), but where possible it
would also be nice to offer a state where the mac is powered down and
the phy left up. If the existing semantics are confused then it would be
nice to fix them, too.
--
Matthew Garrett | [email protected]

2007-06-30 15:56:37

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sat, Jun 30, 2007 at 04:19:23PM +0100, Matthew Garrett wrote:

> I'd agree that there's a need for a state where we power down as much as
> possible (even at the cost of functionality), but where possible it
> would also be nice to offer a state where the mac is powered down and
> the phy left up.

There are PHYs which can detect that someone's on the other end even
when powered down..

2007-06-30 16:42:24

by Jeff Garzik

[permalink] [raw]
Subject: PM policy, hotplug, power saving (was Re: [PATCH] b44: power down PHY when interface down)

Arjan van de Ven wrote:
> Matthew Garrett wrote:
>> Do you still get link beat detection when the phy is powered down?

> does that matter?
> If the interface is down, nic drivers aren't expected to detect link...
> if userspace wants to find link status it should have the interface up.


Definitely matters. Switch renegotiation can take a while, and you must
take into account the common case of interface bouncing (immediate down,
then up).

Hoards actively complained the few times we experimented with this,
because of e.g. DHCP's habit of bouncing the interface, which resulted
in PHY power bouncing, which resulted in negotiation, which resulted in
an excrutiating wait on various broken or stupid switches.

Overall, this may be classed with other problems of a similar sort: we
can power down a PHY, but that removes hotplug capability and extends
partner/link negotiation time.

Like SATA, we actually want to support BOTH -- active hotplug and PHY
power-down -- and so this wanders into power management policy.

Give me a knob, and we can program plenty of ethernet|SATA|USB|...
drivers to power down the PHY and save power.

Jeff


2007-06-30 18:11:33

by Stephen Hemminger

[permalink] [raw]
Subject: Re: PM policy, hotplug, power saving (was Re: [PATCH] b44: power down PHY when interface down)

On Sat, 30 Jun 2007 12:42:06 -0400
Jeff Garzik <[email protected]> wrote:

> Arjan van de Ven wrote:
> > Matthew Garrett wrote:
> >> Do you still get link beat detection when the phy is powered down?
>
> > does that matter?
> > If the interface is down, nic drivers aren't expected to detect link...
> > if userspace wants to find link status it should have the interface up.
>
>
> Definitely matters. Switch renegotiation can take a while, and you must
> take into account the common case of interface bouncing (immediate down,
> then up).
>
> Hoards actively complained the few times we experimented with this,
> because of e.g. DHCP's habit of bouncing the interface, which resulted
> in PHY power bouncing, which resulted in negotiation, which resulted in
> an excrutiating wait on various broken or stupid switches.
>
> Overall, this may be classed with other problems of a similar sort: we
> can power down a PHY, but that removes hotplug capability and extends
> partner/link negotiation time.
>
> Like SATA, we actually want to support BOTH -- active hotplug and PHY
> power-down -- and so this wanders into power management policy.
>
> Give me a knob, and we can program plenty of ethernet|SATA|USB|...
> drivers to power down the PHY and save power.
>
> Jeff

We do have IFF_DORMANT, but almost no driver uses it. And most
certainly, the common applications wouldn't know how to use it.

Subject: PM policy, hotplug, power saving and WoL

On Sat, 30 Jun 2007, Jeff Garzik wrote:
> Like SATA, we actually want to support BOTH -- active hotplug and PHY
> power-down -- and so this wanders into power management policy.
>
> Give me a knob, and we can program plenty of ethernet|SATA|USB|... drivers
> to power down the PHY and save power.

While at it, could we please fix our borked WOL interface requirements, so
that the PHY is *never* to be powered down when WOL is active? This is
another deficiency that userspace has to work around in halt(8)...

If WOL is enabled and supported in a device, the kernel must never do
anything that would cause that device to stop responding to WOL packets.

OTOH, if WOL is disabled, it would be very very nice indeed to be able to
tell the kernel that yes, it is to power down the PHY if it is not in use.
Laptops appreciate it a lot, and so do the switches (which will know they
don't have to forward any traffic over that port).

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2007-06-30 21:54:32

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Saturday 30 June 2007 13:47:35 T?r?k Edvin wrote:
> When the interface is down (or driver removed), the BroadCom 44xx card remains
> powered on, and both its MAC and PHY is using up power.
> This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> is halted, and does a partial chip reset turns off the activity LEDs too.
>
> Applies to 2.6.22-rc6, or current git head.
>
> Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using powertop).

Hm, I was going to measure the real power advantage with a
PCI-extender card. But my B44B0 card doesn't seem to work in
that extender card. It works perfectly fine sticked directly into
the motherboard, though, and other cards like a BCM4318 work in
the extender, too.
Not sure what this is.
The extender has an application note about nonworking cards in the
extender and a too big resistor on the board IDSEL pin being the
cause of this.
Maybe I can try with another machine tomorrow.

--
Greetings Michael.

2007-06-30 22:03:16

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sat, Jun 30, 2007 at 11:53:25PM +0200, Michael Buesch wrote:

> > When the interface is down (or driver removed), the BroadCom 44xx card remains
> > powered on, and both its MAC and PHY is using up power.
> > This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> > is halted, and does a partial chip reset turns off the activity LEDs too.
> >
> > Applies to 2.6.22-rc6, or current git head.
> >
> > Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using powertop).
>
> Hm, I was going to measure the real power advantage with a
> PCI-extender card. But my B44B0 card doesn't seem to work in
> that extender card. It works perfectly fine sticked directly into
> the motherboard, though, and other cards like a BCM4318 work in
> the extender, too.
> Not sure what this is.
> The extender has an application note about nonworking cards in the
> extender and a too big resistor on the board IDSEL pin being the
> cause of this.

Does the card show up in lspci at all? IDSEL drive strength
issues should only affect config space accesses.

Does the extender board have a PCI-PCI bridge on it? (If not,
there's not really any reason to resistively couple the IDSEL
line to the host, since the host should take care of that.)


> Maybe I can try with another machine tomorrow.

That would only make a difference if there is no PCI-PCI bridge on
the extender board.

If the extender resistively couples the host's IDSEL line, you might
see different results on a different host bridge, since different
host bridges can use different numbers of IDSEL stepping cycles.

2007-06-30 22:25:33

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sunday 01 July 2007 00:03:01 Lennert Buytenhek wrote:
> On Sat, Jun 30, 2007 at 11:53:25PM +0200, Michael Buesch wrote:
>
> > > When the interface is down (or driver removed), the BroadCom 44xx card remains
> > > powered on, and both its MAC and PHY is using up power.
> > > This patch makes the driver issue a MAC_CTRL_PHY_PDOWN when the interface
> > > is halted, and does a partial chip reset turns off the activity LEDs too.
> > >
> > > Applies to 2.6.22-rc6, or current git head.
> > >
> > > Tested on a Broadcom BCM4401-B0 card, it saves ~0.5W (measured using powertop).
> >
> > Hm, I was going to measure the real power advantage with a
> > PCI-extender card. But my B44B0 card doesn't seem to work in
> > that extender card. It works perfectly fine sticked directly into
> > the motherboard, though, and other cards like a BCM4318 work in
> > the extender, too.
> > Not sure what this is.
> > The extender has an application note about nonworking cards in the
> > extender and a too big resistor on the board IDSEL pin being the
> > cause of this.
>
> Does the card show up in lspci at all?

No it doesn't.

> IDSEL drive strength
> issues should only affect config space accesses.

Yeah.

> Does the extender board have a PCI-PCI bridge on it? (If not,
> there's not really any reason to resistively couple the IDSEL
> line to the host, since the host should take care of that.)

There's no bridge. It just decouples all voltage lines, so you can
drive it from external supply and/or measure voltages and current.
On the PCB it looks like the the IDSEL line is rather directly
routed to the host IDSEL. It just goes through one of the bus
isolation chips. So I guess (just my guess) that this chip has some
resistance and if the total resistance of the chip + the IDSEL
resistor on the mainboard goes above some threshold it doesn't work
anymore for some cards. In the application note they write
about trouble for IDSEL resistors >51ohms.

> > Maybe I can try with another machine tomorrow.
>
> That would only make a difference if there is no PCI-PCI bridge on
> the extender board.

Well, they suggest it in the application note as a possible fix. ;)


--
Greetings Michael.

2007-06-30 23:17:49

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH] b44: power down PHY when interface down

On Sun, Jul 01, 2007 at 12:24:40AM +0200, Michael Buesch wrote:

> > > Hm, I was going to measure the real power advantage with a
> > > PCI-extender card. But my B44B0 card doesn't seem to work in
> > > that extender card. It works perfectly fine sticked directly into
> > > the motherboard, though, and other cards like a BCM4318 work in
> > > the extender, too.
> > > Not sure what this is.
> > > The extender has an application note about nonworking cards in the
> > > extender and a too big resistor on the board IDSEL pin being the
> > > cause of this.
> >
> > Does the card show up in lspci at all?
>
> No it doesn't.

Right, so it sounds like it might be this issue.


> > Does the extender board have a PCI-PCI bridge on it? (If not,
> > there's not really any reason to resistively couple the IDSEL
> > line to the host, since the host should take care of that.)
>
> There's no bridge. It just decouples all voltage lines, so you can
> drive it from external supply and/or measure voltages and current.
> On the PCB it looks like the the IDSEL line is rather directly
> routed to the host IDSEL. It just goes through one of the bus
> isolation chips. So I guess (just my guess) that this chip has some
> resistance and if the total resistance of the chip + the IDSEL
> resistor on the mainboard goes above some threshold it doesn't work
> anymore for some cards. In the application note they write
> about trouble for IDSEL resistors >51ohms.

More or less. You can't add the resistances like that, since the
bus isolation chip buffers the IDSEL signal, but it is correct that
if the host's IDSEL resistor is larger than a certain value, the
combination of the resistive coupling of IDSEL plus the extra buffer
in the isolator might be causing the IDSEL input on the 'guest' PCI
board to assert too late (or not assert at all), causing config
accesses to fail.

(This also depends on the specific 'guest' PCI board used, as you
noted, due to differing IDSEL trace lengths/capacitances and input
pin capacitances on different PCI boards. Also, it might work at
33 MHz but not work at 66 MHz, etc.)

If you feel adventurous, you could try to hack around this by
figuring out which AD[31:16] line this PCI slot's IDSEL line is
resistively coupled to (depends on the slot), and then adding
another parallel resistor on the board itself to make the bus
isolator's input buffer charge faster. Note that this does
increase the load on that specific AD[] line, which might cause
other funny effects.


> > > Maybe I can try with another machine tomorrow.
> >
> > That would only make a difference if there is no PCI-PCI bridge on
> > the extender board.
>
> Well, they suggest it in the application note as a possible fix. ;)

The bus isolation chip doesn't count as a PCI-PCI bridge. :) I'm just
saying that you wouldn't see the issue you are seeing now if the
extender board had a real PCI-PCI bridge on it, since in that case the
type 0 config access to the guest PCI board would be generated by the
bridge instead of by the host.