[U-Boot] [PATCH 16/26] net: mt7628-eth: remove phy link up detection

[Weijie Gao]

On Wed, 2019-08-28 at 15:37 +0200, Stefan Roese wrote:
> On 28.08.19 08:38, Weijie Gao wrote:
> > The mt7628 has an embedded ethernet switch (5 phy ports + 1 cpu port).
> > Although in IOT mode only port0 is usable, the phy0 is still connected
> > to the switch, not the ethernet gmac directly.
> > 
> > This patch removes these codes as we should not check only the status of
> > phy0 because phy0 may not be linked up in router mode which has all 5 phy
> > ports working.
> 
> I understand. But for those embedded targets that use the mainline code
> right now, this setup is exactly the one that's used. With this patch applied
> this "feature" of link detection of the only ethernet port is removed.
> 
> Can't we somehow change this code, that it supports both operations modes?
> So targets without switch functionality and targets with multiple ports
> (with switch functionality) can be supported? Perhaps via some DT property
> or at least a Kconfig option?
> 
> BTW: Do you plan to add support for a target with switch functionality? That
> would be just great.
> 
> Thanks,
> Stefan
> 
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > ---
> >   drivers/net/mt7628-eth.c | 32 --------------------------------
> >   1 file changed, 32 deletions(-)
> > 
> > diff --git a/drivers/net/mt7628-eth.c b/drivers/net/mt7628-eth.c
> > index 4675b0f003..eef25da8a8 100644
> > --- a/drivers/net/mt7628-eth.c
> > +++ b/drivers/net/mt7628-eth.c
> > @@ -120,9 +120,6 @@ struct fe_tx_dma {
> >   #define CONFIG_DMA_STOP_TIMEOUT	100
> >   #define CONFIG_TX_DMA_TIMEOUT	100
> >   
> > -#define LINK_DELAY_TIME		500		/* 500 ms */
> > -#define LINK_TIMEOUT		10000		/* 10 seconds */
> > -
> >   struct mt7628_eth_dev {
> >   	void __iomem *base;		/* frame engine base address */
> >   	void __iomem *eth_sw_base;	/* switch base address */
> > @@ -437,14 +434,6 @@ static int mt7628_eth_free_pkt(struct udevice *dev, uchar *packet, int length)
> >   	return 0;
> >   }
> >   
> > -static int phy_link_up(struct mt7628_eth_dev *priv)
> > -{
> > -	u32 val;
> > -
> > -	mii_mgr_read(priv, 0x00, MII_BMSR, &val);
> > -	return !!(val & BMSR_LSTATUS);
> > -}
> > -
> >   static int mt7628_eth_start(struct udevice *dev)
> >   {
> >   	struct mt7628_eth_dev *priv = dev_get_priv(dev);
> > @@ -493,27 +482,6 @@ static int mt7628_eth_start(struct udevice *dev)
> >   	wmb();
> >   	eth_dma_start(priv);
> >   
> > -	/* Check if link is not up yet */
> > -	if (!phy_link_up(priv)) {
> > -		/* Wait for link to come up */
> > -
> > -		printf("Waiting for link to come up .");
> > -		for (i = 0; i < (LINK_TIMEOUT / LINK_DELAY_TIME); i++) {
> > -			mdelay(LINK_DELAY_TIME);
> > -			if (phy_link_up(priv)) {
> > -				mdelay(100);	/* Ensure all is ready */
> > -				break;
> > -			}
> > -
> > -			printf(".");
> > -		}
> > -
> > -		if (phy_link_up(priv))
> > -			printf(" done\n");
> > -		else
> > -			printf(" timeout! Trying anyways\n");
> > -	}
> > -
> >   	/*
> >   	 * The integrated switch seems to queue some received ethernet
> >   	 * packets in some FIFO. Lets read the already queued packets
> > 
> 
> Viele Grüße,
> Stefan
> 

I think adding a phy interface should do. I'll try to implement this.