Skip to contentSkip to conversation

au1000_lowlevel_probe on au1000_eth.c

DISCUSSION Rodolfo Giometti 3 participants 7 messages Jun 26, 2006 – Jul 2, 2006

Subsystems (2)
Network drivers MIPS
lore
Share & tools
lore

Rodolfo Giometti

author
#0 lore raw

Hello,

I notice that during sleep/wakeup au1000_lowlevel_probe() tries to access to variables arcs_cmdline,prom_envp & Co.. This sometime does an oops.

What I like to understand is if I simply have to skip accessing to these variables during wake up or I have to save them during system boot or what...?

Thanks,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
#1 lore raw
Quoted 7 lines · Rodolfo Giometti

I notice that during sleep/wakeup au1000_lowlevel_probe() tries to access to variables arcs_cmdline,prom_envp & Co.. This sometime does an oops.

What I like to understand is if I simply have to skip accessing to these variables during wake up or I have to save them during system boot or what...?

What is the _right_ thing is difficult to answer, but the reasons for the oops are simple (at least they were on the platform I used): you have more than one device connected to the same system bus. In my case, it was the boot-PROM and (I think) the PCMCIA memory. So, in practice one could only access one of the two and you had to toggle between them. I'm not sure it is worth the hassle of toggling (and doing so in an orderly fashion!), I'd rather parse the PROM once and then store the results in RAM.

HTH

Uli

****************************************************
Visit our website at <http://www.domino-printing.com/>
****************************************************
This Email and any files transmitted with it are intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any reading, redistribution, disclosure or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient please contact the sender immediately and delete the material from your computer.

E-mail may be susceptible to data corruption, interception, viruses and unauthorised amendment and Domino UK Limited does not accept liability for any such corruption, interception, viruses or amendment or their consequences.
****************************************************

author
#2 lore raw
Quoted 8 lines · Ulrich Eckhardt

What is the _right_ thing is difficult to answer, but the reasons for the oops are simple (at least they were on the platform I used): you have more than one device connected to the same system bus. In my case, it was the boot-PROM and (I think) the PCMCIA memory. So, in practice one could only access one of the two and you had to toggle between them. I'm not sure it is worth the hassle of toggling (and doing so in an orderly fashion!), I'd rather parse the PROM once and then store the results in RAM.

I see. But maybe the «best» thing to do is just to skip these settings during wake up after a sleep since they are magniful only during boot time...

Ciao,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
author
#3 lore raw
In reply to Rodolfo Giometti
Quoted 4 lines · Rodolfo Giometti

I notice that during sleep/wakeup au1000_lowlevel_probe() tries to access to variables arcs_cmdline,prom_envp & Co.. This sometime does an oops.

Here my proposal to avoid oops during wake up.

Ciao,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
drivers/net/au1000_eth.c +20 -19
index 341fdc4..c49004a 100644
@@ -84,7 +84,7 @@ MODULE_LICENSE("GPL");
 // prototypes
 static void hard_stop(struct net_device *);
 static void enable_rx_tx(struct net_device *dev);
-static int au1000_lowlevel_probe(struct net_device *ndev, void *ioaddr, void *macen_addr, int port_num);
+static int au1000_lowlevel_probe(struct net_device *ndev, void *ioaddr, void *macen_addr, int port_num, int skip_prom);
 static void au1000_lowlevel_remove(struct net_device *ndev);
 static int au1000_init(struct net_device *);
 static int au1000_open(struct net_device *);

@@ -1393,7 +1393,7 @@ static struct ethtool_ops au1000_ethtool
 };
 
 static int 
