All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Andy Fleming <afleming@freescale.com>
Cc: linuxppc-dev@ozlabs.org, netdev@vger.kernel.org,
	Li Yang <leoli@freescale.com>, David Miller <davem@davemloft.net>
Subject: Re: [PATCH] gianfar: Fix half-duplex operation for non-MII/RMII interfaces
Date: Thu, 25 Jun 2009 01:10:38 +0400	[thread overview]
Message-ID: <20090624211038.GA29555@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <ED58F994-6BDC-4496-8445-EA1D984F1379@freescale.com>

On Wed, Jun 24, 2009 at 03:18:59PM -0500, Andy Fleming wrote:
>
> On Jun 24, 2009, at 1:27 PM, Anton Vorontsov wrote:
>
>> It appears that gianfar driver has the same problem[1] that I
>> just fixed for ucc_geth.
>>
>> NFS boot using 10/half link takes about 10 minutes to complete:
>>
>
>>
>> The symptoms were observed on MPC8379E-RDB boards (eTSEC). Although
>> I didn't find where documentation forbids clearing Full Duplex bit
>> for non-MII/RMII modes, it's pretty distinct that the bit should be
>> set.
>>
>> It's no wonder though, QE Ethernet and TSEC are pretty similar.
>
>> -			if (!(phydev->duplex))
>> -				tempval &= ~(MACCFG2_FULL_DUPLEX);
>> +			if (!phydev->duplex &&
>> +					(phyi == PHY_INTERFACE_MODE_MII ||
>> +					 phyi == PHY_INTERFACE_MODE_RMII))
>
>
> Hmm....have you tested this on a GMII interface?

Nope, only RGMII.

>  *Technically*, full  
> duplex is required for GMII, as GMII is used only for gigabit.  However, 
> we've been treating the GMII interface type as an indicator that the PHY 
> *has* a GMII connection to the NIC.  When gianfar detects the speed is 
> 10/100 it switches to the compatible MII interface via this code, just 
> below:
>
>                         case 100:
>                         case 10:
>                                 tempval =
>                                     ((tempval & ~(MACCFG2_IF)) |  
> MACCFG2_MII);
>
>
> My concern is that you will be detecting the GMII interface, and  
> disallowing half-duplex, despite the fact that the interface is actually 
> running at 10 or 100 Mbit.

Very interesting, though I'm not sure I'm completely following. :-)

Are you saying that I should do this instead:

	if (!phydev->duplex &&
			(phyi == PHY_INTERFACE_MODE_MII ||
			 phyi == PHY_INTERFACE_MODE_RMII ||
			 (phyi == PHY_INTERFACE_MODE_GMII &&
			  phydev->speed < 1000)))
		tempval &= ~MACCFG2_FULL_DUPLEX;
	else
		tempval |= MACCFG2_FULL_DUPLEX;

i.e. we detected GMII interface initially, but it downgraded
to MII since speed is < 1000, thus we can set half-duplex in MAC?

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

WARNING: multiple messages have this Message-ID (diff)
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Andy Fleming <afleming@freescale.com>
Cc: David Miller <davem@davemloft.net>,
	Kumar Gala <galak@kernel.crashing.org>,
	Li Yang <leoli@freescale.com>,
	linuxppc-dev@ozlabs.org, netdev@vger.kernel.org
Subject: Re: [PATCH] gianfar: Fix half-duplex operation for non-MII/RMII interfaces
Date: Thu, 25 Jun 2009 01:10:38 +0400	[thread overview]
Message-ID: <20090624211038.GA29555@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <ED58F994-6BDC-4496-8445-EA1D984F1379@freescale.com>

On Wed, Jun 24, 2009 at 03:18:59PM -0500, Andy Fleming wrote:
>
> On Jun 24, 2009, at 1:27 PM, Anton Vorontsov wrote:
>
>> It appears that gianfar driver has the same problem[1] that I
>> just fixed for ucc_geth.
>>
>> NFS boot using 10/half link takes about 10 minutes to complete:
>>
>
>>
>> The symptoms were observed on MPC8379E-RDB boards (eTSEC). Although
>> I didn't find where documentation forbids clearing Full Duplex bit
>> for non-MII/RMII modes, it's pretty distinct that the bit should be
>> set.
>>
>> It's no wonder though, QE Ethernet and TSEC are pretty similar.
>
>> -			if (!(phydev->duplex))
>> -				tempval &= ~(MACCFG2_FULL_DUPLEX);
>> +			if (!phydev->duplex &&
>> +					(phyi == PHY_INTERFACE_MODE_MII ||
>> +					 phyi == PHY_INTERFACE_MODE_RMII))
>
>
> Hmm....have you tested this on a GMII interface?

Nope, only RGMII.

>  *Technically*, full  
> duplex is required for GMII, as GMII is used only for gigabit.  However, 
> we've been treating the GMII interface type as an indicator that the PHY 
> *has* a GMII connection to the NIC.  When gianfar detects the speed is 
> 10/100 it switches to the compatible MII interface via this code, just 
> below:
>
>                         case 100:
>                         case 10:
>                                 tempval =
>                                     ((tempval & ~(MACCFG2_IF)) |  
> MACCFG2_MII);
>
>
> My concern is that you will be detecting the GMII interface, and  
> disallowing half-duplex, despite the fact that the interface is actually 
> running at 10 or 100 Mbit.

Very interesting, though I'm not sure I'm completely following. :-)

Are you saying that I should do this instead:

	if (!phydev->duplex &&
			(phyi == PHY_INTERFACE_MODE_MII ||
			 phyi == PHY_INTERFACE_MODE_RMII ||
			 (phyi == PHY_INTERFACE_MODE_GMII &&
			  phydev->speed < 1000)))
		tempval &= ~MACCFG2_FULL_DUPLEX;
	else
		tempval |= MACCFG2_FULL_DUPLEX;

i.e. we detected GMII interface initially, but it downgraded
to MII since speed is < 1000, thus we can set half-duplex in MAC?

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

  reply	other threads:[~2009-06-24 21:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-24 18:27 [PATCH] gianfar: Fix half-duplex operation for non-MII/RMII interfaces Anton Vorontsov
2009-06-24 18:27 ` Anton Vorontsov
2009-06-24 20:18 ` Andy Fleming
2009-06-24 21:10   ` Anton Vorontsov [this message]
2009-06-24 21:10     ` Anton Vorontsov
2009-06-24 21:25     ` Andy Fleming
2009-06-24 21:25       ` Andy Fleming
2009-06-24 21:39       ` Anton Vorontsov
2009-06-24 21:39         ` Anton Vorontsov
2009-06-24 21:50         ` Anton Vorontsov
2009-06-24 21:50           ` Anton Vorontsov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090624211038.GA29555@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=afleming@freescale.com \
    --cc=davem@davemloft.net \
    --cc=leoli@freescale.com \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.