[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

[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

[Shaohui Xie]

> -----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
>=20
> 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 val=
ues 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;
>=20
> 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 tw=
o new
> modes?
>=20
> With a bit of googling, K means copper backplane, X means 4B/5B and R mea=
ns
> 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.=20
There are interfaces could be different bit rates but same types,=20
e.g. 100BASE-LX10 and 1000BASE-LX10, or 40GBASE-KR4 and 100GBASE-KR4,=20
having bit rate is clear to represent hardware.

Thank you!

 Shaohui

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

[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@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.

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

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

[Shaohui Xie]

PiA+IElmIHlvdSBsb29rIGF0IHRoZSBsaXN0IG9mIHBvc3NpYmxlIHZhbHVlcyBmb3IgInBoeS1t
b2RlIiB5b3UnZCBzZWUNCj4gPiB0aGF0IG5vbmUgb2YgaXQgZGVzY3JpYmVzIGEgUEhZLXRvLVBI
WSBjb25uZWN0aW9uIGJ1dCBhbGwgYXJlIGZvcg0KPiA+IE1BQy10by1QSFkgY29ubmVjdGlvbnMu
IEFsc28sIG5hbWVzIGFib3ZlIHN1Z2dlc3QgaXQgYWxyZWFkeTogTUlJIGlzDQo+ID4gc2hvcnQg
Zm9yIG1lZGlhIF9pbmRlcGVuZGVudF8gaW50ZXJmYWNlLg0KPiA+DQo+ID4gSSBjb3B5IEFuZHJl
dydzIGNvbmNlcm5zIGFuZCB0aGluayB0aGF0IG5laXRoZXIgMTAwMDBiYXNlLWt4IG5vcg0KPiA+
IDEwZ2Jhc2Uta3IgYmVsb25nIGluIHRoZSBsaXN0IG9mIHBoeS1tb2RlIHByb3BlcnRpZXMuDQo+
IA0KPiBJIGNvbmN1ciB3aXRoIHRoYXQgYXMgd2VsbCwgaWYgdGhlIHBoeSBjb25uZWN0aW9uIGRv
ZXMgbm90IHJlYWxseSBtYXR0ZXIgaGVyZSwNCj4gb3IgZG9lcyBub3Qgc2VlbSBsaWtlIGEgZ29v
ZCBmaXQsIG1heWJlIHdlIHNob3VsZCBoYXZlIGEgZGlmZmVyZW50IHByb3BlcnR5LCBvcg0KPiBq
dXN0IGRlZmluZSB0aGUgaGFyZHdhcmUgaW50ZXJmYWNlIGEgbGl0dGxlIGRpZmZlcmVudGx5Pw0K
UmlnaHQsICdwaHktbW9kZScgaXMgbm90IGEgZ29vZCBmaXQgZm9yIGJhY2twbGFuZXMsIGhvdyBh
Ym91dCBhIG5ldyBwcm9wZXJ0eSBsaWtlDQonYmFja3BsYW5lLW1vZGUnIG9yIHNvbWV0aGluZywg
bGlrZSBiZWxvdzoNCg0KLS0tIGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL25l
dC9waHkudHh0DQorKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvbmV0L3Bo
eS50eHQNCkBAIC0zMyw2ICszMyw5IEBAIE9wdGlvbmFsIFByb3BlcnRpZXM6DQogLSBicm9rZW4t
dHVybi1hcm91bmQ6IElmIHNldCwgaW5kaWNhdGVzIHRoZSBQSFkgZGV2aWNlIGRvZXMgbm90IGNv
cnJlY3RseQ0KICAgcmVsZWFzZSB0aGUgdHVybiBhcm91bmQgbGluZSBsb3cgYXQgdGhlIGVuZCBv
ZiBhIE1ESU8gdHJhbnNhY3Rpb24uDQogDQorLSBiYWNrcGxhbmUtbW9kZTogc3RyaW5nLCBvcGVy
YXRpb24gbW9kZSBvZiB0aGUgYmFja3BsYW5lIFBIWTsNCisgIG11c3QgYmUgIjEwMDBiYXNlLWt4
IiBmb3IgMTAwMEJBU0UtS1gsIG9yICIxMGdiYXNlLWtyIiBmb3IgMTBHQkFTRS1LUi4NCisNCiBF
eGFtcGxlOg0KIA0KIGV0aGVybmV0LXBoeUAwIHsNCg0KVGhhbmsgeW91IQ0KDQpTaGFvaHVpDQo=

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

[Shaohui Xie]

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBTZWJhc3RpYW4gSGVzc2VsYmFy
dGggW21haWx0bzpzZWJhc3RpYW4uaGVzc2VsYmFydGhAZ21haWwuY29tXQ0KPiBTZW50OiBNb25k
YXksIEphbnVhcnkgMTgsIDIwMTYgNDowNiBQTQ0KPiBUbzogU2hhb2h1aSBYaWU7IEZsb3JpYW4g
RmFpbmVsbGk7IEFuZHJldyBMdW5uOyBzaGgueGllQGdtYWlsLmNvbQ0KPiBDYzogZGV2aWNldHJl
ZUB2Z2VyLmtlcm5lbC5vcmc7IG5ldGRldkB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBjLQ0KPiBk
ZXZAbGlzdHMub3psYWJzLm9yZzsgZGF2ZW1AZGF2ZW1sb2Z0Lm5ldDsgU2hhb2h1aSBYaWUNCj4g
U3ViamVjdDogUmU6IFtQQVRDSCAxLzNdW3YyXSBuZXQ6IHBoeTogaW50cm9kdWNlIDEwMDBCQVNF
LUtYIGFuZCAxMEdCQVNFLUtSDQo+IA0KPiBPbiAxOC4wMS4yMDE2IDA4OjIzLCBTaGFvaHVpIFhp
ZSB3cm90ZToNCj4gPj4+IElmIHlvdSBsb29rIGF0IHRoZSBsaXN0IG9mIHBvc3NpYmxlIHZhbHVl
cyBmb3IgInBoeS1tb2RlIiB5b3UnZCBzZWUNCj4gPj4+IHRoYXQgbm9uZSBvZiBpdCBkZXNjcmli
ZXMgYSBQSFktdG8tUEhZIGNvbm5lY3Rpb24gYnV0IGFsbCBhcmUgZm9yDQo+ID4+PiBNQUMtdG8t
UEhZIGNvbm5lY3Rpb25zLiBBbHNvLCBuYW1lcyBhYm92ZSBzdWdnZXN0IGl0IGFscmVhZHk6IE1J
SSBpcw0KPiA+Pj4gc2hvcnQgZm9yIG1lZGlhIF9pbmRlcGVuZGVudF8gaW50ZXJmYWNlLg0KPiA+
Pj4NCj4gPj4+IEkgY29weSBBbmRyZXcncyBjb25jZXJucyBhbmQgdGhpbmsgdGhhdCBuZWl0aGVy
IDEwMDAwYmFzZS1reCBub3INCj4gPj4+IDEwZ2Jhc2Uta3IgYmVsb25nIGluIHRoZSBsaXN0IG9m
IHBoeS1tb2RlIHByb3BlcnRpZXMuDQo+ID4+DQo+ID4+IEkgY29uY3VyIHdpdGggdGhhdCBhcyB3
ZWxsLCBpZiB0aGUgcGh5IGNvbm5lY3Rpb24gZG9lcyBub3QgcmVhbGx5DQo+ID4+IG1hdHRlciBo
ZXJlLCBvciBkb2VzIG5vdCBzZWVtIGxpa2UgYSBnb29kIGZpdCwgbWF5YmUgd2Ugc2hvdWxkIGhh
dmUgYQ0KPiA+PiBkaWZmZXJlbnQgcHJvcGVydHksIG9yIGp1c3QgZGVmaW5lIHRoZSBoYXJkd2Fy
ZSBpbnRlcmZhY2UgYSBsaXR0bGUNCj4gZGlmZmVyZW50bHk/DQo+ID4gUmlnaHQsICdwaHktbW9k
ZScgaXMgbm90IGEgZ29vZCBmaXQgZm9yIGJhY2twbGFuZXMsIGhvdyBhYm91dCBhIG5ldw0KPiA+
IHByb3BlcnR5IGxpa2UgJ2JhY2twbGFuZS1tb2RlJyBvciBzb21ldGhpbmcsIGxpa2UgYmVsb3c6
DQo+IA0KPiBIbW0uIFdlIGFscmVhZHkgaGF2ZSBhIHNwZWVkIHByb3BlcnR5IGZvciB0aGF0IHlv
dSBjYW4gdXNlIGZvciAxMDAwLCAxMDAwMCwNCj4gNDAwMDAuIExlYXZlcyB0aGUgbWVkaWEtdHlw
ZSwgZS5nLiBjb3BwZXIgb3Igd2hhdGV2ZXIuDQpbUy5IXSBZb3UgbWVhbiB0aGUgcHJvcGVydHkg
J21heC1zcGVlZCc/IHRoZSBwcm9ibGVtIGlzIHRoZSBtZWRpYS10eXBlIG1hdHRlcnMuDQpQbGVh
c2Ugc2VlIGJlbG93Lg0KDQo+IA0KPiBDdXJyZW50bHksIHlvdSBmYWlsIHRvIGNvbnZpbmNlIG1l
IHRoYXQgaXQgaXMgcmVxdWlyZWQgdG8gZGVzY3JpYmUgdGhlIG1lZGlhDQo+IHR5cGUgYXQgYWxs
LiBXZSBoYXZlIGNvbWUgYSBsb25nIHdheSB3aXRoIGRpZmZlcmVudCBtZWRpYSB3aXRob3V0IGRl
c2NyaWJpbmcgdGhlDQo+IFBIWS10by1QSFkgbWVkaWEgdHlwZS4NCj4gDQo+IFdoYXQgbWFrZXMg
dGhlIGJhY2twbGFuZSBzZXR1cCBzbyBzcGVjaWFsPw0KW1MuSF0gdGhlIGZzbCBiYWNrcGxhbmUs
IGUuZy4gMTBHQkFTRS1LUiwgbmVlZHMgc29mdHdhcmUgdG8gaGFuZGxlIGxpbmsgdHJhaW5pbmcs
DQpJdCdzIHRvIHRyYWluIGxpbmsgcGFydG5lciwgYW5kIHRyYWluZWQgYnkgbGluayBwYXJ0bmVy
IHBhcmFsbGVsLg0KDQpCdXQgaWYgbWVkaWEgdHlwZSBpcyBub3QgY29wcGVyLCBlLmcuIG9wdGlj
YWwgbW9kdWxlLCB3ZSB3b24ndCBuZWVkIHRoaXMuDQoNClRoYW5rIHlvdSENCg0KU2hhb2h1aSAN
Cg==

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

[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

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

[Shaohui Xie]

> -----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
>=20
> > [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 thi=
s.
>=20
> So what we actually need to know is copper vs fibre?
Copper is not enough to indicate backplane, since backplane is always coppe=
r,
 but copper is not always backplane.

Thank you!
Shaohui

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

[Andrew Lunn]

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....

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

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

[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; dav=
em@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 10GBAS=
E-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 partne=
r
> > parallel.
> > >
> > > But if media type is not copper, e.g. optical module, we won't need t=
his.
> >
> > 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=20
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 i=
ndicate=20
it's backplane, for ex. "copper + 1000" does not mean it has to be 1000BASE=
-KX,=20
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 s=
pecific
binding, like the second patch 2/3 did.

Thank you!
Shaohui=

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

[Sebastian Hesselbarth]

On 22.01.2016 09:15, Shaohui Xie wrote:
> _______________________________________
> 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.

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

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]

>> 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 ba=
sed 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,=20
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 onl=
y
AN to KR if both sides support KR, it cannot give "40g" or anything differe=
nt,=20
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 1000BA=
SE-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

[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 differe=
nt PHY init
routines, this also needs different serdes lane settings, this is done in p=
robe() 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-K=
X 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 b=
inding 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=20
should work in backplane, hardware can deal with different modes, or even n=
o any special=20
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@gmail.com; devicetree@vger.kernel.org; netdev=
@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; davem@davemloft.net; Shaoh=
ui 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 ba=
sed 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 onl=
y
AN to KR if both sides support KR, it cannot give "40g" or anything differe=
nt,
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 1000BA=
SE-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

[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

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