-au1000_lowlevel_probe(struct net_device *ndev, void *ioaddr, void *macen_addr, int port_num)
+au1000_lowlevel_probe(struct net_device *ndev, void *ioaddr, void *macen_addr, int port_num, int skip_prom)
 {
 	struct au1000_private *aup = ndev->priv;
 	db_dest_t *pDB, *pDBfree;

@@ -1419,24 +1419,25 @@ au1000_lowlevel_probe(struct net_device 
 	/* Setup some variables for quick register address access */
 	if (port_num == 0)
 	{
-		/* check env variables first */
-		if (!get_ethernet_addr(ethaddr)) { 
-			memcpy(au1000_mac_addr, ethaddr, sizeof(au1000_mac_addr));
-		} else {
-			/* Check command line */
-			argptr = prom_getcmdline();
-			if ((pmac = strstr(argptr, "ethaddr=")) == NULL) {
-				printk(KERN_INFO "%s: No mac address found\n", 
-						ndev->name);
-				/* use the hard coded mac addresses */
+		if (!skip_prom) {
+			/* check env variables first */
+			if (!get_ethernet_addr(ethaddr)) { 
+				memcpy(au1000_mac_addr, ethaddr, sizeof(au1000_mac_addr));
 			} else {
-				str2eaddr(ethaddr, pmac + strlen("ethaddr="));
-				memcpy(au1000_mac_addr, ethaddr, 
-						sizeof(au1000_mac_addr));
+				/* Check command line */
+				argptr = prom_getcmdline();
+				if ((pmac = strstr(argptr, "ethaddr=")) == NULL) {
+					printk(KERN_INFO "%s: No mac address found\n", 
+							ndev->name);
+					/* use the hard coded mac addresses */
+				} else {
+					str2eaddr(ethaddr, pmac + strlen("ethaddr="));
+					memcpy(au1000_mac_addr, ethaddr, 
+							sizeof(au1000_mac_addr));
+				}
 			}
 		}
-			aup->enable = (volatile u32 *) 
-				((unsigned long) macen_addr);
+		aup->enable = (volatile u32 *) ((unsigned long) macen_addr);
 		memcpy(ndev->dev_addr, au1000_mac_addr, sizeof(au1000_mac_addr));
 		setup_hw_rings(aup, MAC0_RX_DMA_ADDR, MAC0_TX_DMA_ADDR);
 		aup->mac_id = 0;

@@ -2196,7 +2197,7 @@ static int au1000_drv_probe(struct devic
 
 	/* Force the device name to a know state... */
 	sprintf(ndev->name, "au1xxx_eth(%d)", pdev->id);
-	ret = au1000_lowlevel_probe(ndev, base_addr, macen_addr, pdev->id);
+	ret = au1000_lowlevel_probe(ndev, base_addr, macen_addr, pdev->id, 0);
 	if (ret < 0) {
 		printk (KERN_ERR "%s: low level probe failed\n", DRV_NAME);  
 		goto out_free_netdev;

@@ -2304,7 +2305,7 @@ static int au1000_drv_resume(struct devi
 	if (!ndev)
 		return 0;
 
-	ret = au1000_lowlevel_probe(ndev, (void *) aup->mac, (void *) aup->enable, aup->mac_id);
+	ret = au1000_lowlevel_probe(ndev, (void *) aup->mac, (void *) aup->enable, aup->mac_id, ~0);
 	if (ret < 0) {
 		printk (KERN_ERR "%s: low level probe failed\n", DRV_NAME);  
 		return ret;
#4 lore raw

Hello.

Quoted 7 lines · Rodolfo Giometti
Quoted 3 lines · Rodolfo Giometti

I notice that during sleep/wakeup au1000_lowlevel_probe() tries to access to variables arcs_cmdline,prom_envp & Co.. This sometime does an oops.

Here my proposal to avoid oops during wake up.

This is against your rewrite, if I don't mistake?

Quoted 3 lines

Ciao,

Rodolfo

WBR, Sergei

Quoted 30 lines

------------------------------------------------------------------------

drivers/net/au1000_eth.c +6 -13
index 341fdc4..c49004a 100644
@@ -1419,24 +1419,25 @@ au1000_lowlevel_probe(struct net_device 
 	/* Setup some variables for quick register address access */
 	if (port_num == 0)
 	{
-		/* check env variables first */
-		if (!get_ethernet_addr(ethaddr)) { 
-			memcpy(au1000_mac_addr, ethaddr, sizeof(au1000_mac_addr));
-		} else {
-			/* Check command line */
-			argptr = prom_getcmdline();
-			if ((pmac = strstr(argptr, "ethaddr=")) == NULL) {
-				printk(KERN_INFO "%s: No mac address found\n", 
-						ndev->name);
-				/* use the hard coded mac addresses */
+		if (!skip_prom) {
+			/* check env variables first */
+			if (!get_ethernet_addr(ethaddr)) { 
+				memcpy(au1000_mac_addr, ethaddr, sizeof(au1000_mac_addr));
 			} else {
-				str2eaddr(ethaddr, pmac + strlen("ethaddr="));
-				memcpy(au1000_mac_addr, ethaddr, 
-						sizeof(au1000_mac_addr));
+				/* Check command line */
+				argptr = prom_getcmdline();

drivers/net/au1000_eth.c au1000_lowlevel_probe(struct net_device · line 1438

Quoted 10 lines
drivers/net/au1000_eth.c +9 -0
+				if ((pmac = strstr(argptr, "ethaddr=")) == NULL) {
+					printk(KERN_INFO "%s: No mac address found\n", 
+							ndev->name);
+					/* use the hard coded mac addresses */
+				} else {
+					str2eaddr(ethaddr, pmac + strlen("ethaddr="));
+					memcpy(au1000_mac_addr, ethaddr, 
+							sizeof(au1000_mac_addr));
+				}
 			}

Hrm, wouldn't it be better to put this stuff into a separate function then?

WBR, Sergei

author
#5 lore raw
Quoted 2 lines · Sergei Shtylyov

This is against your rewrite, if I don't mistake?

Yes.

Quoted 2 lines

Hrm, wouldn't it be better to put this stuff into a separate function then?

Maybe, but I considered that these stuff are dignificative only during boot time so I decided to do not consider them during wake up. Is that wrong?

Ciao,

Rodolfo

Signature · 4 lines
GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127
#6 lore raw

Hello.

Quoted 6 lines · Rodolfo Giometti
Quoted 2 lines

Hrm, wouldn't it be better to put this stuff into a separate function then?

Maybe, but I considered that these stuff are dignificative only during boot time so I decided to do not consider them during wake up. Is that wrong?

No. And exactly because of this, the __init time stuff should better be
separated (if possible)...

Quoted 3 lines

Ciao,

Rodolfo