| Message ID | 20220321152515.287119-3-andy.chiu@sifive.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v4,1/4] net: axienet: setup mdio unconditionally | expand |
> -----Original Message----- > From: Andy Chiu <andy.chiu@sifive.com> > Sent: Monday, March 21, 2022 8:55 PM > To: Radhey Shyam Pandey <radheys@xilinx.com>; robert.hancock@calian.com; > Michal Simek <michals@xilinx.com> > Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com; > robh+dt@kernel.org; linux@armlinux.org.uk; andrew@lunn.ch; > netdev@vger.kernel.org; devicetree@vger.kernel.org; Andy Chiu > <andy.chiu@sifive.com>; Greentime Hu <greentime.hu@sifive.com> > Subject: [PATCH v4 3/4] dt-bindings: net: xilinx_axienet: add pcs-handle > attribute > > Document the new pcs-handle attribute to support connecting to an external > PHY in SGMII or 1000Base-X modes through the internal PCS/PMA PHY. > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > Reviewed-by: Greentime Hu <greentime.hu@sifive.com> > --- > Documentation/devicetree/bindings/net/xilinx_axienet.txt | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > index b8e4894bc634..ba720a2ea5fc 100644 > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > @@ -26,7 +26,8 @@ Required properties: > specified, the TX/RX DMA interrupts should be on that node > instead, and only the Ethernet core interrupt is optionally > specified here. > -- phy-handle : Should point to the external phy device. > +- phy-handle : Should point to the external phy device if exists. Pointing > + this to the PCS/PMA PHY is deprecated and should be > avoided. > See ethernet.txt file in the same directory. > - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the hardware > > @@ -68,6 +69,11 @@ Optional properties: > required through the core's MDIO interface (i.e. always, > unless the PHY is accessed through a different bus). > > + - pcs-handle: Phandle to the internal PCS/PMA PHY in SGMII or 1000Base-X > + modes, where "pcs-handle" should be preferably used to > point > + to the PCS/PMA PHY, and "phy-handle" should point to an > + external PHY if exists. I would like to have Rob feedback on this pcs-handle DT property. The use case is generic i.e. require separate handle to internal SGMII and external Phy so would prefer this new DT convention is standardized or we discuss possible approaches on how to handle both phys and not add it as vendor specific property in the first place. > + > Example: > axi_ethernet_eth: ethernet@40c00000 { > compatible = "xlnx,axi-ethernet-1.00.a"; > -- > 2.34.1
On Mon, 2022-03-21 at 15:42 +0000, Radhey Shyam Pandey wrote: > > -----Original Message----- > > From: Andy Chiu <andy.chiu@sifive.com> > > Sent: Monday, March 21, 2022 8:55 PM > > To: Radhey Shyam Pandey <radheys@xilinx.com>; robert.hancock@calian.com; > > Michal Simek <michals@xilinx.com> > > Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com; > > robh+dt@kernel.org; linux@armlinux.org.uk; andrew@lunn.ch; > > netdev@vger.kernel.org; devicetree@vger.kernel.org; Andy Chiu > > <andy.chiu@sifive.com>; Greentime Hu <greentime.hu@sifive.com> > > Subject: [PATCH v4 3/4] dt-bindings: net: xilinx_axienet: add pcs-handle > > attribute > > > > Document the new pcs-handle attribute to support connecting to an external > > PHY in SGMII or 1000Base-X modes through the internal PCS/PMA PHY. > > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > Reviewed-by: Greentime Hu <greentime.hu@sifive.com> > > --- > > Documentation/devicetree/bindings/net/xilinx_axienet.txt | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > index b8e4894bc634..ba720a2ea5fc 100644 > > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > @@ -26,7 +26,8 @@ Required properties: > > specified, the TX/RX DMA interrupts should be on that node > > instead, and only the Ethernet core interrupt is optionally > > specified here. > > -- phy-handle : Should point to the external phy device. > > +- phy-handle : Should point to the external phy device if exists. > > Pointing > > + this to the PCS/PMA PHY is deprecated and should be > > avoided. > > See ethernet.txt file in the same directory. > > - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the > > hardware > > > > @@ -68,6 +69,11 @@ Optional properties: > > required through the core's MDIO interface (i.e. always, > > unless the PHY is accessed through a different bus). > > > > + - pcs-handle: Phandle to the internal PCS/PMA PHY in SGMII or > > 1000Base-X > > + modes, where "pcs-handle" should be preferably used to > > point > > + to the PCS/PMA PHY, and "phy-handle" should point to an > > + external PHY if exists. > > I would like to have Rob feedback on this pcs-handle DT property. > > The use case is generic i.e. require separate handle to internal SGMII > and external Phy so would prefer this new DT convention is > standardized or we discuss possible approaches on how to handle > both phys and not add it as vendor specific property in the first > place. > The pcs-handle property is currently used in the Freescale dpaa2 driver, so there's some precedent for it. But I suppose if it's intended to be a standard mechanism it should be documented more globally? > > > + > > Example: > > axi_ethernet_eth: ethernet@40c00000 { > > compatible = "xlnx,axi-ethernet-1.00.a"; > > -- > > 2.34.1
On Mon, Mar 21, 2022 at 03:42:52PM +0000, Radhey Shyam Pandey wrote: > > -----Original Message----- > > From: Andy Chiu <andy.chiu@sifive.com> > > Sent: Monday, March 21, 2022 8:55 PM > > To: Radhey Shyam Pandey <radheys@xilinx.com>; robert.hancock@calian.com; > > Michal Simek <michals@xilinx.com> > > Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com; > > robh+dt@kernel.org; linux@armlinux.org.uk; andrew@lunn.ch; > > netdev@vger.kernel.org; devicetree@vger.kernel.org; Andy Chiu > > <andy.chiu@sifive.com>; Greentime Hu <greentime.hu@sifive.com> > > Subject: [PATCH v4 3/4] dt-bindings: net: xilinx_axienet: add pcs-handle > > attribute > > > > Document the new pcs-handle attribute to support connecting to an external > > PHY in SGMII or 1000Base-X modes through the internal PCS/PMA PHY. > > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > Reviewed-by: Greentime Hu <greentime.hu@sifive.com> > > --- > > Documentation/devicetree/bindings/net/xilinx_axienet.txt | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > index b8e4894bc634..ba720a2ea5fc 100644 > > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > @@ -26,7 +26,8 @@ Required properties: > > specified, the TX/RX DMA interrupts should be on that node > > instead, and only the Ethernet core interrupt is optionally > > specified here. > > -- phy-handle : Should point to the external phy device. > > +- phy-handle : Should point to the external phy device if exists. Pointing > > + this to the PCS/PMA PHY is deprecated and should be > > avoided. > > See ethernet.txt file in the same directory. > > - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the hardware > > > > @@ -68,6 +69,11 @@ Optional properties: > > required through the core's MDIO interface (i.e. always, > > unless the PHY is accessed through a different bus). > > > > + - pcs-handle: Phandle to the internal PCS/PMA PHY in SGMII or 1000Base-X > > + modes, where "pcs-handle" should be preferably used to > > point > > + to the PCS/PMA PHY, and "phy-handle" should point to an > > + external PHY if exists. > > I would like to have Rob feedback on this pcs-handle DT property. > > The use case is generic i.e. require separate handle to internal SGMII > and external Phy so would prefer this new DT convention is > standardized or we discuss possible approaches on how to handle > both phys and not add it as vendor specific property in the first > place. IMO, you should use 'phys' for the internal PCS phy. That's aligned with other uses like PCIe, SATA, etc. (there is phy h/w that will do PCS, PCIe, SATA). 'phy-handle' is for the ethernet PHY. Rob
On Mon, 2022-03-21 at 18:44 -0500, Rob Herring wrote: > On Mon, Mar 21, 2022 at 03:42:52PM +0000, Radhey Shyam Pandey wrote: > > > -----Original Message----- > > > From: Andy Chiu <andy.chiu@sifive.com> > > > Sent: Monday, March 21, 2022 8:55 PM > > > To: Radhey Shyam Pandey <radheys@xilinx.com>; robert.hancock@calian.com; > > > Michal Simek <michals@xilinx.com> > > > Cc: davem@davemloft.net; kuba@kernel.org; pabeni@redhat.com; > > > robh+dt@kernel.org; linux@armlinux.org.uk; andrew@lunn.ch; > > > netdev@vger.kernel.org; devicetree@vger.kernel.org; Andy Chiu > > > <andy.chiu@sifive.com>; Greentime Hu <greentime.hu@sifive.com> > > > Subject: [PATCH v4 3/4] dt-bindings: net: xilinx_axienet: add pcs-handle > > > attribute > > > > > > Document the new pcs-handle attribute to support connecting to an > > > external > > > PHY in SGMII or 1000Base-X modes through the internal PCS/PMA PHY. > > > > > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com> > > > Reviewed-by: Greentime Hu <greentime.hu@sifive.com> > > > --- > > > Documentation/devicetree/bindings/net/xilinx_axienet.txt | 8 +++++++- > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > > b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > > index b8e4894bc634..ba720a2ea5fc 100644 > > > --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > > +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt > > > @@ -26,7 +26,8 @@ Required properties: > > > specified, the TX/RX DMA interrupts should be on that node > > > instead, and only the Ethernet core interrupt is optionally > > > specified here. > > > -- phy-handle : Should point to the external phy device. > > > +- phy-handle : Should point to the external phy device if exists. > > > Pointing > > > + this to the PCS/PMA PHY is deprecated and should be > > > avoided. > > > See ethernet.txt file in the same directory. > > > - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the > > > hardware > > > > > > @@ -68,6 +69,11 @@ Optional properties: > > > required through the core's MDIO interface (i.e. always, > > > unless the PHY is accessed through a different bus). > > > > > > + - pcs-handle: Phandle to the internal PCS/PMA PHY in SGMII or > > > 1000Base-X > > > + modes, where "pcs-handle" should be preferably used to > > > point > > > + to the PCS/PMA PHY, and "phy-handle" should point to an > > > + external PHY if exists. > > > > I would like to have Rob feedback on this pcs-handle DT property. > > > > The use case is generic i.e. require separate handle to internal SGMII > > and external Phy so would prefer this new DT convention is > > standardized or we discuss possible approaches on how to handle > > both phys and not add it as vendor specific property in the first > > place. > > IMO, you should use 'phys' for the internal PCS phy. That's aligned with > other uses like PCIe, SATA, etc. (there is phy h/w that will do PCS, > PCIe, SATA). 'phy-handle' is for the ethernet PHY. That might be confusing in some cases - I'm thinking of the Cadence GEM controllers on the Xilinx MPSoC devices, where there is a PCS internal to the controller, as well as a generic PHY device which is used to control the actual low-level I/O transceiver on the chip (which can work in SGMII mode but also in USB, PCIe, SATA modes). Calling them both just a "PHY" makes that distinction rather unclear. In the case of that driver I believe its PCS is just "known about" by the MAC when it is present and not configured in the device tree like this driver does, but possibly it could be DT configurable someday (since I think it's potentially possible to use a different PCS implemented in the programmable logic if one wanted). The way this PCS is accessed, both it and the potential external PHY are both "Ethernet PHYs" in that they support standard Ethernet PHY registers etc., it's just that generally the external one is the one the outside Linux networking infrastructure cares about, the PCS PHY is more of an implementation detail internal to the driver. > > Rob
> > The use case is generic i.e. require separate handle to internal SGMII > > and external Phy so would prefer this new DT convention is > > standardized or we discuss possible approaches on how to handle > > both phys and not add it as vendor specific property in the first > > place. > > IMO, you should use 'phys' for the internal PCS phy. That's aligned with > other uses like PCIe, SATA, etc. (there is phy h/w that will do PCS, > PCIe, SATA). 'phy-handle' is for the ethernet PHY. We need to be careful here, because the PCS can have a well defined set of registers accessible over MDIO. Generic PHY has no infrastructure for that, it is all inside phylink which implements the pcs registers which are part of 802.3. I also wonder if a PCS might actually have a generic PHY embedded in it to provide its lower interface? Andrew
On Tue, Mar 22, 2022 at 01:21:05AM +0100, Andrew Lunn wrote: > > > The use case is generic i.e. require separate handle to internal SGMII > > > and external Phy so would prefer this new DT convention is > > > standardized or we discuss possible approaches on how to handle > > > both phys and not add it as vendor specific property in the first > > > place. > > > > IMO, you should use 'phys' for the internal PCS phy. That's aligned with > > other uses like PCIe, SATA, etc. (there is phy h/w that will do PCS, > > PCIe, SATA). 'phy-handle' is for the ethernet PHY. > > We need to be careful here, because the PCS can have a well defined > set of registers accessible over MDIO. Generic PHY has no > infrastructure for that, it is all inside phylink which implements the > pcs registers which are part of 802.3. Using the phy binding doesn't mean you have to use the kernel's 'generic PHY' subsytem. But if there's a need to do something different then propose something that handles the complex cases. > > I also wonder if a PCS might actually have a generic PHY embedded in > it to provide its lower interface? That's just looking at a single PCS/PHY block the other way around. PCS is part of the PHY or the PHY is part of PCS? I don't think that matters too much. I think the 2 cases would be it's all 1 block or 2 blocks. Rob
diff --git a/Documentation/devicetree/bindings/net/xilinx_axienet.txt b/Documentation/devicetree/bindings/net/xilinx_axienet.txt index b8e4894bc634..ba720a2ea5fc 100644 --- a/Documentation/devicetree/bindings/net/xilinx_axienet.txt +++ b/Documentation/devicetree/bindings/net/xilinx_axienet.txt @@ -26,7 +26,8 @@ Required properties: specified, the TX/RX DMA interrupts should be on that node instead, and only the Ethernet core interrupt is optionally specified here. -- phy-handle : Should point to the external phy device. +- phy-handle : Should point to the external phy device if exists. Pointing + this to the PCS/PMA PHY is deprecated and should be avoided. See ethernet.txt file in the same directory. - xlnx,rxmem : Set to allocated memory buffer for Rx/Tx in the hardware @@ -68,6 +69,11 @@ Optional properties: required through the core's MDIO interface (i.e. always, unless the PHY is accessed through a different bus). + - pcs-handle: Phandle to the internal PCS/PMA PHY in SGMII or 1000Base-X + modes, where "pcs-handle" should be preferably used to point + to the PCS/PMA PHY, and "phy-handle" should point to an + external PHY if exists. + Example: axi_ethernet_eth: ethernet@40c00000 { compatible = "xlnx,axi-ethernet-1.00.a";