[PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[shh.xie]

From: Shaohui Xie <Shaohui.Xie@freescale.com>

This commit adds necessary definitions for the PHY layer to recognize
backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
"1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.

Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
changes in v2:
new patch.

 Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
 include/linux/phy.h                                | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 5d88f37..1166a5c 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -11,8 +11,8 @@ The following properties are common to the Ethernet controllers:
   the maximum frame size (there's contradiction in ePAPR).
 - phy-mode: string, operation mode of the PHY interface; supported values are
   "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
-  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
-  standard property;
+  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx", "10gbase-kr";
+  this is now a de-facto standard property;
 - phy-connection-type: the same as "phy-mode" property but described in ePAPR;
 - phy-handle: phandle, specifies a reference to a node representing a PHY
   device; this property is described in ePAPR and so preferred;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index d6f3641..7acecf7 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -80,6 +80,8 @@ typedef enum {
 	PHY_INTERFACE_MODE_XGMII,
 	PHY_INTERFACE_MODE_MOCA,
 	PHY_INTERFACE_MODE_QSGMII,
+	PHY_INTERFACE_MODE_1000BASE_KX,
+	PHY_INTERFACE_MODE_10GBASE_KR,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -123,6 +125,10 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "moca";
 	case PHY_INTERFACE_MODE_QSGMII:
 		return "qsgmii";
+	case PHY_INTERFACE_MODE_1000BASE_KX:
+		return "1000base-kx";
+	case PHY_INTERFACE_MODE_10GBASE_KR:
+		return "10gbase-kr";
 	default:
 		return "unknown";
 	}
-- 
2.1.0.27.g96db324

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Andrew Lunn]

On Thu, Jan 14, 2016 at 04:23:59PM +0800, shh.xie@gmail.com wrote:
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
> 
> This commit adds necessary definitions for the PHY layer to recognize
> backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
> "1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.
> 
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---
> changes in v2:
> new patch.
> 
>  Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
>  include/linux/phy.h                                | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
> index 5d88f37..1166a5c 100644
> --- a/Documentation/devicetree/bindings/net/ethernet.txt
> +++ b/Documentation/devicetree/bindings/net/ethernet.txt
> @@ -11,8 +11,8 @@ The following properties are common to the Ethernet controllers:
>    the maximum frame size (there's contradiction in ePAPR).
>  - phy-mode: string, operation mode of the PHY interface; supported values are
>    "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
> -  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
> -  standard property;
> +  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx", "10gbase-kr";
> +  this is now a de-facto standard property;

I know very little about this, so i'm just asking a question. None of
the other interface modes contain a bit rate. So is the bit rate
needed for your two new modes?

With a bit of googling, K means copper backplane, X means 4B/5B and R
means 64B/66B. Could there be a 10Gbps KX? a 1GBps KR? Do we actually
need the speed here, or is kx and kr sufficient?

     Thanks
	Andrew

RE: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> Sent: Friday, January 15, 2016 12:44 AM
> To: shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
> 
> On Thu, Jan 14, 2016 at 04:23:59PM +0800, shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> > From: Shaohui Xie <Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >
> > This commit adds necessary definitions for the PHY layer to recognize
> > backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
> > "1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.
> >
> > Signed-off-by: Shaohui Xie <Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> > changes in v2:
> > new patch.
> >
> >  Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
> >  include/linux/phy.h                                | 6 ++++++
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
> > b/Documentation/devicetree/bindings/net/ethernet.txt
> > index 5d88f37..1166a5c 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet.txt
> > +++ b/Documentation/devicetree/bindings/net/ethernet.txt
> > @@ -11,8 +11,8 @@ The following properties are common to the Ethernet
> controllers:
> >    the maximum frame size (there's contradiction in ePAPR).
> >  - phy-mode: string, operation mode of the PHY interface; supported values are
> >    "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii",
> > "rgmii", "rgmii-id",
> > -  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a
> > de-facto
> > -  standard property;
> > +  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx",
> > + "10gbase-kr";  this is now a de-facto standard property;
> 
> I know very little about this, so i'm just asking a question. None of the other
> interface modes contain a bit rate. So is the bit rate needed for your two new
> modes?
> 
> With a bit of googling, K means copper backplane, X means 4B/5B and R means
> 64B/66B. Could there be a 10Gbps KX? a 1GBps KR? Do we actually need the speed
> here, or is kx and kr sufficient?
Hello Andrew,

1000BASE-KX and 10GBASE-KR are terms in IEEE802.3, so as XGMII and GMII. 
There are interfaces could be different bit rates but same types, 
e.g. 100BASE-LX10 and 1000BASE-LX10, or 40GBASE-KR4 and 100GBASE-KR4, 
having bit rate is clear to represent hardware.

Thank you!

 Shaohui
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Sebastian Hesselbarth]

On 15.01.2016 05:01, Shaohui Xie wrote:
>> -----Original Message-----
>> From: Andrew Lunn [mailto:andrew@lunn.ch]
>> Sent: Friday, January 15, 2016 12:44 AM
>> To: shh.xie@gmail.com
>> Cc: devicetree@vger.kernel.org; netdev@vger.kernel.org; linuxppc-
>> dev@lists.ozlabs.org; f.fainelli@gmail.com; davem@davemloft.net; Shaohui Xie
>> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
>>
>> On Thu, Jan 14, 2016 at 04:23:59PM +0800, shh.xie@gmail.com wrote:
>>> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>>>
>>> This commit adds necessary definitions for the PHY layer to recognize
>>> backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
>>> "1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.
>>>
>>> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>>> ---
>>> changes in v2:
>>> new patch.

Shaohui,

it would be more useful to describe _what_ is new here compared to v1.

Anyway:

>>>  Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
>>>  include/linux/phy.h                                | 6 ++++++
>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
>>> b/Documentation/devicetree/bindings/net/ethernet.txt
>>> index 5d88f37..1166a5c 100644
>>> --- a/Documentation/devicetree/bindings/net/ethernet.txt
>>> +++ b/Documentation/devicetree/bindings/net/ethernet.txt
>>> @@ -11,8 +11,8 @@ The following properties are common to the Ethernet
>> controllers:
>>>    the maximum frame size (there's contradiction in ePAPR).
>>>  - phy-mode: string, operation mode of the PHY interface; supported values are
>>>    "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii",
>>> "rgmii", "rgmii-id",
>>> -  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a
>>> de-facto
>>> -  standard property;
>>> +  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx",
>>> + "10gbase-kr";  this is now a de-facto standard property;
>>
>> I know very little about this, so i'm just asking a question. None of the other
>> interface modes contain a bit rate. So is the bit rate needed for your two new
>> modes?
> 
> 1000BASE-KX and 10GBASE-KR are terms in IEEE802.3, so as XGMII and GMII. 
> There are interfaces could be different bit rates but same types, 
> e.g. 100BASE-LX10 and 1000BASE-LX10, or 40GBASE-KR4 and 100GBASE-KR4, 
> having bit rate is clear to represent hardware.
> 

If you look at the list of possible values for "phy-mode" you'd see that
none of it describes a PHY-to-PHY connection but all are for MAC-to-PHY
connections. Also, names above suggest it already: MII is short for
media _independent_ interface.

I copy Andrew's concerns and think that neither 10000base-kx nor
10gbase-kr belong in the list of phy-mode properties.

Sebastian

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Florian Fainelli]

Le 15/01/2016 14:57, Sebastian Hesselbarth a écrit :
> On 15.01.2016 05:01, Shaohui Xie wrote:
>>> -----Original Message-----
>>> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
>>> Sent: Friday, January 15, 2016 12:44 AM
>>> To: shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
>>> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
>>> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
>>>
>>> On Thu, Jan 14, 2016 at 04:23:59PM +0800, shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>> From: Shaohui Xie <Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>>>
>>>> This commit adds necessary definitions for the PHY layer to recognize
>>>> backplane Ethernet 1000BASE-KX and 10GBASE-KR as valid PHY interfaces,
>>>> "1000base-kx" for 1000BASE-KX, "10gbase-kr" for 10GBASE-KR.
>>>>
>>>> Signed-off-by: Shaohui Xie <Shaohui.Xie-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>>> ---
>>>> changes in v2:
>>>> new patch.
> 
> Shaohui,
> 
> it would be more useful to describe _what_ is new here compared to v1.
> 
> Anyway:
> 
>>>>  Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
>>>>  include/linux/phy.h                                | 6 ++++++
>>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/ethernet.txt
>>>> b/Documentation/devicetree/bindings/net/ethernet.txt
>>>> index 5d88f37..1166a5c 100644
>>>> --- a/Documentation/devicetree/bindings/net/ethernet.txt
>>>> +++ b/Documentation/devicetree/bindings/net/ethernet.txt
>>>> @@ -11,8 +11,8 @@ The following properties are common to the Ethernet
>>> controllers:
>>>>    the maximum frame size (there's contradiction in ePAPR).
>>>>  - phy-mode: string, operation mode of the PHY interface; supported values are
>>>>    "mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii",
>>>> "rgmii", "rgmii-id",
>>>> -  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a
>>>> de-facto
>>>> -  standard property;
>>>> +  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "1000base-kx",
>>>> + "10gbase-kr";  this is now a de-facto standard property;
>>>
>>> I know very little about this, so i'm just asking a question. None of the other
>>> interface modes contain a bit rate. So is the bit rate needed for your two new
>>> modes?
>>
>> 1000BASE-KX and 10GBASE-KR are terms in IEEE802.3, so as XGMII and GMII. 
>> There are interfaces could be different bit rates but same types, 
>> e.g. 100BASE-LX10 and 1000BASE-LX10, or 40GBASE-KR4 and 100GBASE-KR4, 
>> having bit rate is clear to represent hardware.
>>
> 
> If you look at the list of possible values for "phy-mode" you'd see that
> none of it describes a PHY-to-PHY connection but all are for MAC-to-PHY
> connections. Also, names above suggest it already: MII is short for
> media _independent_ interface.
> 
> I copy Andrew's concerns and think that neither 10000base-kx nor
> 10gbase-kr belong in the list of phy-mode properties.

I concur with that as well, if the phy connection does not really matter
here, or does not seem like a good fit, maybe we should have a different
property, or just define the hardware interface a little differently?
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

> > If you look at the list of possible values for "phy-mode" you'd see
> > that none of it describes a PHY-to-PHY connection but all are for
> > MAC-to-PHY connections. Also, names above suggest it already: MII is
> > short for media _independent_ interface.
> >
> > I copy Andrew's concerns and think that neither 10000base-kx nor
> > 10gbase-kr belong in the list of phy-mode properties.
> 
> I concur with that as well, if the phy connection does not really matter here,
> or does not seem like a good fit, maybe we should have a different property, or
> just define the hardware interface a little differently?
Right, 'phy-mode' is not a good fit for backplanes, how about a new property like
'backplane-mode' or something, like below:

--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -33,6 +33,9 @@ Optional Properties:
 - broken-turn-around: If set, indicates the PHY device does not correctly
   release the turn around line low at the end of a MDIO transaction.
 
+- backplane-mode: string, operation mode of the backplane PHY;
+  must be "1000base-kx" for 1000BASE-KX, or "10gbase-kr" for 10GBASE-KR.
+
 Example:
 
 ethernet-phy@0 {

Thank you!

Shaohui

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Sebastian Hesselbarth]

On 18.01.2016 08:23, Shaohui Xie wrote:
>>> If you look at the list of possible values for "phy-mode" you'd see
>>> that none of it describes a PHY-to-PHY connection but all are for
>>> MAC-to-PHY connections. Also, names above suggest it already: MII is
>>> short for media _independent_ interface.
>>>
>>> I copy Andrew's concerns and think that neither 10000base-kx nor
>>> 10gbase-kr belong in the list of phy-mode properties.
>>
>> I concur with that as well, if the phy connection does not really matter here,
>> or does not seem like a good fit, maybe we should have a different property, or
>> just define the hardware interface a little differently?
> Right, 'phy-mode' is not a good fit for backplanes, how about a new property like
> 'backplane-mode' or something, like below:

Hmm. We already have a speed property for that you can use for
1000, 10000, 40000. Leaves the media-type, e.g. copper or whatever.

Currently, you fail to convince me that it is required to describe
the media type at all. We have come a long way with different media
without describing the PHY-to-PHY media type.

What makes the backplane setup so special?

Sebastian

>
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -33,6 +33,9 @@ Optional Properties:
>   - broken-turn-around: If set, indicates the PHY device does not correctly
>     release the turn around line low at the end of a MDIO transaction.
>
> +- backplane-mode: string, operation mode of the backplane PHY;
> +  must be "1000base-kx" for 1000BASE-KX, or "10gbase-kr" for 10GBASE-KR.
> +
>   Example:
>
>   ethernet-phy@0 {
>
> Thank you!
>
> Shaohui
>

RE: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

> -----Original Message-----
> From: Sebastian Hesselbarth [mailto:sebastian.hesselbarth@gmail.com]
> Sent: Monday, January 18, 2016 4:06 PM
> To: Shaohui Xie; Florian Fainelli; Andrew Lunn; shh.xie@gmail.com
> Cc: devicetree@vger.kernel.org; netdev@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; davem@davemloft.net; Shaohui Xie
> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
> 
> On 18.01.2016 08:23, Shaohui Xie wrote:
> >>> If you look at the list of possible values for "phy-mode" you'd see
> >>> that none of it describes a PHY-to-PHY connection but all are for
> >>> MAC-to-PHY connections. Also, names above suggest it already: MII is
> >>> short for media _independent_ interface.
> >>>
> >>> I copy Andrew's concerns and think that neither 10000base-kx nor
> >>> 10gbase-kr belong in the list of phy-mode properties.
> >>
> >> I concur with that as well, if the phy connection does not really
> >> matter here, or does not seem like a good fit, maybe we should have a
> >> different property, or just define the hardware interface a little
> differently?
> > Right, 'phy-mode' is not a good fit for backplanes, how about a new
> > property like 'backplane-mode' or something, like below:
> 
> Hmm. We already have a speed property for that you can use for 1000, 10000,
> 40000. Leaves the media-type, e.g. copper or whatever.
[S.H] You mean the property 'max-speed'? the problem is the media-type matters.
Please see below.

> 
> Currently, you fail to convince me that it is required to describe the media
> type at all. We have come a long way with different media without describing the
> PHY-to-PHY media type.
> 
> What makes the backplane setup so special?
[S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle link training,
It's to train link partner, and trained by link partner parallel.

But if media type is not copper, e.g. optical module, we won't need this.

Thank you!

Shaohui 

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Andrew Lunn]

> [S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle link training,
> It's to train link partner, and trained by link partner parallel.
> 
> But if media type is not copper, e.g. optical module, we won't need this.

So what we actually need to know is copper vs fibre?

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> Sent: Monday, January 18, 2016 11:15 PM
> To: Shaohui Xie
> Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
> 
> > [S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle
> > link training, It's to train link partner, and trained by link partner
> parallel.
> >
> > But if media type is not copper, e.g. optical module, we won't need this.
> 
> So what we actually need to know is copper vs fibre?
Copper is not enough to indicate backplane, since backplane is always copper,
 but copper is not always backplane.

Thank you!
Shaohui
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Andrew Lunn]

On Tue, Jan 19, 2016 at 05:00:35AM +0000, Shaohui Xie wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> > Sent: Monday, January 18, 2016 11:15 PM
> > To: Shaohui Xie
> > Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
> > devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
> > dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
> > Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
> > 
> > > [S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle
> > > link training, It's to train link partner, and trained by link partner
> > parallel.
> > >
> > > But if media type is not copper, e.g. optical module, we won't need this.
> > 
> > So what we actually need to know is copper vs fibre?

> Copper is not enough to indicate backplane, since backplane is
> always copper, but copper is not always backplane.

O.K, lets try again....

If it is copper backplane you need to perform training.

Looking at the driver probe function, it is either 1000BASE-KX, no
training needed, or else it is 10GBASE-RK and training is needed.

Looking at fsl_backplane_config_aneg() you expect phydev->speed to be
set, and from the speed you then kick of either KR autoneg or KX
autoneg. Could you also start the training at this point? Use the
speed to indicate if training is needed?

      Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

_______________________________________
From: Andrew Lunn <andrew@lunn.ch>
Sent: Friday, January 22, 2016 5:12 AM
To: Shaohui Xie
Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie@gmail.com; devicetree@vger.kernel.org; netdev@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; davem@davemloft.net; Shaohui Xie
Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

On Tue, Jan 19, 2016 at 05:00:35AM +0000, Shaohui Xie wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Monday, January 18, 2016 11:15 PM
> > To: Shaohui Xie
> > Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie@gmail.com;
> > devicetree@vger.kernel.org; netdev@vger.kernel.org; linuxppc-
> > dev@lists.ozlabs.org; davem@davemloft.net; Shaohui Xie
> > Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
> >
> > > [S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle
> > > link training, It's to train link partner, and trained by link partner
> > parallel.
> > >
> > > But if media type is not copper, e.g. optical module, we won't need this.
> >
> > So what we actually need to know is copper vs fibre?

> Copper is not enough to indicate backplane, since backplane is
> always copper, but copper is not always backplane.

>O.K, lets try again....

[S.H]Seems I did not get your point, Sorry for the inconvenient.

>If it is copper backplane you need to perform training.

>Looking at the driver probe function, it is either 1000BASE-KX, no
>training needed, or else it is 10GBASE-RK and training is needed.

>Looking at fsl_backplane_config_aneg() you expect phydev->speed to be
>set, and from the speed you then kick of either KR autoneg or KX
>autoneg. Could you also start the training at this point? Use the
>speed to indicate if training is needed?

 [S.H]The training cannot be started at this point, yet, because it's based on 
autoneg result, only when both sides autoneg-ed to 10G-KR, then to start
the training.

Besides the driver, generally speaking, "copper + speed" is not enough to indicate 
it's backplane, for ex. "copper + 1000" does not mean it has to be 1000BASE-KX, 
it could be SGMII, hence cannot use KX autoneg.

If putting backplane property to phy.txt is not good, I can put it to fsl specific
binding, like the second patch 2/3 did.

Thank you!
Shaohui
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Sebastian Hesselbarth]

On 22.01.2016 09:15, Shaohui Xie wrote:
> _______________________________________
> From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
> Sent: Friday, January 22, 2016 5:12 AM
> To: Shaohui Xie
> Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
>
> On Tue, Jan 19, 2016 at 05:00:35AM +0000, Shaohui Xie wrote:
>>> -----Original Message-----
>>> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
>>> Sent: Monday, January 18, 2016 11:15 PM
>>> To: Shaohui Xie
>>> Cc: Sebastian Hesselbarth; Florian Fainelli; shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
>>> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-
>>> dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
>>> Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR
>>>
>>>> [S.H] the fsl backplane, e.g. 10GBASE-KR, needs software to handle
>>>> link training, It's to train link partner, and trained by link partner
>>> parallel.
>>>>
>>>> But if media type is not copper, e.g. optical module, we won't need this.
>>>
>>> So what we actually need to know is copper vs fibre?
>
>> Copper is not enough to indicate backplane, since backplane is
>> always copper, but copper is not always backplane.
>
>> O.K, lets try again....
>
> [S.H]Seems I did not get your point, Sorry for the inconvenient.
>
>> If it is copper backplane you need to perform training.
>
>> Looking at the driver probe function, it is either 1000BASE-KX, no
>> training needed, or else it is 10GBASE-RK and training is needed.
>
>> Looking at fsl_backplane_config_aneg() you expect phydev->speed to be
>> set, and from the speed you then kick of either KR autoneg or KX
>> autoneg. Could you also start the training at this point? Use the
>> speed to indicate if training is needed?
>
>   [S.H]The training cannot be started at this point, yet, because it's based on
> autoneg result, only when both sides autoneg-ed to 10G-KR, then to start
> the training.

Shaohui,

look, we want you to convince us why to have a generic backplane
property in the phy binding. You had a bad start by adding new
property values to a property that does not relate to your use
case at all. Your job now really is to give strong reasons _why_
and _what_ a phy driver needs to know about the backplane setup.

Your first approach was to add "10gbase-rk" or "40gbase-foo" but
now you tell us about ANEG. Of what use is the information given
by the property when ANEG tells you something different? E.g.
consider the property tells you "10g-something" but ANEG gives
you "40g"; does the property add any value to your training
decision now?

> Besides the driver, generally speaking, "copper + speed" is not enough to indicate
> it's backplane, for ex. "copper + 1000" does not mean it has to be 1000BASE-KX,
> it could be SGMII, hence cannot use KX autoneg.

So, is it copper + speed + backplane? or speed + backplane? And out of
the set of required input, is there anything your _cannot_ determine
from other things, e.g. ANEG?

If it is backplane only, would a boolean property ("backplane-mode")
be enough for the training decision?

> If putting backplane property to phy.txt is not good, I can put it to fsl specific
> binding, like the second patch 2/3 did.

You seem to see vendor specific properties as a place to dump all your
waste you don't want to think about. You fail to give good reasons what
is so special about the backplane setup and now you are telling us that
it is even fsl-specific?

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

>> Looking at fsl_backplane_config_aneg() you expect phydev->speed to be
>> set, and from the speed you then kick of either KR autoneg or KX
>> autoneg. Could you also start the training at this point? Use the
>> speed to indicate if training is needed?
>
>   [S.H]The training cannot be started at this point, yet, because it's based on
> autoneg result, only when both sides autoneg-ed to 10G-KR, then to start
> the training.

Shaohui,

look, we want you to convince us why to have a generic backplane
property in the phy binding. You had a bad start by adding new
property values to a property that does not relate to your use
case at all. Your job now really is to give strong reasons _why_
and _what_ a phy driver needs to know about the backplane setup.

[S.H] The PHY driver needs to know what the backplane mode is, 
1000BASE-KX or 10GBASE-KR, the driver parses the property for
"1000base-kx" or "10gbase-kr", the driver does use the property,
I don't understand why the property is not related to my use case?

Your first approach was to add "10gbase-rk" or "40gbase-foo" but
now you tell us about ANEG. Of what use is the information given
by the property when ANEG tells you something different? E.g.
consider the property tells you "10g-something" but ANEG gives
you "40g"; does the property add any value to your training
decision now?

[S.H] The ANEG is not a gerneric AN, it's special to 10G-KR,  KR AN can only
AN to KR if both sides support KR, it cannot give "40g" or anything different, 
drive needs the property to do speicific init.

> Besides the driver, generally speaking, "copper + speed" is not enough to indicate
> it's backplane, for ex. "copper + 1000" does not mean it has to be 1000BASE-KX,
> it could be SGMII, hence cannot use KX autoneg.

So, is it copper + speed + backplane? or speed + backplane? And out of
the set of required input, is there anything your _cannot_ determine
from other things, e.g. ANEG?

[S.H] Backplane is enough, 1000BASE-KX means : copper + 1000 + KX stuff.
The KR AN is to check whether LP is also KR, if yes, do training, otherwise, do nothing.

If it is backplane only, would a boolean property ("backplane-mode")
be enough for the training decision?

[S.H] a boolean property is not enough, there are different backplane modes.

> If putting backplane property to phy.txt is not good, I can put it to fsl specific
> binding, like the second patch 2/3 did.

You seem to see vendor specific properties as a place to dump all your
waste you don't want to think about. You fail to give good reasons what
is so special about the backplane setup and now you are telling us that
it is even fsl-specific?

[S.H] I don't think it's waste, I just thought it might be special to fsl.
Agreed I failed to give good reasons, but I tried hard to. :(

Thanks,
Shaohui

Sebastian

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Shaohui Xie]

Hi,

I'm not sure I explained myself clearly in previous reply, so I guess it's worth to
rephrase it.

1000BASE-KX and 10GBASE-KR are backplane modes supported by Freescale's PCS
PHY, but different modes need different hardware settings, not just different PHY init
routines, this also needs different serdes lane settings, this is done in probe() according
to DTS properties.

Regarding the autoneg part, it's not like normal autoneg which can autoneg to different
capabilities, when the PHY is 1000BSE-KX, it can only autoneg to 1000BASE-KX only if
LP is 1000BASE-KX,  same is true for 10GBASE-KR. The purpose of KR AN is to detect whether
LP is also KR, if yes, do training, if not, do nothing, no any other result the KR AN can give.

The reason "copper + speed" is not enough for backplane is because they are not precise,
and backplane itself explains what it is, for ex. 1000BASE-KX clearly means the media is copper,
speed is 1000, and should follow 1000BASE-KX standard in IEEE802.3.

The reason I mentioned maybe I should put the backplane property in fsl's binding is because
the backplane implementation is really vendor specific, it's heavily relay how hardware implements
the feature, maybe another vendor's hardware only needs a boolean property for driver to tell it 
should work in backplane, hardware can deal with different modes, or even no any special 
property needed if the hardware is strong enough to handle any connections, I cannot assume.
But I know what fsl's hardware needs to support backplane.

Thank you for your time and reviewing!
Shaohui

________________________________________
From: Shaohui Xie
Sent: Friday, January 22, 2016 6:05 PM
To: Sebastian Hesselbarth; Andrew Lunn
Cc: Florian Fainelli; shh.xie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org; davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Shaohui Xie
Subject: Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

>> Looking at fsl_backplane_config_aneg() you expect phydev->speed to be
>> set, and from the speed you then kick of either KR autoneg or KX
>> autoneg. Could you also start the training at this point? Use the
>> speed to indicate if training is needed?
>
>   [S.H]The training cannot be started at this point, yet, because it's based on
> autoneg result, only when both sides autoneg-ed to 10G-KR, then to start
> the training.

Shaohui,

look, we want you to convince us why to have a generic backplane
property in the phy binding. You had a bad start by adding new
property values to a property that does not relate to your use
case at all. Your job now really is to give strong reasons _why_
and _what_ a phy driver needs to know about the backplane setup.

[S.H] The PHY driver needs to know what the backplane mode is,
1000BASE-KX or 10GBASE-KR, the driver parses the property for
"1000base-kx" or "10gbase-kr", the driver does use the property,
I don't understand why the property is not related to my use case?

Your first approach was to add "10gbase-rk" or "40gbase-foo" but
now you tell us about ANEG. Of what use is the information given
by the property when ANEG tells you something different? E.g.
consider the property tells you "10g-something" but ANEG gives
you "40g"; does the property add any value to your training
decision now?

[S.H] The ANEG is not a gerneric AN, it's special to 10G-KR,  KR AN can only
AN to KR if both sides support KR, it cannot give "40g" or anything different,
drive needs the property to do speicific init.

> Besides the driver, generally speaking, "copper + speed" is not enough to indicate
> it's backplane, for ex. "copper + 1000" does not mean it has to be 1000BASE-KX,
> it could be SGMII, hence cannot use KX autoneg.

So, is it copper + speed + backplane? or speed + backplane? And out of
the set of required input, is there anything your _cannot_ determine
from other things, e.g. ANEG?

[S.H] Backplane is enough, 1000BASE-KX means : copper + 1000 + KX stuff.
The KR AN is to check whether LP is also KR, if yes, do training, otherwise, do nothing.

If it is backplane only, would a boolean property ("backplane-mode")
be enough for the training decision?

[S.H] a boolean property is not enough, there are different backplane modes.

> If putting backplane property to phy.txt is not good, I can put it to fsl specific
> binding, like the second patch 2/3 did.

You seem to see vendor specific properties as a place to dump all your
waste you don't want to think about. You fail to give good reasons what
is so special about the backplane setup and now you are telling us that
it is even fsl-specific?

[S.H] I don't think it's waste, I just thought it might be special to fsl.
Agreed I failed to give good reasons, but I tried hard to. :(

Thanks,
Shaohui

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3][v2] net: phy: introduce 1000BASE-KX and 10GBASE-KR

[Andrew Lunn]

> The reason I mentioned maybe I should put the backplane property in
> fsl's binding is because the backplane implementation is really
> vendor specific, it's heavily relay how hardware implements the
> feature, maybe another vendor's hardware only needs a boolean
> property for driver to tell it should work in backplane, hardware
> can deal with different modes, or even no any special property
> needed if the hardware is strong enough to handle any connections, I
> cannot assume.  But I know what fsl's hardware needs to support
> backplane.

This is the key point really. We don't really care about the Freescale
PCS. We want a generic solution for 1000BASE-KX and 10GBASE-KR,
independent of who makes it, Marvell, Micrel, Broadcom, or Acme.

So, what generic properties are needed for 1000BASE-KX and 10GBASE-KR?
Properties that most/all manufactures are likely to need?

	   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html