Re: [RFC PATCH v2 net-next 04/15] phy: allow querying the address of protocol converters through phy_get_status()

[Vinod Koul]

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!

> + *
> + * @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