All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* Question about settings array in phy.c
@ 2014-07-11 15:50 Tom Lendacky
  2014-07-11 17:21 ` Florian Fainelli
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Lendacky @ 2014-07-11 15:50 UTC (permalink / raw
  To: netdev, Florian Fainelli; +Cc: David Miller

In drivers/net/phy/phy.c there is an array of struct phy_setting named
settings that is used when auto negotiation is disabled to sanitize
the phy settings.  This array uses only *baseT* features for validating
the various speed/duplex combinations.  This may result in not finding
the correct array entry in regards to speed and duplex if the *baseT*
feature is not supported.

For example, a 10GbE phy that supports KR mode would end up not matching
an entry and defaulting to the last entry of the array (SPEED_10 and
DUPLEX_HALF) which is not what is desired.

Is it appropriate to be able to extend this array?  Either by adding
additional array entries or extending the "setting" to include other
features?  Or is there a specific reason that only the *baseT* values
are used?

Examples of changing the array:

   Add a new entry
	{
		.speed = SPEED_10000,
		.duplex = DUPLEX_FULL,
		.setting = SUPPORTED_10000baseKR_Full,
	},

   or extend current entry
	{
		.speed = SPEED_10000,
		.duplex = DUPLEX_FULL,
		.setting = SUPPORTED_10000baseT_Full |
			   SUPPORTED_10000baseKR_Full,
	},

Thanks,
Tom

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Question about settings array in phy.c
  2014-07-11 15:50 Question about settings array in phy.c Tom Lendacky
@ 2014-07-11 17:21 ` Florian Fainelli
  0 siblings, 0 replies; 2+ messages in thread
From: Florian Fainelli @ 2014-07-11 17:21 UTC (permalink / raw
  To: Tom Lendacky; +Cc: netdev, David Miller

Hi Tom,

2014-07-11 8:50 GMT-07:00 Tom Lendacky <thomas.lendacky@amd.com>:
> In drivers/net/phy/phy.c there is an array of struct phy_setting named
> settings that is used when auto negotiation is disabled to sanitize
> the phy settings.  This array uses only *baseT* features for validating
> the various speed/duplex combinations.  This may result in not finding
> the correct array entry in regards to speed and duplex if the *baseT*
> feature is not supported.

Part of the reason is that libphy was designed around 2006-2007 and
initially only supported 10/100/1000 PHY devices, and until recently
it did not support 10GbE PHYs at all.

>
> For example, a 10GbE phy that supports KR mode would end up not matching
> an entry and defaulting to the last entry of the array (SPEED_10 and
> DUPLEX_HALF) which is not what is desired.
>
> Is it appropriate to be able to extend this array?  Either by adding
> additional array entries or extending the "setting" to include other
> features?  Or is there a specific reason that only the *baseT* values
> are used?

It seems to me like it will be easier to refine/extend the matching
logic by adding a new entries which contain a single 'setting' to be
matched against, such as the one you show below:

>
> Examples of changing the array:
>
>   Add a new entry
>         {
>                 .speed = SPEED_10000,
>                 .duplex = DUPLEX_FULL,
>                 .setting = SUPPORTED_10000baseKR_Full,
>         },
>
>   or extend current entry
>         {
>                 .speed = SPEED_10000,
>                 .duplex = DUPLEX_FULL,
>                 .setting = SUPPORTED_10000baseT_Full |
>                            SUPPORTED_10000baseKR_Full,
>         },
>
> Thanks,
> Tom



-- 
Florian

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-07-11 17:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-11 15:50 Question about settings array in phy.c Tom Lendacky
2014-07-11 17:21 ` Florian Fainelli

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.