linux-mips
[Top] [All Lists]

Re: [PATCH] Oops! - Re: Power management for au1000_eth.c

To: Linux MIPS <linux-mips@linux-mips.org>
Subject: Re: [PATCH] Oops! - Re: Power management for au1000_eth.c
From: Rodolfo Giometti <giometti@linux.it>
Date: Thu, 6 Apr 2006 17:50:11 +0200
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>, Jordan Crouse <jordan.crouse@amd.com>, Pete Popov <ppopov@embeddedalley.com>
In-reply-to: <4435290C.50607@ru.mvista.com>
Organization: GNU/Linux Device Drivers, Embedded Systems and Courses
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20060405154711.GL7029@enneenne.com> <20060405222332.GO7029@enneenne.com> <20060405222620.GP7029@enneenne.com> <4435290C.50607@ru.mvista.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.11+cvs20060126
On Thu, Apr 06, 2006 at 06:43:24PM +0400, Sergei Shtylyov wrote:
> 
>    Actually, the network driver patches should be sent to Jeff Garzik and 
> Andrew Morton.

Ok. Sorry. Hope they can get the patch from this list.

>    Funny, I've also been cooking a patch to straighten Alchemy Ethernet 
> probing code a bit...

:)

> >------------------------------------------------------------------------
> >
> >--- 
> >/home/develop/embedded/mipsel/linux/linux-mips.git/arch/mips/au1000/common/au1xxx_irqmap.c
> >   2006-03-31 16:57:26.000000000 +0200
> >+++ arch/mips/au1000/common/au1xxx_irqmap.c  2006-04-03 
> >17:50:49.000000000 +0200
> >@@ -118,7 +118,7 @@
> >     { AU1000_USB_DEV_SUS_INT, INTC_INT_RISE_EDGE, 0 },
> >     { AU1000_USB_HOST_INT, INTC_INT_LOW_LEVEL, 0 },
> >     { AU1000_ACSYNC_INT, INTC_INT_RISE_EDGE, 0 },
> >-    { AU1500_MAC0_DMA_INT, INTC_INT_HIGH_LEVEL, 0},
> >+    { AU1000_MAC0_DMA_INT, INTC_INT_HIGH_LEVEL, 0},
> >     { AU1500_MAC1_DMA_INT, INTC_INT_HIGH_LEVEL, 0},
> >     { AU1000_AC97C_INT, INTC_INT_RISE_EDGE, 0 },
> > 
> >@@ -152,7 +152,7 @@
> >     { AU1000_USB_DEV_SUS_INT, INTC_INT_RISE_EDGE, 0 },
> >     { AU1000_USB_HOST_INT, INTC_INT_LOW_LEVEL, 0 },
> >     { AU1000_ACSYNC_INT, INTC_INT_RISE_EDGE, 0 },
> >-    { AU1100_MAC0_DMA_INT, INTC_INT_HIGH_LEVEL, 0},
> >+    { AU1000_MAC0_DMA_INT, INTC_INT_HIGH_LEVEL, 0},
> >     /*{ AU1000_GPIO215_208_INT, INTC_INT_HIGH_LEVEL, 0},*/
> >     { AU1100_LCD_INT, INTC_INT_HIGH_LEVEL, 0},
> >     { AU1000_AC97C_INT, INTC_INT_RISE_EDGE, 0 },
> 
>    Don't think these changes are necessary.

These are necessary to group togheter CPUs that have the same
parameters.

> >--- 
> >/home/develop/embedded/mipsel/linux/linux-mips.git/arch/mips/au1000/common/platform.c
> >        2006-04-03 18:22:05.000000000 +0200
> >+++ arch/mips/au1000/common/platform.c       2006-04-05 
> >23:08:55.000000000 +0200
> >@@ -16,6 +16,78 @@
> > 
> > #include <asm/mach-au1x00/au1xxx.h>
> > 
> >+#if defined(CONFIG_MIPS_AU1X00_ENET) || 
> >defined(CONFIG_MIPS_AU1X00_ENET_MODULE)
> >+/* Ethernet controllers */
> >+static struct resource au1xxx_eth0_resources[] = {
> >+    [0] = {
> >+            .name   = "eth-base",
> >+            .start  = ETH0_BASE,
> >+            .end    = ETH0_BASE + 0x0ffff,
> > +           .flags  = IORESOURCE_MEM,
> > +   },
> 
>    NAK, ETH0_BASE not defined anywhere, and that address differs between 
>    SOCs.
> Note that this must be a *physical* address, not KSEG1-base.

You are right! I forgot to add the attached file... I'm very sorry but
I must work on an older Linux version which still use CVS. I can't
switch to GIT right now and I have to build my patches by hands.

In the attached file you can see that I grouped togheter CPUs that
have the same configuration parameters.

> >     /* Setup some variables for quick register address access */
> >-    if (ioaddr == iflist[0].base_addr)
> >+    if (port_num == 0)
> 
>    Yep, I disliked this code too. :-)

