(unknown)

(unknown)

[kou.ishizaki]

jgarzik@pobox.com, jim@jklewis.com
Subject: Re: [Cbe-oss-dev] spidernet: dynamic phy setup code
In-Reply-To: <200701261409.29537.jens@de.ibm.com>
From: Ishizaki Kou <kouish@swc.toshiba.co.jp>
X-Mailer: Mew version 4.2 on Emacs 21.3 / Mule 5.0 (SAKAKI)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Jens-san

> This patch modifies the patch submitted by Kou Ishizaki to make it
work on the
> blade
(http://marc.theaimsgroup.com/?l=linux-netdev&m=116593424505539&w=2).
> Unfortunately I dont have access to a Celleb so I cannot test it
there.

Thanks for arranging our patch to work on Cell Blade.

This patch partially works on celleb but remains 
following several problems.
1. It doesn't recover once an ethernet cable which is
   connected to a spider_net card is unpluged. 
2. It doesn't work when the spider_net card is connected to 
   a 100Mbps ethernet switch.

To solve these problems, we need to restore some codes
you removed from your patch.

(1)
>- if (card->aneg_count > 10) {
>-  /* timeout */
>-  card->aneg_count = 0;
>-  is1000 = !is1000;
>-  goto re_setup;

>- if (phy->speed == 1000 && !is1000) {
>-  is1000 = 1;
>-  goto re_setup;
>- } else if(phy->speed != 1000 && is1000) {
>-  is1000 = 0;
>-  goto re_setup;
>- }

We need to use different auto-neg initial settings between
for 10/100Mbps ethernet switches and for Gbps ethernet switches.
Driver don't know which type of network switch is connected to
network card, so we try both settings alternately in auto negtiation
sequences by using a variable "is1000".
Furthermore, we have a problem that poll_link() may succeed even when
the auto-neg initial setting is for different network switch type,
and the network card does not work on this case. We retry auto-neg
with the another initial setting on this case.

#We are commented that "is1000" should be in spider_net_card.
#We fixed it in another patch. Please refer the following.
#http://ozlabs.org/pipermail/linuxppc-dev/2007-January/030203.html

But we don't think this is the best solution, and we are still
developing 
our spidernet driver. If you have a good alternative idea, please tell
us.

(2)
>- spider_net_write_reg(card, SPIDER_NET_GMACST,
>-        spider_net_read_reg(card, SPIDER_NET_GMACST));
>- spider_net_write_reg(card, SPIDER_NET_GMACINTEN, 0x4);

These codes are enabling LINK status interrupt which is disabled
at the beginning of auto-neg.
Without this operation, auto negotiation works only when a connection
detected for the first time, and auto negotiation will not work 
when an ethernet cable is unpluged or pluged.

(3)
>- mii_phy_probe(phy, phy->mii_id);
It seems that PHY reset is necessary before auto negotiation,
after a link once went down.
We can't call directly reset routine from driver, so we call
mii_phy_probe().
We are still developping the patch as we noted, and we are considering
to call mii_phy_probe() from spider_net_setup_aneg(), or to call
reset_one_mii_phy() from bcm54xx_setup_aneg().

We think these (1)-(3) are necessary, but we are afraid that you removed
them
by a reason that they causes some trouble in Cell Blade. If so please
tell us.


Best regards,
Kou Ishizaki

Re: No Subject

[Benjamin Herrenschmidt]

> We need to use different auto-neg initial settings between
> for 10/100Mbps ethernet switches and for Gbps ethernet switches.

That is strange ! What PHY chip are you using ? Are you sure it's not
just you not properly configuring the PHY ? Is the datasheet for the PHY
available somewhere ?

> Driver don't know which type of network switch is connected to
> network card, so we try both settings alternately in auto negtiation
> sequences by using a variable "is1000".

sungem has an autoneg sequence that falls back to forced speeds but it's
not useful on any modern setup. Your PHY should be perfectly capable to
autoneg on both 1000bT and 10/100bT...

> Furthermore, we have a problem that poll_link() may succeed even when
> the auto-neg initial setting is for different network switch type,
> and the network card does not work on this case. We retry auto-neg
> with the another initial setting on this case.

Ugh ? What is that initial setting bit exactly ? If the link is up, it
should work.

> >- spider_net_write_reg(card, SPIDER_NET_GMACST,
> >-        spider_net_read_reg(card, SPIDER_NET_GMACST));
> >- spider_net_write_reg(card, SPIDER_NET_GMACINTEN, 0x4);
> 
> These codes are enabling LINK status interrupt which is disabled
> at the beginning of auto-neg.
> Without this operation, auto negotiation works only when a connection
> detected for the first time, and auto negotiation will not work 
> when an ethernet cable is unpluged or pluged.

Most drivers poll the link rather than use an interrupt as those are
often unreliable.

> (3)
> >- mii_phy_probe(phy, phy->mii_id);
> It seems that PHY reset is necessary before auto negotiation,
> after a link once went down.

It shouldn't be... again, what PHY are you using ?

> We can't call directly reset routine from driver, so we call
> mii_phy_probe().

If you really need to reset it, then change sungem_phy.c to export the
reset function. But I'm surprised you need that. Another option is to
reset the PHY in your PHY's setup_aneg() function.

> We are still developping the patch as we noted, and we are considering
> to call mii_phy_probe() from spider_net_setup_aneg(), or to call
> reset_one_mii_phy() from bcm54xx_setup_aneg().
> 
> We think these (1)-(3) are necessary, but we are afraid that you removed
> them
> by a reason that they causes some trouble in Cell Blade. If so please
> tell us.

You might want to borrow the link state machine from sungem.c instead...
it tends to just work :-)

Ben.

Re: spidernet: dynamic phy setup code

[Ishizaki Kou]

Ben-san, Jens-san

Thanks for your comments and advices.
First, we should apologize to you that some troubles which we informed
about spidernet auto-negotiation are caused by our mistake.
We used wrong MACRO for "advertise" variable to
phy->def->ops->setup_aneg(). 

We will post the revised patch for spider_net.
Following are the current status of troubles we informed.

(1)
>- if (card->aneg_count > 10) {
>-  /* timeout */
>-  card->aneg_count = 0;
>-  is1000 = !is1000;
>-  goto re_setup;
>- if (phy->speed == 1000 && !is1000) {
>-  is1000 = 1;
>-  goto re_setup;
>- } else if(phy->speed != 1000 && is1000) {
>-  is1000 = 0;
>-  goto re_setup;
>- }
> We need to use different auto-neg initial settings between
> for 10/100Mbps ethernet switches and for Gbps ethernet switches.
> Driver don't know which type of network switch is connected to
> network card, so we try both settings alternately in auto negtiation
> sequences by using a variable "is1000".
> Furthermore, we have a problem that poll_link() may succeed even when
> the auto-neg initial setting is for different network switch type,
> and the network card does not work on this case. We retry auto-neg
> with the another initial setting on this case.
Solved.

We are now able to use Gbps switches and 10/100Mbps switches with 
the same advertise setting and we don't need to use is1000.
The re_setup is not necessary and they are removed in the revised patch.

2.
>> - spider_net_write_reg(card, SPIDER_NET_GMACST,
>> -        spider_net_read_reg(card, SPIDER_NET_GMACST));
>> - spider_net_write_reg(card, SPIDER_NET_GMACINTEN, 0x4);
>
> These codes are enabling LINK status interrupt which is disabled
> at the beginning of auto-neg.
> Without this operation, auto negotiation works only when a connection
> detected for the first time, and auto negotiation will not work 
> when an ethernet cable is unpluged or pluged.
Pending(No problem, if they don't cause any trouble on Cell Blades).

By the way, why do you suggest to use polling for auto-neg, Ben-san? 
Is there a merit to use polling, or is there a problem to use interrupt?
Sorry, we couldn't find the reason why sungem use polling for auto-neg. 

They might not be necessary, if we use polling to handle LINK status as
Ben-san said.

3
>> - mii_phy_probe(phy, phy->mii_id);
> It seems that PHY reset is necessary before auto negotiation,
> in the case that ethernet card is disconnected from an ethernet switch
> then connected to another ethernet switch.
> We can't call directly reset routine from driver, so we call
mii_phy_probe().
Pending.

After all, we need phy reset.
> If you really need to reset it, then change sungem_phy.c to export the
> reset function. But I'm surprised you need that. Another option is to
> reset the PHY in your PHY's setup_aneg() function.
We still wonder which is the best way, and mii_phy_probe() still remains
so far.

Best regards,
Kou Ishizaki

No subject

[auto595907]

>I created a Bugzilla entry at 
>https://bugzilla.kernel.org/show_bug.cgi?id=16423
>for your bug report, please add your address to the CC list in 
>there, thanks!
>
>-- 
>Maciej Rutecki
>http://www.maciek.unixy.pl


this patch fixes the bug above
(http://marc.info/?l=netfilter-devel&m=128116762801769&w=2)


thanks.