netdev - Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 22 Mar 2011 15:28:33 +0800
From:	Shawn Guo <shawn.guo@...escale.com>
To:	Lothar Waßmann <LW@...O-electronics.de>
CC:	<netdev@...r.kernel.org>, <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH 3/4] drivers/net/fec: Fix dual MAC support on i.MX28

Hi Lothar,

On Mon, Mar 21, 2011 at 05:37:35PM +0100, Lothar Waßmann wrote:
> The variant of the FEC chip found on i.MX28 features two distinct
> ethernet MACs that both share a single MII interface. When
> deconfiguring the eth0 interface, the MII interface is currently being
> switched off, so that the second MAC won't work any more.
> 
I'm wondering what the use case is you intend to support with this
patch.

> Make sure, the first MAC is never disabled when the second FEC device
> is registered.
> 
> Signed-off-by: Lothar Waßmann <LW@...O-electronics.de>
> ---
>  drivers/net/fec.c |   38 +++++++++++++++++++++++++++++++++-----
>  1 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index 2436db8..3666524 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -82,6 +82,9 @@ static unsigned char macaddr[ETH_ALEN];
>  module_param_array(macaddr, byte, NULL, 0);
>  MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>  
> +static int dual_mac;
> +module_param(dual_mac, int, S_IRUGO);
> +

As you are detecting dual_mac in probe function, is this necessary?

>  #if defined(CONFIG_M5272)
>  /*
>   * Some hardware gets it MAC address out of local flash memory.
> @@ -456,6 +459,8 @@ static void
>  fec_stop(struct net_device *ndev)
>  {
>  	struct fec_enet_private *fep = netdev_priv(ndev);
> +	const struct platform_device_id *id_entry =
> +				platform_get_device_id(fep->pdev);
>  
>  	/* We cannot expect a graceful transmit stop without link !!! */
>  	if (fep->link) {
> @@ -466,11 +471,22 @@ fec_stop(struct net_device *ndev)
>  			printk("fec_stop : Graceful transmit stop did not complete !\n");
>  	}
>  
> -	/* Whack a reset.  We should wait for this. */
> -	writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> -	udelay(10);
> -	writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> -	writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> +	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && dual_mac &&
> +		fep->pdev->id == 0) {
> +		/* clear the RDAR/TDAR bits to stop recv/xmit,
> +		 * but do not reset the controller since the second MAC
> +		 * may still be using the MII interface and requires ETHER_EN
> +		 * on the first MAC to be asserted for MII interrupts!
> +		 */
> +		writel(0, fep->hwp + FEC_ECNTRL);
> +		writel(FEC_ECNTRL_ETHER_EN, fep->hwp + FEC_ECNTRL);
> +	} else {
> +		/* Whack a reset.  We should wait for this. */
> +		writel(FEC_ECNTRL_RESET, fep->hwp + FEC_ECNTRL);
> +		udelay(10);
> +		writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
> +		writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> +	}
>  }
>  
>  static void
> @@ -1135,6 +1151,8 @@ static int
>  fec_enet_open(struct net_device *ndev)
>  {
>  	struct fec_enet_private *fep = netdev_priv(ndev);
> +	const struct platform_device_id *id_entry =
> +				platform_get_device_id(fep->pdev);
>  	int ret;
>  
>  	/* I should reset the ring buffers here, but I don't yet know
> @@ -1145,6 +1163,14 @@ fec_enet_open(struct net_device *ndev)
>  	if (ret)
>  		return ret;
>  
> +	if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) &&
> +		fep->pdev->id == 0) {
> +		/* ETHER_EN on first MAC has to be asserted
> +		 * in order to generate MII interrupts
> +		 */
> +		fec_restart(ndev, 0);
> +	}
> +

Is this relevant to the use case you intend to support?  Or something
is broken without these codes?

>  	/* Probe and connect to PHY when open the interface */
>  	ret = fec_enet_mii_probe(ndev);
>  	if (ret) {
> @@ -1435,6 +1461,8 @@ fec_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto failed_register;
>  
> +	if (pdev->id > 0)
> +		dual_mac = 1;
>  	return 0;
>  
Blank line above 'return 0;'?

-- 
Regards,
Shawn

>  failed_register:
> -- 
> 1.5.6.5
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html