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@xxxxxxx>
> > ---
> > 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