I just use the "id" filed instaed of using the base address. It seems
to me more readable...

> >     {
> >             /* check env variables first */
> >             if (!get_ethernet_addr(ethaddr)) { 
> >@@ -1495,7 +1426,7 @@
> >                     argptr = prom_getcmdline();
> >                     if ((pmac = strstr(argptr, "ethaddr=")) == NULL) {
> >                             printk(KERN_INFO "%s: No mac address 
> >                             found\n", -                                     
> >         dev->name);
> >+                                            ndev->name);
> >                             /* use the hard coded mac addresses */
> >                     } else {
> >                             str2eaddr(ethaddr, pmac + 
> >                             strlen("ethaddr="));
> >@@ -1504,26 +1435,26 @@
> >                     }
> >             }
> >                     aup->enable = (volatile u32 *) 
> >-                            ((unsigned long)iflist[0].macen_addr);
> >-            memcpy(dev->dev_addr, au1000_mac_addr, 
> >sizeof(au1000_mac_addr));
> >+                            ((unsigned long) macen_addr);
> 
>    NAK, macen_addr should have been a physical address at that point too
> (if the plarform definitions were correct :-).  Also, this could have been 
> done
> outside of "if".

I just keeped the original structure...

> >             setup_hw_rings(aup, MAC0_RX_DMA_ADDR, MAC0_TX_DMA_ADDR);
> >             aup->mac_id = 0;
> >             au_macs[0] = aup;
> >     }
> >             else
> >-    if (ioaddr == iflist[1].base_addr)
> >+    if (port_num == 1)
> >     {
> >                     aup->enable = (volatile u32 *) 
> >-                            ((unsigned long)iflist[1].macen_addr);
> >-            memcpy(dev->dev_addr, au1000_mac_addr, 
> >sizeof(au1000_mac_addr));
> >-            dev->dev_addr[4] += 0x10;
> >+                            ((unsigned long) macen_addr);
> >+            memcpy(ndev->dev_addr, au1000_mac_addr, 
> >sizeof(au1000_mac_addr));
> >+            ndev->dev_addr[4] += 0x10;
> 
>    Actually, the DBAu15x0 boards have their Ethernet addresess differ by 1 
>    in the last byte, not by 0x10 in the next to last one. This code assigns 
> to 
> the port an address different to what's printed on a port's sticker. This 
> should be fixed I guess...

Yes. However this comes from the previous driver version and I'm
working on an Au1100 based board. :)

> >@@ -2255,5 +2162,232 @@
> >     return 0;
> > }
> > 
> >-module_init(au1000_init_module);
> >-module_exit(au1000_cleanup_module);
> >+/*
> >+ * Setup the base address and interupt of the Au1xxx ethernet macs
> >+ * based on cpu type and whether the interface is enabled in sys_pinfunc
> >+ * register. The last interface is enabled if SYS_PF_NI2 (bit 4) is 0.
> >+ */
> >+static int au1000_drv_probe(struct device *dev)
> >+{
> >+    struct platform_device *pdev = to_platform_device(dev);
> >+    struct net_device *ndev;
> >+    struct au1000_private *aup;
> >+    struct resource *res;
> >+    static unsigned version_printed = 0;
> >+    u32 base_addr, macen_addr;
> >+    int irq, ret;
> >+
> >+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "eth-base");
> >+    if (!res) {
> >+            ret = -ENODEV;
> >+            goto out;
> >+    }
> >+    base_addr = res->start;
> >+    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "eth-mac");
> >+    if (!res) {
> >+            ret = -ENODEV;
> >+            goto out;
> >+    }
> >+    macen_addr = res->start;
> >+    res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "eth-irq");
> >+    if (!res) {
> >+            ret = -ENODEV;
> >+            goto out;
> >+    }
> >+    irq = res->start;
> >+
> >+    if (!request_mem_region(CPHYSADDR(base_addr), MAC_IOSIZE, "Au1x00 
> >ENET")) {
> 
>    NAK, base_addr should adready be a physical address. Also, why no 
> request_mem_region()
> for the MAC enable register?

Yes. I forgot to add it.

Thanks for your suggestions! But I'd like to know if I have to change
something and resubmit the patch or these suggestions can be resolved
during the merging of the code...

Ciao,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    giometti@enneenne.com
Linux Device Driver                             giometti@gnudd.com
Embedded Systems                                giometti@linux.it
UNIX programming                     phone:     +39 349 2432127

Attachment: patch-au1000_eth-pm-2
Description: Text document

Attachment: signature.asc
Description: Digital signature

<Prev in Thread] Current Thread [Next in Thread>