[PATCH v2] net: zynq_gem: Wait for SGMII PCS link in zynq_gem_init()

[Michal Simek]

On 1/25/23 08:09, Stefan Roese wrote:
> In our system using ZynqMP with an external SGMII PHY it's necessary
> to wait for the PCS link and auto negotiation to finish before the xfer
> starts. Otherwise the first packet(s) might get dropped, resulting in a
> delay at the start of the ethernet transfers.
> 
> This is only done when the PHY link is already up, which is done in
> phy_startup(). As waiting for the PHY link bits via pcsstatus does not
> make much sense, when the link is not available in general (e.g. no
> cable connected).
> 
> This patch adds the necessary code including a minimal delay of 1 ms
> which fixes problems of dropped first packages.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Michal Simek <michal.simek at amd.com>
> Cc: Katakam Harini <harini.katakam at amd.com>
> Cc: Ramon Fried <rfried.dev at gmail.com>
> Cc: Sean Anderson <sean.anderson at seco.com>
> ---
> v2:
> - Move wait for PCS link into the code path only executed for SGMII w/o
>    fixed-link
> - Reduce additional delay from 10 to 1ms
> - Change dev_err to dev_warn
> 
>   drivers/net/zynq_gem.c | 36 +++++++++++++++++++++++++++++++++---
>   1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index 507b19b75975..26565d78dafd 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -125,6 +125,10 @@
>    */
>   #define PHY_DETECT_MASK 0x1808
>   
> +/* PCS (SGMII) Link Status */
> +#define ZYNQ_GEM_PCSSTATUS_LINK		BIT(2)
> +#define ZYNQ_GEM_PCSSTATUS_ANEG_COMPL	BIT(5)
> +
>   /* TX BD status masks */
>   #define ZYNQ_GEM_TXBUF_FRMLEN_MASK	0x000007ff
>   #define ZYNQ_GEM_TXBUF_EXHAUSTED	0x08000000
> @@ -164,7 +168,8 @@ struct zynq_gem_regs {
>   	u32 stat[STAT_SIZE]; /* 0x100 - Octects transmitted Low reg */
>   	u32 reserved9[20];
>   	u32 pcscntrl;
> -	u32 rserved12[36];
> +	u32 pcsstatus;
> +	u32 rserved12[35];
>   	u32 dcfg6; /* 0x294 Design config reg6 */
>   	u32 reserved7[106];
>   	u32 transmit_q1_ptr; /* 0x440 - Transmit priority queue 1 */
> @@ -491,12 +496,37 @@ static int zynq_gem_init(struct udevice *dev)
>   		 * Must be written after PCS_SEL is set in nwconfig,
>   		 * otherwise writes will not take effect.
>   		 */
> -		if (priv->phydev->phy_id != PHY_FIXED_ID)
> +		if (priv->phydev->phy_id != PHY_FIXED_ID) {
>   			writel(readl(&regs->pcscntrl) | ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
>   			       &regs->pcscntrl);
> -		else
> +			/*
> +			 * When the PHY link is already up, the PCS link needs
> +			 * to get re-checked
> +			 */
> +			if (priv->phydev->link) {
> +				u32 pcsstatus;
> +
> +				pcsstatus = ZYNQ_GEM_PCSSTATUS_LINK |
> +					ZYNQ_GEM_PCSSTATUS_ANEG_COMPL;
> +				ret = wait_for_bit_le32(&regs->pcsstatus,
> +							pcsstatus,
> +							true, 5000, true);
> +				if (ret) {
> +					dev_warn(dev,
> +						 "no PCS (SGMII) link\n");
> +				} else {
> +					/*
> +					 * Some additional minimal delay seems
> +					 * to be needed so that the first
> +					 * packet will be sent correctly
> +					 */
> +					mdelay(1);
> +				}
> +			}
> +		} else {
>   			writel(readl(&regs->pcscntrl) & ~ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
>   			       &regs->pcscntrl);
> +		}
>   	}
>   #endif
>   

Applied.
M