[uClinux-dev] PATCH : fec bugfixes and cleanup

Philippe De Muyter phdm at macqel.be
Thu Aug 11 11:05:20 EDT 2005


Greg Ungerer wrote :
> Hi Philippe,
> 
> Philippe De Muyter wrote:
> > Here is a long (but simple) patch for fec.c, for uClinux-dist-test-20050808
> 
> Is this against the 2.4.x kernel or for 2.6.x?

Sorry.  This is for 2.6.x.  (but I surmise that the bugs are also in 2.4.x)

> 
> Regards
> Greg
> 
> 
> 
> > It provides :
> > 	- typo fixes
> > 	- a compiler warning fix
> > 	- c99 fields initialisation
> > 	- do not reread phy_status multiple times in mii_parse_cr,
> > 		mii_parse_anar and mii_display_config.
> > 	- a comment fix
> > 	- replace a numeric constant with its symbolic equivalent FEC_ENET_GRA
> > 	- three bug fixes :
> > 		fec_timeout : restart the fec with the current duplex mode
> > 			given by the PHY, not always with half duplex.
> > 		phy_cmd_lxt971_ack_int : acknowledge the interrupt before
> > 			reading the status, not after (when status could again
> > 			have changed)
> > 		fec_enet_init, fec_restart : ievent must be cleared with
> > 			0xffc00000, not 0xffc0.
> > 
> > Feel free to apply
> > 
> > Philippe
> > 		
> > --- drivers/net/fec.c.orig	2005-07-29 02:47:48.000000000 +0200
> > +++ drivers/net/fec.c	2005-08-10 14:48:51.000000000 +0200
> > @@ -19,7 +19,10 @@
> >   * Copyright (c) 2000 Ericsson Radio Systems AB.
> >   *
> >   * Support for FEC controller of ColdFire/5270/5271/5272/5274/5275/5280/5282.
> > - * Copyrught (c) 2001-2004 Greg Ungerer (gerg at snapgear.com)
> > + * Copyright (c) 2001-2004 Greg Ungerer (gerg at snapgear.com)
> > + *
> > + * Bug fixes and cleanup by Philippe De Muyter (phdm at macqel.be)
> > + * Copyright (c) 2004-2005 Macq Electronique SA.
> >   */
> >  
> >  #include <linux/config.h>
> > @@ -427,7 +430,7 @@ fec_timeout(struct net_device *dev)
> >  	}
> >  	}
> >  #endif
> > -	fec_restart(dev, 0);
> > +	fec_restart(dev, fep->full_duplex);
> >  	netif_wake_queue(dev);
> >  }
> >  
> > @@ -777,30 +780,34 @@ static void mii_parse_cr(uint mii_reg, s
> >  {
> >  	struct fec_enet_private *fep = netdev_priv(dev);
> >  	volatile uint *s = &(fep->phy_status);
> > +	uint status;
> >  
> > -	*s &= ~(PHY_CONF_ANE | PHY_CONF_LOOP);
> > +	status = *s & ~(PHY_CONF_ANE | PHY_CONF_LOOP);
> >  
> >  	if (mii_reg & 0x1000)
> > -		*s |= PHY_CONF_ANE;
> > +		status |= PHY_CONF_ANE;
> >  	if (mii_reg & 0x4000)
> > -		*s |= PHY_CONF_LOOP;
> > +		status |= PHY_CONF_LOOP;
> > +	*s = status;
> >  }
> >  
> >  static void mii_parse_anar(uint mii_reg, struct net_device *dev)
> >  {
> >  	struct fec_enet_private *fep = netdev_priv(dev);
> >  	volatile uint *s = &(fep->phy_status);
> > +	uint status;
> >  
> > -	*s &= ~(PHY_CONF_SPMASK);
> > +	status = *s & ~(PHY_CONF_SPMASK);
> >  
> >  	if (mii_reg & 0x0020)
> > -		*s |= PHY_CONF_10HDX;
> > +		status |= PHY_CONF_10HDX;
> >  	if (mii_reg & 0x0040)
> > -		*s |= PHY_CONF_10FDX;
> > +		status |= PHY_CONF_10FDX;
> >  	if (mii_reg & 0x0080)
> > -		*s |= PHY_CONF_100HDX;
> > +		status |= PHY_CONF_100HDX;
> >  	if (mii_reg & 0x00100)
> > -		*s |= PHY_CONF_100FDX;
> > +		status |= PHY_CONF_100FDX;
> > +	*s = status;
> >  }
> >  
> >  /* ------------------------------------------------------------------------- */
> > @@ -857,12 +864,12 @@ static phy_cmd_t const phy_cmd_lxt970_sh
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_info_t const phy_info_lxt970 = {
> > -	0x07810000, 
> > -	"LXT970",
> > -	phy_cmd_lxt970_config,
> > -	phy_cmd_lxt970_startup,
> > -	phy_cmd_lxt970_ack_int,
> > -	phy_cmd_lxt970_shutdown
> > +	.id = 0x07810000, 
> > +	.name = "LXT970",
> > +	.config = phy_cmd_lxt970_config,
> > +	.startup = phy_cmd_lxt970_startup,
> > +	.ack_int = phy_cmd_lxt970_ack_int,
> > +	.shutdown = phy_cmd_lxt970_shutdown
> >  };
> >  	
> >  /* ------------------------------------------------------------------------- */
> > @@ -935,11 +942,11 @@ static phy_cmd_t const phy_cmd_lxt971_st
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_cmd_t const phy_cmd_lxt971_ack_int[] = {
> > +		/* acknowledge the int before reading status ! */
> > +		{ mk_mii_read(MII_LXT971_ISR), NULL },
> >  		/* find out the current status */
> >  		{ mk_mii_read(MII_REG_SR), mii_parse_sr },
> >  		{ mk_mii_read(MII_LXT971_SR2), mii_parse_lxt971_sr2 },
> > -		/* we only need to read ISR to acknowledge */
> > -		{ mk_mii_read(MII_LXT971_ISR), NULL },
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_cmd_t const phy_cmd_lxt971_shutdown[] = { /* disable interrupts */
> > @@ -947,12 +954,12 @@ static phy_cmd_t const phy_cmd_lxt971_sh
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_info_t const phy_info_lxt971 = {
> > -	0x0001378e, 
> > -	"LXT971",
> > -	phy_cmd_lxt971_config,
> > -	phy_cmd_lxt971_startup,
> > -	phy_cmd_lxt971_ack_int,
> > -	phy_cmd_lxt971_shutdown
> > +	.id = 0x0001378e, 
> > +	.name = "LXT971",
> > +	.config = phy_cmd_lxt971_config,
> > +	.startup = phy_cmd_lxt971_startup,
> > +	.ack_int = phy_cmd_lxt971_ack_int,
> > +	.shutdown = phy_cmd_lxt971_shutdown
> >  };
> >  
> >  /* ------------------------------------------------------------------------- */
> > @@ -1016,12 +1023,12 @@ static phy_cmd_t const phy_cmd_qs6612_sh
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_info_t const phy_info_qs6612 = {
> > -	0x00181440, 
> > -	"QS6612",
> > -	phy_cmd_qs6612_config,
> > -	phy_cmd_qs6612_startup,
> > -	phy_cmd_qs6612_ack_int,
> > -	phy_cmd_qs6612_shutdown
> > +	.id = 0x00181440, 
> > +	.name = "QS6612",
> > +	.config = phy_cmd_qs6612_config,
> > +	.startup = phy_cmd_qs6612_startup,
> > +	.ack_int = phy_cmd_qs6612_ack_int,
> > +	.shutdown = phy_cmd_qs6612_shutdown
> >  };
> >  
> >  /* ------------------------------------------------------------------------- */
> > @@ -1080,12 +1087,12 @@ static phy_cmd_t const phy_cmd_am79c874_
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_info_t const phy_info_am79c874 = {
> > -	0x00022561,
> > -	"AM79C874",
> > -	phy_cmd_am79c874_config,
> > -	phy_cmd_am79c874_startup,
> > -	phy_cmd_am79c874_ack_int,
> > -	phy_cmd_am79c874_shutdown
> > +	.id = 0x00022561,
> > +	.name = "AM79C874",
> > +	.config = phy_cmd_am79c874_config,
> > +	.startup = phy_cmd_am79c874_startup,
> > +	.ack_int = phy_cmd_am79c874_ack_int,
> > +	.shutdown = phy_cmd_am79c874_shutdown
> >  };
> >  
> >  
> > @@ -1121,12 +1128,12 @@ static phy_cmd_t const phy_cmd_ks8721bl_
> >  		{ mk_mii_end, }
> >  	};
> >  static phy_info_t const phy_info_ks8721bl = {
> > -	0x00022161, 
> > -	"KS8721BL",
> > -	phy_cmd_ks8721bl_config,
> > -	phy_cmd_ks8721bl_startup,
> > -	phy_cmd_ks8721bl_ack_int,
> > -	phy_cmd_ks8721bl_shutdown
> > +	.id = 0x00022161, 
> > +	.name = "KS8721BL",
> > +	.config = phy_cmd_ks8721bl_config,
> > +	.startup = phy_cmd_ks8721bl_startup,
> > +	.ack_int = phy_cmd_ks8721bl_ack_int,
> > +	.shutdown = phy_cmd_ks8721bl_shutdown
> >  };
> >  
> >  /* ------------------------------------------------------------------------- */
> > @@ -1214,7 +1221,7 @@ static void __inline__ fec_get_mac(struc
> >  		 * Get MAC address from FLASH.
> >  		 * If it is all 1's or 0's, use the default.
> >  		 */
> > -		iap = FEC_FLASHMAC;
> > +		iap = (unsigned char *)FEC_FLASHMAC;
> >  		if ((iap[0] == 0) && (iap[1] == 0) && (iap[2] == 0) &&
> >  		    (iap[3] == 0) && (iap[4] == 0) && (iap[5] == 0))
> >  			iap = fec_mac_default;
> > @@ -1418,7 +1425,7 @@ static void __inline__ fec_uncache(unsig
> >  #else
> >  
> >  /*
> > - *	Code sepcific to the MPC860T setup.
> > + *	Code specific to the MPC860T setup.
> >   */
> >  static void __inline__ fec_request_intrs(struct net_device *dev)
> >  {
> > @@ -1573,7 +1580,7 @@ static void mii_display_status(struct ne
> >  static void mii_display_config(struct net_device *dev)
> >  {
> >  	struct fec_enet_private *fep = netdev_priv(dev);
> > -	volatile uint *s = &(fep->phy_status);
> > +	uint status = fep->phy_status;
> >  
> >  	/*
> >  	** When we get here, phy_task is already removed from
> > @@ -1582,23 +1589,23 @@ static void mii_display_config(struct ne
> >  	fep->mii_phy_task_queued = 0;
> >  	printk("%s: config: auto-negotiation ", dev->name);
> >  
> > -	if (*s & PHY_CONF_ANE)
> > +	if (status & PHY_CONF_ANE)
> >  		printk("on");
> >  	else
> >  		printk("off");
> >  
> > -	if (*s & PHY_CONF_100FDX)
> > +	if (status & PHY_CONF_100FDX)
> >  		printk(", 100FDX");
> > -	if (*s & PHY_CONF_100HDX)
> > +	if (status & PHY_CONF_100HDX)
> >  		printk(", 100HDX");
> > -	if (*s & PHY_CONF_10FDX)
> > +	if (status & PHY_CONF_10FDX)
> >  		printk(", 10FDX");
> > -	if (*s & PHY_CONF_10HDX)
> > +	if (status & PHY_CONF_10HDX)
> >  		printk(", 10HDX");
> > -	if (!(*s & PHY_CONF_SPMASK))
> > +	if (!(status & PHY_CONF_SPMASK))
> >  		printk(", No speed/duplex selected?");
> >  
> > -	if (*s & PHY_CONF_LOOP)
> > +	if (status & PHY_CONF_LOOP)
> >  		printk(", loopback enabled");
> >  	
> >  	printk(".\n");
> > @@ -1656,7 +1663,7 @@ static void mii_queue_relink(uint mii_re
> >  	schedule_work(&fep->phy_task);
> >  }
> >  
> > -/* mii_queue_config is called in user context from fec_enet_open */
> > +/* mii_queue_config is called in interrupt context from fec_enet_mii */
> >  static void mii_queue_config(uint mii_reg, struct net_device *dev)
> >  {
> >  	struct fec_enet_private *fep = netdev_priv(dev);
> > @@ -1958,7 +1965,7 @@ int __init fec_enet_init(struct net_devi
> >  	udelay(10);
> >  
> >  	/* Clear and enable interrupts */
> > -	fecp->fec_ievent = 0xffc0;
> > +	fecp->fec_ievent = 0xffc00000;
> >  	fecp->fec_imask = (FEC_ENET_TXF | FEC_ENET_TXB |
> >  		FEC_ENET_RXF | FEC_ENET_RXB | FEC_ENET_MII);
> >  	fecp->fec_hash_table_high = 0;
> > @@ -2116,7 +2123,7 @@ fec_restart(struct net_device *dev, int 
> >  
> >  	/* Clear any outstanding interrupt.
> >  	*/
> > -	fecp->fec_ievent = 0xffc0;
> > +	fecp->fec_ievent = 0xffc00000;
> >  	fec_enable_phy_intr();
> >  
> >  	/* Set station address.
> > @@ -2219,7 +2226,7 @@ fec_stop(struct net_device *dev)
> >  
> >  	fecp->fec_x_cntrl = 0x01;	/* Graceful transmit stop */
> >  
> > -	while(!(fecp->fec_ievent & 0x10000000));
> > +	while(!(fecp->fec_ievent & FEC_ENET_GRA));
> >  
> >  	/* Whack a reset.  We should wait for this.
> >  	*/
> > _______________________________________________
> > uClinux-dev mailing list
> > uClinux-dev at uclinux.org
> > http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
> > This message was resent by uclinux-dev at uclinux.org
> > 
> 
> -- 
> ------------------------------------------------------------------------
> Greg Ungerer  --  Chief Software Dude       EMAIL:     gerg at snapgear.com
> SnapGear -- a CyberGuard Company            PHONE:       +61 7 3435 2888
> 825 Stanley St,                             FAX:         +61 7 3891 3630
> Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com
> _______________________________________________
> uClinux-dev mailing list
> uClinux-dev at uclinux.org
> http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
> This message was resent by uclinux-dev at uclinux.org
> 




More information about the uClinux-dev mailing list