| Message ID | 20230923134904.3627402-5-vladimir.oltean@nxp.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | Add C72/C73 copper backplane support for LX2160 | expand |
On 23-09-23, 16:48, Vladimir Oltean wrote: > The bit stream handled by a SerDes lane needs protocol converters to be > usable for Ethernet. On Freescale/NXP SoCs, those protocol converters > are located on the internal MDIO buses of the Ethernet MACs that need > them. > > The location on that MDIO bus, on these SoCs, is not fixed, but given by > some control registers of the SerDes block itself. > > Because no one modifies those addresses from the power-on default, so > far we've relied on hardcoding the default values in the device trees, > resulting in something like this: > > pcs_mdio1: mdio@8c07000 { > compatible = "fsl,fman-memac-mdio"; > > pcs1: ethernet-phy@0 { > reg = <0>; > }; > }; > > where the "reg" of "pcs1" can actually be retrieved from "serdes_1". > > That was for the PCS. For AN/LT blocks, that can also be done, but the > MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to > get wrong, which will confuse and frustrate any device tree writers. > > The proposal is to take advantage of the fact that these protocol > converters *are* discoverable, and to side-step that entire device tree > mapping issue by not putting them in the device tree at all. So, one of > the consumers of the SerDes PHY uses the phy_get_status() API to figure > out the address on the MDIO bus, it also has a reference to the MDIO bus > => it can create the mdio_device in a non OF-based manner. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > v1->v2: patch is new > > include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index f1f03fa66943..ee721067517b 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -56,6 +56,33 @@ enum phy_media { > enum phy_status_type { > /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */ > PHY_STATUS_CDR_LOCK, > + PHY_STATUS_PCVT_ADDR, > +}; > + > +/* enum phy_pcvt_type - PHY protocol converter type It is not a generic protocol converter but an ethernet phy protocol converter, so i guess we should add that here (we are generic phy and not ethernet phy here! > + * > + * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of > + * an Ethernet PHY. Connects through MII to the MAC, > + * and handles link status detection and the conversion > + * of MII signals to link-specific code words (8b/10b, > + * 64b/66b etc). > + * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training, > + * bottom-most layer of an Ethernet PHY, beneath the > + * PMA and PMD. Its activity is only visible on the > + * physical medium, and it is responsible for > + * selecting the most adequate PCS/PMA/PMD set that > + * can operate on that medium. > + */ > +enum phy_pcvt_type { > + PHY_PCVT_ETHERNET_PCS, > + PHY_PCVT_ETHERNET_ANLT, > +}; > + > +struct phy_status_opts_pcvt { > + enum phy_pcvt_type type; > + union { > + unsigned int mdio; > + } addr; > }; > > /* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit > @@ -71,9 +98,11 @@ struct phy_status_opts_cdr { > * union phy_status_opts - Opaque generic phy status > * > * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK. > + * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR. > */ > union phy_status_opts { > struct phy_status_opts_cdr cdr; > + struct phy_status_opts_pcvt pcvt; > }; > > /** > -- > 2.34.1
Hi Vinod, On Fri, Sep 29, 2023 at 09:53:22PM +0530, Vinod Koul wrote: > On 23-09-23, 16:48, Vladimir Oltean wrote: > > The bit stream handled by a SerDes lane needs protocol converters to be > > usable for Ethernet. On Freescale/NXP SoCs, those protocol converters > > are located on the internal MDIO buses of the Ethernet MACs that need > > them. > > > > The location on that MDIO bus, on these SoCs, is not fixed, but given by > > some control registers of the SerDes block itself. > > > > Because no one modifies those addresses from the power-on default, so > > far we've relied on hardcoding the default values in the device trees, > > resulting in something like this: > > > > pcs_mdio1: mdio@8c07000 { > > compatible = "fsl,fman-memac-mdio"; > > > > pcs1: ethernet-phy@0 { > > reg = <0>; > > }; > > }; > > > > where the "reg" of "pcs1" can actually be retrieved from "serdes_1". > > > > That was for the PCS. For AN/LT blocks, that can also be done, but the > > MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to > > get wrong, which will confuse and frustrate any device tree writers. > > > > The proposal is to take advantage of the fact that these protocol > > converters *are* discoverable, and to side-step that entire device tree > > mapping issue by not putting them in the device tree at all. So, one of > > the consumers of the SerDes PHY uses the phy_get_status() API to figure > > out the address on the MDIO bus, it also has a reference to the MDIO bus > > => it can create the mdio_device in a non OF-based manner. > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > --- > > v1->v2: patch is new > > > > include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > > index f1f03fa66943..ee721067517b 100644 > > --- a/include/linux/phy/phy.h > > +++ b/include/linux/phy/phy.h > > @@ -56,6 +56,33 @@ enum phy_media { > > enum phy_status_type { > > /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */ > > PHY_STATUS_CDR_LOCK, > > + PHY_STATUS_PCVT_ADDR, > > +}; > > + > > +/* enum phy_pcvt_type - PHY protocol converter type > > It is not a generic protocol converter but an ethernet phy protocol > converter, so i guess we should add that here (we are generic phy and > not ethernet phy here! Can you please show, using a diff, what modification you would like to see? In my mind, enum phy_pcvt_type could also contain non-Ethernet protocol converter types, and so, I didn't want to add "ethernet_" in it. The "ETHERNET_" prefix will be part of the individual enum values that are applicable to Ethernet. > > + * > > + * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of > > + * an Ethernet PHY. Connects through MII to the MAC, > > + * and handles link status detection and the conversion > > + * of MII signals to link-specific code words (8b/10b, > > + * 64b/66b etc). > > + * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training, > > + * bottom-most layer of an Ethernet PHY, beneath the > > + * PMA and PMD. Its activity is only visible on the > > + * physical medium, and it is responsible for > > + * selecting the most adequate PCS/PMA/PMD set that > > + * can operate on that medium. > > + */ > > +enum phy_pcvt_type { > > + PHY_PCVT_ETHERNET_PCS, > > + PHY_PCVT_ETHERNET_ANLT, > > +}; > > + > > +struct phy_status_opts_pcvt { > > + enum phy_pcvt_type type; > > + union { > > + unsigned int mdio; > > + } addr; > > }; > > > > /* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit > > @@ -71,9 +98,11 @@ struct phy_status_opts_cdr { > > * union phy_status_opts - Opaque generic phy status > > * > > * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK. > > + * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR. > > */ > > union phy_status_opts { > > struct phy_status_opts_cdr cdr; > > + struct phy_status_opts_pcvt pcvt; > > }; > > > > /** > > -- > > 2.34.1 > > -- > ~Vinod
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index f1f03fa66943..ee721067517b 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -56,6 +56,33 @@ enum phy_media { enum phy_status_type { /* Valid for PHY_MODE_ETHERNET and PHY_MODE_ETHTOOL */ PHY_STATUS_CDR_LOCK, + PHY_STATUS_PCVT_ADDR, +}; + +/* enum phy_pcvt_type - PHY protocol converter type + * + * @PHY_PCVT_ETHERNET_PCS: Ethernet Physical Coding Sublayer, top-most layer of + * an Ethernet PHY. Connects through MII to the MAC, + * and handles link status detection and the conversion + * of MII signals to link-specific code words (8b/10b, + * 64b/66b etc). + * @PHY_PCVT_ETHERNET_ANLT: Ethernet Auto-Negotiation and Link Training, + * bottom-most layer of an Ethernet PHY, beneath the + * PMA and PMD. Its activity is only visible on the + * physical medium, and it is responsible for + * selecting the most adequate PCS/PMA/PMD set that + * can operate on that medium. + */ +enum phy_pcvt_type { + PHY_PCVT_ETHERNET_PCS, + PHY_PCVT_ETHERNET_ANLT, +}; + +struct phy_status_opts_pcvt { + enum phy_pcvt_type type; + union { + unsigned int mdio; + } addr; }; /* If the CDR (Clock and Data Recovery) block is able to lock onto the RX bit @@ -71,9 +98,11 @@ struct phy_status_opts_cdr { * union phy_status_opts - Opaque generic phy status * * @cdr: Configuration set applicable for PHY_STATUS_CDR_LOCK. + * @pcvt: Configuration set applicable for PHY_STATUS_PCVT_ADDR. */ union phy_status_opts { struct phy_status_opts_cdr cdr; + struct phy_status_opts_pcvt pcvt; }; /**
The bit stream handled by a SerDes lane needs protocol converters to be usable for Ethernet. On Freescale/NXP SoCs, those protocol converters are located on the internal MDIO buses of the Ethernet MACs that need them. The location on that MDIO bus, on these SoCs, is not fixed, but given by some control registers of the SerDes block itself. Because no one modifies those addresses from the power-on default, so far we've relied on hardcoding the default values in the device trees, resulting in something like this: pcs_mdio1: mdio@8c07000 { compatible = "fsl,fman-memac-mdio"; pcs1: ethernet-phy@0 { reg = <0>; }; }; where the "reg" of "pcs1" can actually be retrieved from "serdes_1". That was for the PCS. For AN/LT blocks, that can also be done, but the MAC to PCS to AN/LT block mapping is non-trivial and extremely easy to get wrong, which will confuse and frustrate any device tree writers. The proposal is to take advantage of the fact that these protocol converters *are* discoverable, and to side-step that entire device tree mapping issue by not putting them in the device tree at all. So, one of the consumers of the SerDes PHY uses the phy_get_status() API to figure out the address on the MDIO bus, it also has a reference to the MDIO bus => it can create the mdio_device in a non OF-based manner. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- v1->v2: patch is new include/linux/phy/phy.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)