Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] net: ftgmac100: Support phyless operation
To:     "William A. Kennington III" <wak@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Joel Stanley <joel@jms.id.au>
References: <20210218034040.296869-1-wak@google.com>
From:   Florian Fainelli <f.fainelli@gmail.com>
Message-ID: <bcafa57a-c849-0115-311d-e8859db93b87@gmail.com>
Date:   Wed, 17 Feb 2021 20:02:35 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210218034040.296869-1-wak@google.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/17/2021 7:40 PM, William A. Kennington III wrote:
> We have BMC to BMC connections that lack a PHY in between but don't
> want to use the NC-SI state machinery of the kernel. Instead,
> allow for an option to disable the phy detection and mdio logic.
> 
> Signed-off-by: William A. Kennington III <wak@google.com>

The way drivers deal with MAC to MAC connection is to use a fixed-link
Device Tree node that just presents hard coded link parameters. This
provides an emulation for the PHY library as well as user-space and
requires little Ethernet MAC driver changes other than doing something
along these lines:

          /* Fetch the PHY phandle */
          priv->phy_dn = of_parse_phandle(dn, "phy-handle", 0);

          /* In the case of a fixed PHY, the DT node associated
           * to the PHY is the Ethernet MAC DT node.
           */
          if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
                  ret = of_phy_register_fixed_link(dn);
                  if (ret)
                          return ret;

                  priv->phy_dn = of_node_get(dn);
          }

Can this work for you here?

> ---
>  .../devicetree/bindings/net/ftgmac100.txt     |  2 ++
>  drivers/net/ethernet/faraday/ftgmac100.c      | 30 +++++++++++--------
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/ftgmac100.txt b/Documentation/devicetree/bindings/net/ftgmac100.txt
> index 29234021f601..22c729c5fd3e 100644
> --- a/Documentation/devicetree/bindings/net/ftgmac100.txt
> +++ b/Documentation/devicetree/bindings/net/ftgmac100.txt
> @@ -19,6 +19,8 @@ Optional properties:
>  - phy-mode: See ethernet.txt file in the same directory. If the property is
>    absent, "rgmii" is assumed. Supported values are "rgmii*" and "rmii" for
>    aspeed parts. Other (unknown) parts will accept any value.
> +- no-phy: Disable any MDIO or PHY connection logic and assume the interface
> +  is always up.
>  - use-ncsi: Use the NC-SI stack instead of an MDIO PHY. Currently assumes
>    rmii (100bT) but kept as a separate property in case NC-SI grows support
>    for a gigabit link.
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 88bfe2107938..f2cf190654c8 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1467,18 +1467,18 @@ static int ftgmac100_open(struct net_device *netdev)
>  		return err;
>  	}
>  
> -	/* When using NC-SI we force the speed to 100Mbit/s full duplex,
> +	/* When PHYless we force the speed to 100Mbit/s full duplex,
>  	 *
>  	 * Otherwise we leave it set to 0 (no link), the link
>  	 * message from the PHY layer will handle setting it up to
>  	 * something else if needed.
>  	 */
> -	if (priv->use_ncsi) {
> -		priv->cur_duplex = DUPLEX_FULL;
> -		priv->cur_speed = SPEED_100;
> -	} else {
> +	if (netdev->phydev) {
>  		priv->cur_duplex = 0;
>  		priv->cur_speed = 0;
> +	} else {
> +		priv->cur_duplex = DUPLEX_FULL;
> +		priv->cur_speed = SPEED_100;
>  	}
>  
>  	/* Reset the hardware */
> @@ -1506,14 +1506,16 @@ static int ftgmac100_open(struct net_device *netdev)
>  	if (netdev->phydev) {
>  		/* If we have a PHY, start polling */
>  		phy_start(netdev->phydev);
> -	} else if (priv->use_ncsi) {
> -		/* If using NC-SI, set our carrier on and start the stack */
> +	} else {
> +		/* If PHYless, set our carrier on and start the stack */
>  		netif_carrier_on(netdev);
>  
> -		/* Start the NCSI device */
> -		err = ncsi_start_dev(priv->ndev);
> -		if (err)
> -			goto err_ncsi;
> +		if (priv->use_ncsi) {
> +			/* Start the NCSI device */
> +			err = ncsi_start_dev(priv->ndev);
> +			if (err)
> +				goto err_ncsi;
> +		}
>  	}
>  
>  	return 0;
> @@ -1725,8 +1727,8 @@ static int ftgmac100_setup_clk(struct ftgmac100 *priv)
>  	 * 1000Mbit link speeds. As NCSI is limited to 100Mbit, 25MHz
>  	 * is sufficient
>  	 */
> -	rc = clk_set_rate(priv->clk, priv->use_ncsi ? FTGMAC_25MHZ :
> -			  FTGMAC_100MHZ);
> +	rc = clk_set_rate(priv->clk, priv->netdev->phydev ? FTGMAC_100MHZ :
> +			  FTGMAC_25MHZ);
>  	if (rc)
>  		goto cleanup_clk;
>  
> @@ -1837,6 +1839,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  		priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler);
>  		if (!priv->ndev)
>  			goto err_phy_connect;
> +	} else if (np && of_get_property(np, "no-phy", NULL)) {
> +		dev_info(&pdev->dev, "Using PHYless interface\n");
>  	} else if (np && of_get_property(np, "phy-handle", NULL)) {
>  		struct phy_device *phy;
>  
> 

-- 
Florian