RE: [PATCH] ucc_geth: add support for netpoll
-Original Message- From: Anton Vorontsov [mailto:[EMAIL PROTECTED] Sent: Thursday, November 01, 2007 5:59 AM To: Li Yang-r58472 Cc: netdev@vger.kernel.org; [EMAIL PROTECTED]; [EMAIL PROTECTED] Subject: Re: [PATCH] ucc_geth: add support for netpoll On Mon, Oct 29, 2007 at 03:17:44PM +0300, Anton Vorontsov wrote: [...] Oops. The original patch happened to hit the Junk mail box. :( That one as well? http://lkml.org/lkml/2007/10/11/128 I think the patch is good to merge after the cosmetic change. I can do it in next pull request to Jeff. Ok, great. Thanks. I'm wondering if you missed that email again. Maybe your mail client/server doing weird things with emails from @ru.mvista.com? No. I have explicitly add you to the whitelist. :) Please be patient, isn't this patch a new feature which can only be integrated in the merge window? Thanks. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] Fix ethernet multicast for ucc_geth.
@@ -2255,19 +2253,10 @@ static void ucc_geth_set_multi(struct net_device *dev) if (!(dmi-dmi_addr[0] 1)) continue; - /* The address in dmi_addr is LSB first, -* and taddr is MSB first. We have to -* copy bytes MSB first from dmi_addr. -*/ - mcptr = (u8 *) dmi-dmi_addr + 5; - tdptr = (u8 *) tempaddr; - for (j = 0; j 6; j++) - *tdptr++ = *mcptr--; - /* Ask CPM to run CRC and set bit in * filter mask. */ - hw_add_addr_in_hash(ugeth, tempaddr); + hw_add_addr_in_hash(ugeth, dmi-dmi_addr); did the maintainer ever ACK this? Yes, I did. :) - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ucc_geth: add support for netpoll
-Original Message- From: Anton Vorontsov [mailto:[EMAIL PROTECTED] Sent: Saturday, October 27, 2007 10:38 PM To: Sergei Shtylyov Cc: Anton Vorontsov; netdev@vger.kernel.org; Li Yang-r58472; [EMAIL PROTECTED]; [EMAIL PROTECTED] Subject: Re: [PATCH] ucc_geth: add support for netpoll On Sat, Oct 27, 2007 at 05:09:51PM +0400, Sergei Shtylyov wrote: Hello. Anton Vorontsov wrote: This patch adds netpoll support for the QE UCC Gigabit Ethernet driver. The approach is very similar to the gianfar driver. It's rather contrarywise -- this is standard approach and gianfar with its 3 TSEC IRQs has a quite non-standard poll_controller() implementation. Oh.. well, right -- gianfar a bit more comlex in that regard. Tested using netconsole. KGDBoE is considered a better test (I hope you've also tested with it). At the time of posting it was tested using netconsole only, a few days later it's was tested using KGDBoE also. So, it works indeed. Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index 18a6f48..06807ce 100644 --- a/drivers/net/ucc_geth.c +++ b/drivers/net/ucc_geth.c @@ -3691,6 +3691,22 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info) return IRQ_HANDLED; } +#ifdef CONFIG_NET_POLL_CONTROLLER +/* + * Polling 'interrupt' - used by things like netconsole to send +skbs + * without having to re-enable interrupts. It's not called while + * the interrupt routine is executing. + */ +static void ucc_netpoll(struct net_device *dev) { + struct ucc_geth_private *ugeth = netdev_priv(dev); + + disable_irq(ugeth-ug_info-uf_info.irq); + ucc_geth_irq_handler(ugeth-ug_info-uf_info.irq, dev); + enable_irq(ugeth-ug_info-uf_info.irq); Why not make it less complex (for a reader and gcc too :-) ? Yup, I'm agree here but it's too late. Again. ;-) This patch already accepted into the -mm (a week or so after the silence), so.. now I'd rather not bother Andrew with such really cosmetic changes. But if Jeff would directly apply modfied patch, I'll send it. ;-) Oops. The original patch happened to hit the Junk mail box. :( I think the patch is good to merge after the cosmetic change. I can do it in next pull request to Jeff. Thanks - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings
-Original Message- From: Medve Emilian-EMMEDVE1 Sent: Monday, October 22, 2007 9:48 PM To: David Miller Cc: [EMAIL PROTECTED]; Li Yang-r58472; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: RE: [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings Hello David, No piece of code in the kernel should live in a vacuum. In order to improve overall code quality, every piece of driver code should avoid assuming things about pointer sizes and things of this nature. I'm afraid we might be talking about orthogonal issues here. I actively agree that all code (not only kernel) should be written up to the coding/quality standards you mention above, but I see a difference between fixing a warning and making a driver portable (to 64-bit PowerPCs, to other platforms, etc.). If there is a kernel todo list somewhere lets add to it the task to make the ucc_geth more portable. Then the driver can get enabled into the build on every platform, and therefore nobody will break the build of this driver again since it will get hit by allmodconfig et al. builds even on platforms other than the one it is meant for. This hack fix is not acceptable, really. Are you suggesting we leave those warnings there until somebody decides to fix all the portability issues of this driver? My patch is a small and insignificant improvement and not the revolution you're asking for, but is an small improvement today (I dislike warnings) vs. an improbable big one in the future. I'd say we can not use our way of doing things while working with the community. The community has to consider the kernel as a whole and thus has its own virtue. The warning has been there for some time. It stays as an indicator that we have something to do to improve the portability. I will work on a patch to fix this portability issue. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] Fix ethernet multicast for ucc_geth.
-Original Message- From: Joakim Tjernlund [mailto:[EMAIL PROTECTED] Sent: Wednesday, October 17, 2007 5:06 PM To: Netdev; Li Yang-r58472 Subject: [PATCH] Fix ethernet multicast for ucc_geth. From 5761a9e5924b34615c748fba2dcb977ed04c1243 Mon Sep 17 00:00:00 2001 From: Joakim Tjernlund [EMAIL PROTECTED] Date: Wed, 17 Oct 2007 11:01:44 +0200 Subject: [PATCH] Fix ethernet multicast for ucc_geth. hw_add_addr_in_hash() already swaps byte order, don't do it in ucc_geth_set_multi() too. Signed-off-by: Joakim Tjernlund [EMAIL PROTECTED] Acked-by: Li Yang [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [net] fix gianfar (compile and warning)
-Original Message- From: Sebastian Siewior [mailto:[EMAIL PROTECTED] Sent: Wednesday, October 17, 2007 1:25 AM To: [EMAIL PROTECTED] Cc: Li Yang-r58472; netdev@vger.kernel.org; Grant Likely Subject: [PATCH] [net] fix gianfar (compile and warning) Currently it does not compile because a not declared variable is used. struct net_device_stats in driver's private struct is also unsued. This patch uses (hopefully) the right stats. Cc: Li Yang [EMAIL PROTECTED] Signed-off-by: Sebastian Siewior [EMAIL PROTECTED] I have also submitted a patch to fixed this compile error two days before, and it has been applied on Jeff's netdev tree. But you can submit a separate patch to cleanup the struct net_device_stats in driver's private struct. Thanks. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] [POWERPC] ucc_geth: Fix build break introduced by commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0
-Original Message- From: Medve Emilian-EMMEDVE1 Sent: Saturday, October 13, 2007 7:26 AM To: [EMAIL PROTECTED]; [EMAIL PROTECTED]; Li Yang-r58472; netdev@vger.kernel.org; [EMAIL PROTECTED] Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH] [POWERPC] ucc_geth: Fix build break introduced by commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0 CC drivers/net/ucc_geth.o drivers/net/ucc_geth.c: In function 'ucc_geth_startup': drivers/net/ucc_geth.c:2614: warning: assignment makes integer from pointer without a cast drivers/net/ucc_geth.c:2651: warning: assignment makes integer from pointer without a cast drivers/net/ucc_geth.c: In function 'ucc_geth_rx': drivers/net/ucc_geth.c:3483: error: 'dev' undeclared (first use in this function) drivers/net/ucc_geth.c:3483: error: (Each undeclared identifier is reported only once drivers/net/ucc_geth.c:3483: error: for each function it appears in.) make[2]: *** [drivers/net/ucc_geth.o] Error 1 Thanks for the patch. Here are some comments. The patch fixes the dev undeclared compile error not the warnings. So they shouldn't be listed here. Signed-off-by: Emil Medve [EMAIL PROTECTED] --- Here is a convenient link for the culprit patch: http://git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.g it;a=commit;h=09f75cd7bf13720738e6a196cc0107ce9a5bd5a0 netdev-2.6 scripts/checkpatch.pl 0001-POWERPC-ucc_geth-Fix-build-break-introduced-by-co.patch Your patch has no obvious style problems and is ready for submission. drivers/net/ucc_geth.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index d00e7d4..c43d4d1 100644 --- a/drivers/net/ucc_geth.c +++ b/drivers/net/ucc_geth.c @@ -63,7 +63,7 @@ #define UGETH_MSG_DEFAULT(NETIF_MSG_IFUP 1 ) - 1 void uec_set_ethtool_ops(struct net_device *netdev); - + static DEFINE_SPINLOCK(ugeth_lock); static struct { There are quite a few references to the dev structure. I would prefer to add a new variable dev here. - Leo @@ -3480,9 +3480,9 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit dev_kfree_skb_any(skb); ugeth-rx_skbuff[rxQ][ugeth-skb_currx[rxQ]] = NULL; - dev-stats.rx_dropped++; + ugeth-dev-stats.rx_dropped++; } else { - dev-stats.rx_packets++; + ugeth-dev-stats.rx_packets++; howmany++; /* Prep the skb for the packet */ @@ -3491,7 +3491,7 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit /* Tell the skb what kind of packet this is */ skb-protocol = eth_type_trans(skb, ugeth-dev); - dev-stats.rx_bytes += length; + ugeth-dev-stats.rx_bytes += length; /* Send the packet up the stack */ #ifdef CONFIG_UGETH_NAPI netif_receive_skb(skb); @@ -3506,7 +3506,7 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit if (!skb) { if (netif_msg_rx_err(ugeth)) ugeth_warn(%s: No Rx Data Buffer, __FUNCTION__); - dev-stats.rx_dropped++; + ugeth-dev-stats.rx_dropped++; break; } -- 1.5.3.GIT - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] [POWERPC] ucc_geth: Fix build break introduced by commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0
-Original Message- From: Medve Emilian-EMMEDVE1 Sent: Monday, October 15, 2007 9:44 PM To: [EMAIL PROTECTED]; [EMAIL PROTECTED]; Li Yang-r58472; netdev@vger.kernel.org; [EMAIL PROTECTED] Cc: Medve Emilian-EMMEDVE1 Subject: [PATCH v2] [POWERPC] ucc_geth: Fix build break introduced by commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0 drivers/net/ucc_geth.c: In function 'ucc_geth_rx': drivers/net/ucc_geth.c:3483: error: 'dev' undeclared (first use in this function) drivers/net/ucc_geth.c:3483: error: (Each undeclared identifier is reported only once drivers/net/ucc_geth.c:3483: error: for each function it appears in.) make[2]: *** [drivers/net/ucc_geth.o] Error 1 Signed-off-by: Emil Medve [EMAIL PROTECTED] Acked-by: Li Yang [EMAIL PROTECTED] --- Here is a convenient link for the culprit patch: http://git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.g it;a=commit;h=09f75cd7bf13720738e6a196cc0107ce9a5bd5a0 netdev-2.6 scripts/checkpatch.pl 0001-POWERPC-ucc_geth-Fix-build-break-introduced-by-co.patch Your patch has no obvious style problems and is ready for submission. drivers/net/ucc_geth.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index d00e7d4..bec413b 100644 --- a/drivers/net/ucc_geth.c +++ b/drivers/net/ucc_geth.c @@ -63,7 +63,7 @@ #define UGETH_MSG_DEFAULT(NETIF_MSG_IFUP 1 ) - 1 void uec_set_ethtool_ops(struct net_device *netdev); - + static DEFINE_SPINLOCK(ugeth_lock); static struct { @@ -3454,9 +3454,12 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit u16 length, howmany = 0; u32 bd_status; u8 *bdBuffer; + struct net_device * dev; ugeth_vdbg(%s: IN, __FUNCTION__); + dev = ugeth-dev; + /* collect received buffers */ bd = ugeth-rxBd[rxQ]; -- 1.5.3.GIT - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ucc_geth: fix compilation
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Kumar Gala Sent: Friday, September 14, 2007 10:08 PM To: Jeff Garzik Cc: [EMAIL PROTECTED] list; netdev Subject: Re: [PATCH] ucc_geth: fix compilation On Sep 13, 2007, at 10:23 AM, Anton Vorontsov wrote: Currently qe_bd_t is used in the macro call -- dma_unmap_single, which is a no-op on PPC32, thus error is hidden today. Starting with 2.6.24, macro will be replaced by the empty static function, and erroneous use of qe_bd_t will trigger compilation error. Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] --- Jeff, I'm going to pick this up via the powerpc.git tree since its currently only broken in our for-2.6.24 branch (because of other changes in there). Any issues? Kumar, Kim Phillips has posted the same patch to netdev list before Anton. Therefore, I do prefer to use his. Thanks - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] QE Ethernet driver writes to wrong register to mask interrupts
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Tabi Timur-B04825 Sent: Tuesday, July 10, 2007 8:51 PM To: [EMAIL PROTECTED]; netdev@vger.kernel.org Cc: Tabi Timur-B04825 Subject: [PATCH] QE Ethernet driver writes to wrong register to mask interrupts The QE Ethernet driver was writing to the wrong register to mask interrupts. In ucc_geth_stop(), it was clearing UCCE instead of UCCM. Signed-off-by: Timur Tabi [EMAIL PROTECTED] Acked-by: Li Yang [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ucc_geth.c, make PHY device optional.
{snip} OK, but then a new property in PHY node is needed which holds the initial speed and duplex. initial-phy-capacity = speed duplex ? This should be useful for real PHYs too if, for instance, autoneg isn't working/supported/wanted. Maybe it should be: initial-phy-capacity = speed duplex autoneg or initial-phy-capability = speed duplex autoneg How about separate autoneg to a property dumb-phy, which indicates the PHY/switch doesn't provide MII register interface. Therefore, it should use the fixed speed and duplex from device node rather than registers. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ucc_geth.c, make PHY device optional.
-Original Message- From: Joakim Tjernlund [mailto:[EMAIL PROTECTED] Sent: Tuesday, July 03, 2007 7:20 PM To: Li Yang-r58472 Cc: linuxppc-dev Development; Netdev; Fleming Andy-afleming Subject: RE: [PATCH] ucc_geth.c, make PHY device optional. On Tue, 2007-07-03 at 16:22 +0800, Li Yang-r58472 wrote: -Original Message- From: Joakim Tjernlund [mailto:[EMAIL PROTECTED] Sent: Tuesday, July 03, 2007 3:21 PM To: Li Yang-r58472 Cc: linuxppc-dev Development; Netdev; Fleming Andy-afleming Subject: RE: [PATCH] ucc_geth.c, make PHY device optional. On Tue, 2007-07-03 at 11:42 +0800, Li Yang-r58472 wrote: -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Joakim Tjernlund Sent: Tuesday, July 03, 2007 8:52 AM To: 'linuxppc-dev Development'; 'Netdev'; Li Yang-r58472 Subject: [PATCH] ucc_geth.c, make PHY device optional. This patch makes the PHY optional for ucc_geth.c ethernet driver. This is useful to support a direct mii to mii connection to, for example, a onboard swicth. Signed-off-by: Joakim Tjernlund [EMAIL PROTECTED] Hi Joakim, I'm wondering if we really need to have the option to disable phylib. maybe, but it has to be dynamic too. I need to use PHY on UCC2 and mii on UCC3 and UCC4. Actually we have made phylib selected by default for ucc_geth. Many L2 switch chips have the capacity to be controlled. Therefore they can be managed as a phy device. Yes, they can be but why force a PHY impl. when its is of no use? The only thing the eth driver needs from the it is speed and duplex. If these are fixed, you don't need to talk with a PHY. The driver needs to get and set the link speed/status on runtime (such as for ethtool interface). Currently this is implementation through phydev interface. IMHO, it will be easier to maintain if we only use this standard interface, rather than use different interfaces for different cases. hmm maybe, but there is no need to much around with speed/status from user space. The speed and duplex must be set before user space is up. For the MII interface which is not configurable, shouldn't we use the fixed phy support from Vitaly? Well, I think the the fixed phy is great when your eth driver requires a PHY, but it is a workaround with extra processing overhead. IMHO the best impl. is to make the PHY optional in the eth driver and as you can see from the patch, that was really simple. I agree there is overhead. However, it will have the advantage of abstracting all the PHY related stuff out of controller driver. An useful extension would be to add a new propety in the DTS to hold initial speed and duplex(perhaps extend phy-connection-type). This would be useful for the fixed driver too as one could derive speed and duplex for the fixed phy from that property instead of creating a fixed phy for each speed and duplex one want to support. I agree that there should be a device node to configure it. The current fixed phy driver is a little bit too complex to emulate the register access. Maybe it's better to have a null phy driver which just reads PHY capacity and status from device node. A null phy driver is better than the current fixed phy, I agree. Where would you like to put initial speed and duplex? In a fake phy node or in the ethernet node? I think a fake phy node is better. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/5] phylib: enable RGMII-ID on the Marvell m88e1111 PHY
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Kim Phillips Sent: Saturday, May 12, 2007 7:25 AM To: Jeff Garzik; netdev@vger.kernel.org Subject: [PATCH 2/5] phylib: enable RGMII-ID on the Marvell m88e PHY Support for configuring RGMII-ID (RGMII with internal delay) mode on the 88e and 88e1145. Also renamed 88es - 88e (no references to an 88es part were found), and fixed some whitespace. Signed-off-by: Kim Phillips [EMAIL PROTECTED] Acked-by: Li Yang [EMAIL PROTECTED] This is truly a fix for MPC8360EMDS board which was working on 2.6.21. Please push it to Linus in 2.6.22-rc phase rather than waiting until 2.6.23. Thanks. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] Add select PHYLIB to the UCC_GETH Kconfig option
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Jeff Garzik Sent: Friday, May 25, 2007 5:48 AM To: Jan Altenberg Cc: Phillips Kim-R1AAHA; [EMAIL PROTECTED]; netdev@vger.kernel.org Subject: Re: [PATCH] Add select PHYLIB to the UCC_GETH Kconfig option Jan Altenberg wrote: ucc_geth has been migrated to use the common phylib code. So lets add a 'select PHYLIB' to the UCC_GETH Kconfig entry. Signed-off-by: Jan Altenberg [EMAIL PROTECTED] --- drivers/net/Kconfig |1 + 1 file changed, 1 insertion(+) Index: linux-2.6/drivers/net/Kconfig === --- linux-2.6.orig/drivers/net/Kconfig +++ linux-2.6/drivers/net/Kconfig @@ -2280,6 +2280,7 @@ config GFAR_NAPI config UCC_GETH tristate Freescale QE Gigabit Ethernet depends on QUICC_ENGINE + select PHYLIB select UCC_FAST Please fix the Kconfig warnings first. I will follow up this. Also, I ask again: WHO IS THE MAINTAINER OF THIS DRIVER? I am tired of five independent people patching the same driver. Sorry for the trouble, I will post a patch to the MAINTAINER file and help to maintain ucc_geth related patches. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Patch 1/3] ucc_geth: Fix BD processing
-Original Message- From: Jeff Garzik [mailto:[EMAIL PROTECTED] Sent: Tuesday, March 06, 2007 7:11 PM To: Li Yang-r58472 Cc: netdev@vger.kernel.org Subject: Re: [Patch 1/3] ucc_geth: Fix BD processing Li Yang wrote: Fix broken BD processing code. Signed-off-by: Michael Barkowski [EMAIL PROTECTED] Signed-off-by: Li Yang [EMAIL PROTECTED] --- drivers/net/ucc_geth.c | 14 +- 1 files changed, 9 insertions(+), 5 deletions(-) applied 1-2 of 3 Thanks. What's your comment about 3 of 3? - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa
On Feb 6, 2007, at 5:31 AM, Li Yang wrote: Get rid of private immrbar_virt_to_phys() routine and use generic iopa(). Nack. iopa() isn't that generic, shouldn't we really be using the dma mapping API here? Do you mean the dma_map_single()? dma_map_single can map memory space correctly, but I don't think it can handle ioremap-ed space. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa
-Original Message- From: Timur Tabi [mailto:[EMAIL PROTECTED] Sent: Thursday, February 08, 2007 1:03 AM To: Kumar Gala Cc: Li Yang-r58472; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa Kumar Gala wrote: If its been mapped with ioremap() you know the physical address already so why do you need iopa(). That's what the original function immrbar_virt_to_phys() does. We're trying to get rid of it, because we thought is redundant with iopa(). static inline unsigned long immrbar_virt_to_phys(volatile void * address) { if ( ((u32)address = (u32)qe_immr) ((u32)address ((u32)qe_immr + QE_IMMAP_SIZE)) ) return (unsigned long)(address - (u32)qe_immr + (u32)get_qe_base()); return (unsigned long)virt_to_phys(address); } get_qe_base() does a search of the OF tree the first time it's called. Here's the code that calls immrbar_virt_to_phys(): out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, (u32) immrbar_virt_to_phys(ugeth- p_tx_bd_ring[i])); Would it be better to replace this code with something like this: out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, get_qe_base() + ((void *) ugeth-p_tx_bd_ring[i] - (void *) qe_immr)); No, we don't know if the BD ring is in MURAM or main memory as it is configurable. iopa() is best choice to handle both case, IMHO. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa
-Original Message- From: Kumar Gala [mailto:[EMAIL PROTECTED] Sent: Thursday, February 08, 2007 1:58 PM To: Li Yang-r58472 Cc: Tabi Timur-B04825; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa On Feb 7, 2007, at 11:52 PM, Li Yang-r58472 wrote: -Original Message- From: Timur Tabi [mailto:[EMAIL PROTECTED] Sent: Thursday, February 08, 2007 1:03 AM To: Kumar Gala Cc: Li Yang-r58472; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa Kumar Gala wrote: If its been mapped with ioremap() you know the physical address already so why do you need iopa(). That's what the original function immrbar_virt_to_phys() does. We're trying to get rid of it, because we thought is redundant with iopa(). static inline unsigned long immrbar_virt_to_phys(volatile void * address) { if ( ((u32)address = (u32)qe_immr) ((u32)address ((u32)qe_immr + QE_IMMAP_SIZE)) ) return (unsigned long)(address - (u32)qe_immr + (u32)get_qe_base()); return (unsigned long)virt_to_phys(address); } get_qe_base() does a search of the OF tree the first time it's called. Here's the code that calls immrbar_virt_to_phys(): out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, (u32) immrbar_virt_to_phys(ugeth- p_tx_bd_ring[i])); Would it be better to replace this code with something like this: out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, get_qe_base() + ((void *) ugeth-p_tx_bd_ring[i] - (void *) qe_immr)); No, we don't know if the BD ring is in MURAM or main memory as it is configurable. iopa() is best choice to handle both case, IMHO. Does MURAM behave differently than normal memory? MURAM is a mmio region so it don't share the characteristic of main memory that phy_addr = virt_addr - PAGE_OFFSET. While they can both be mapped through page table using iopa(). - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa
If its been mapped with ioremap() you know the physical address already so why do you need iopa(). That's what the original function immrbar_virt_to_phys() does. We're trying to get rid of it, because we thought is redundant with iopa(). static inline unsigned long immrbar_virt_to_phys(volatile void * address) { if ( ((u32)address = (u32)qe_immr) ((u32)address ((u32)qe_immr + QE_IMMAP_SIZE)) ) return (unsigned long)(address - (u32)qe_immr + (u32)get_qe_base()); return (unsigned long)virt_to_phys(address); } get_qe_base() does a search of the OF tree the first time it's called. Here's the code that calls immrbar_virt_to_phys(): out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, (u32) immrbar_virt_to_phys(ugeth- p_tx_bd_ring[i])); Would it be better to replace this code with something like this: out_be32(ugeth-p_send_q_mem_reg-sqqd[i].bd_ring_base, get_qe_base() + ((void *) ugeth-p_tx_bd_ring[i] - (void *) qe_immr)); No, we don't know if the BD ring is in MURAM or main memory as it is configurable. iopa() is best choice to handle both case, IMHO. Does MURAM behave differently than normal memory? MURAM is a mmio region so it don't share the characteristic of main memory that phy_addr = virt_addr - PAGE_OFFSET. While they can both be mapped through page table using iopa(). Right, so when do you know if you'll be using MURAM or normal memory? Why not just keep around a token that is the physical address at the point you make the decision of MURAM vs normal memory. Yes, that can be a way. But as the virt to phy mapping is only used once, it's nothing bad to do it this way. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa
-Original Message- From: Kumar Gala [mailto:[EMAIL PROTECTED] Sent: Thursday, February 08, 2007 3:16 PM To: Li Yang-r58472 Cc: Tabi Timur-B04825; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 1/4] ucc_geth: Change private immrbar_virt_to_phys to generic iopa On Feb 8, 2007, at 1:06 AM, Li Yang-r58472 wrote: MURAM is a mmio region so it don't share the characteristic of main memory that phy_addr = virt_addr - PAGE_OFFSET. While they can both be mapped through page table using iopa(). Right, so when do you know if you'll be using MURAM or normal memory? Why not just keep around a token that is the physical address at the point you make the decision of MURAM vs normal memory. Yes, that can be a way. But as the virt to phy mapping is only used once, it's nothing bad to do it this way. The problem as I stated before with using iopa() is that its not supported across platforms. Yes, it is only for PPC32. But we don't have another API to do it. How about make it more generic to add PPC64 version? - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [git patches] net driver fixes
Hi Jeff, Could you apply the updates for ucc_geth driver? The patches from Timur that make the driver compile correctly on 2.6.20. [PATCH] Fix phy_read/write redefinition errors in ucc_geth_phy.c [PATCH] Update ucc_geth.c for new workqueue structure The patch from Ahmed that cleans up some unnecessary casts. [PATCH 2.6.20-rc3] UCC Ether driver: kmalloc casting cleanups - Leo -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Jeff Garzik Sent: Monday, January 08, 2007 5:48 PM To: Andrew Morton; Linus Torvalds Cc: netdev@vger.kernel.org; LKML Subject: [git patches] net driver fixes Please pull from 'upstream-linus' branch of master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream-linus to receive the following updates: drivers/net/e1000/e1000_main.c |6 drivers/net/ixgb/ixgb.h |1 + drivers/net/ixgb/ixgb_ethtool.c |1 + drivers/net/ixgb/ixgb_hw.c |3 +- drivers/net/ixgb/ixgb_main.c| 57 ++ drivers/net/qla3xxx.c | 38 +++-- drivers/net/wireless/ipw2100.c |2 +- drivers/s390/net/qeth_main.c| 13 +--- include/net/ieee80211.h |2 +- 9 files changed, 88 insertions(+), 35 deletions(-) Aaron Salter (1): ixgb: Write RA register high word first, increment version Heiko Carstens (1): qeth: fix uaccess handling and get rid of unused variable Jeff Garzik (1): Revert e1000: disable TSO on the 82544 with slab debugging Jesse Brandeburg (2): ixgb: Fix early TSO completion ixgb: Maybe stop TX if not enough free descriptors Ron Mercer (2): qla3xxx: Remove NETIF_F_LLTX from driver features. qla3xxx: Add delay to NVRAM register access. Zhu Yi (2): ieee80211: WLAN_GET_SEQ_SEQ fix (select correct region) ipw2100: Fix dropping fragmented small packet problem diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c index 4c1ff75..c6259c7 100644 --- a/drivers/net/e1000/e1000_main.c +++ b/drivers/net/e1000/e1000_main.c @@ -995,12 +995,6 @@ e1000_probe(struct pci_dev *pdev, (adapter-hw.mac_type != e1000_82547)) netdev-features |= NETIF_F_TSO; -#ifdef CONFIG_DEBUG_SLAB - /* 82544's work arounds do not play nicely with DEBUG SLAB */ - if (adapter-hw.mac_type == e1000_82544) - netdev-features = ~NETIF_F_TSO; -#endif - #ifdef NETIF_F_TSO6 if (adapter-hw.mac_type e1000_82547_rev_2) netdev-features |= NETIF_F_TSO6; diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h index 50ffe90..f4aba43 100644 --- a/drivers/net/ixgb/ixgb.h +++ b/drivers/net/ixgb/ixgb.h @@ -171,6 +171,7 @@ struct ixgb_adapter { /* TX */ struct ixgb_desc_ring tx_ring cacheline_aligned_in_smp; + unsigned int restart_queue; unsigned long timeo_start; uint32_t tx_cmd_type; uint64_t hw_csum_tx_good; diff --git a/drivers/net/ixgb/ixgb_ethtool.c b/drivers/net/ixgb/ixgb_ethtool.c index cd22523..82c044d 100644 --- a/drivers/net/ixgb/ixgb_ethtool.c +++ b/drivers/net/ixgb/ixgb_ethtool.c @@ -79,6 +79,7 @@ static struct ixgb_stats ixgb_gstrings_stats[] = { {tx_window_errors, IXGB_STAT(net_stats.tx_window_errors)}, {tx_deferred_ok, IXGB_STAT(stats.dc)}, {tx_timeout_count, IXGB_STAT(tx_timeout_count) }, + {tx_restart_queue, IXGB_STAT(restart_queue) }, {rx_long_length_errors, IXGB_STAT(stats.roc)}, {rx_short_length_errors, IXGB_STAT(stats.ruc)}, #ifdef NETIF_F_TSO diff --git a/drivers/net/ixgb/ixgb_hw.c b/drivers/net/ixgb/ixgb_hw.c index 02089b6..ecbf458 100644 --- a/drivers/net/ixgb/ixgb_hw.c +++ b/drivers/net/ixgb/ixgb_hw.c @@ -399,8 +399,9 @@ ixgb_init_rx_addrs(struct ixgb_hw *hw) /* Zero out the other 15 receive addresses. */ DEBUGOUT(Clearing RAR[1-15]\n); for(i = 1; i IXGB_RAR_ENTRIES; i++) { - IXGB_WRITE_REG_ARRAY(hw, RA, (i 1), 0); + /* Write high reg first to disable the AV bit first */ IXGB_WRITE_REG_ARRAY(hw, RA, ((i 1) + 1), 0); + IXGB_WRITE_REG_ARRAY(hw, RA, (i 1), 0); } return; diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c index e628126..a083a91 100644 --- a/drivers/net/ixgb/ixgb_main.c +++ b/drivers/net/ixgb/ixgb_main.c @@ -36,7 +36,7 @@ static char ixgb_driver_string[] = Intel(R) PRO/10GbE Network Driver; #else #define DRIVERNAPI -NAPI #endif -#define DRV_VERSION 1.0.117-k2DRIVERNAPI +#define DRV_VERSION 1.0.126-k2DRIVERNAPI char ixgb_driver_version[] = DRV_VERSION; static char ixgb_copyright[] = Copyright (c) 1999-2006 Intel Corporation.; @@ -1287,6 +1287,7 @@ ixgb_tx_map(struct ixgb_adapter *adapter, struct sk_buff *skb, struct ixgb_buffer *buffer_info; int len = skb-len;
RE: [PATCH] ucc_geth: kmalloc casting cleanups
-Original Message- From: Jeff Garzik [mailto:[EMAIL PROTECTED] Sent: Tuesday, January 09, 2007 3:16 PM To: Li Yang-r58472 Cc: netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH] ucc_geth: kmalloc casting cleanups Li Yang wrote: From: Ahmed S. Darwish [EMAIL PROTECTED] Switch kmalloc to kzalloc and clean some redundant kmalloc casts. Signed-off-by: Ahmed S. Darwish [EMAIL PROTECTED] Signed-off-by: Li Yang [EMAIL PROTECTED] --- I should wait for Kumar to resend ucc_geth fixes before applying this, right? Ok. The patch is based on 2.6.20-rc3, but I think it can still apply. If not, please ping me. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2.6.20-rc3] UCC Ether driver: kmalloc casting cleanups
-Original Message- From: Ahmed S. Darwish [mailto:[EMAIL PROTECTED] Sent: Saturday, January 06, 2007 9:19 PM To: Li Yang-r58472 Cc: linux-kernel@vger.kernel.org Subject: [PATCH 2.6.20-rc3] UCC Ether driver: kmalloc casting cleanups [Please inform me if you are not the maintaner cause I'm not sure:)] Hi, A kmalloc casting cleanup patch. I wasn't able to compile the file drivers/net/ucc_geth.c cause of some not found headers (asm/of_platform.h, asm/qe.h, and others ) You need to use ARCH=powerpc to compile this driver. I don't know how you could select this driver without using powerpc arch. Signed-off-by: Ahmed Darwish [EMAIL PROTECTED] diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index 8243150..001109e 100644 --- a/drivers/net/ucc_geth.c +++ b/drivers/net/ucc_geth.c @@ -2864,8 +2864,8 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) if (UCC_GETH_TX_BD_RING_ALIGNMENT 4) align = UCC_GETH_TX_BD_RING_ALIGNMENT; ugeth-tx_bd_ring_offset[j] = - (u32) (kmalloc((u32) (length + align), - GFP_KERNEL)); + kmalloc((u32) (length + align), GFP_KERNEL); + if (ugeth-tx_bd_ring_offset[j] != 0) ugeth-p_tx_bd_ring[j] = (void*)((ugeth-tx_bd_ring_offset[j] + @@ -2900,7 +2900,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) if (UCC_GETH_RX_BD_RING_ALIGNMENT 4) align = UCC_GETH_RX_BD_RING_ALIGNMENT; ugeth-rx_bd_ring_offset[j] = - (u32) (kmalloc((u32) (length + align), GFP_KERNEL)); + kmalloc((u32) (length + align), GFP_KERNEL); NACK about the 2 clean-ups above. Cast from pointer to integer is required here. if (ugeth-rx_bd_ring_offset[j] != 0) ugeth-p_rx_bd_ring[j] = (void*)((ugeth-rx_bd_ring_offset[j] + @@ -2926,10 +2926,9 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) /* Init Tx bds */ for (j = 0; j ug_info-numQueuesTx; j++) { /* Setup the skbuff rings */ - ugeth-tx_skbuff[j] = - (struct sk_buff **)kmalloc(sizeof(struct sk_buff *) * - ugeth-ug_info-bdRingLenTx[j], -GFP_KERNEL); + ugeth-tx_skbuff[j] = kmalloc(sizeof(struct sk_buff *) * + ugeth-ug_info-bdRingLenTx[j], + GFP_KERNEL); if (ugeth-tx_skbuff[j] == NULL) { ugeth_err(%s: Could not allocate tx_skbuff, @@ -2958,10 +2957,9 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) /* Init Rx bds */ for (j = 0; j ug_info-numQueuesRx; j++) { /* Setup the skbuff rings */ - ugeth-rx_skbuff[j] = - (struct sk_buff **)kmalloc(sizeof(struct sk_buff *) * - ugeth-ug_info-bdRingLenRx[j], -GFP_KERNEL); + ugeth-rx_skbuff[j] = kmalloc(sizeof(struct sk_buff *) * + ugeth-ug_info-bdRingLenRx[j], + GFP_KERNEL); if (ugeth-rx_skbuff[j] == NULL) { ugeth_err(%s: Could not allocate rx_skbuff, @@ -3452,8 +3450,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth) * allocated resources can be released when the channel is freed. */ if (!(ugeth-p_init_enet_param_shadow = - (struct ucc_geth_init_pram *) kmalloc(sizeof(struct ucc_geth_init_pram), - GFP_KERNEL))) { + kmalloc(sizeof(struct ucc_geth_init_pram), GFP_KERNEL))) { ugeth_err (%s: Can not allocate memory for p_UccInitEnetParamShadows., __FUNCTION__); -- Ahmed S. Darwish http://darwish-07.blogspot.com - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2.6.20-rc3] UCC Ether driver: kmalloc casting cleanups
-Original Message- From: Ahmed S. Darwish [mailto:[EMAIL PROTECTED] Sent: Monday, January 08, 2007 12:27 PM To: Li Yang-r58472 Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org Subject: Re: [PATCH 2.6.20-rc3] UCC Ether driver: kmalloc casting cleanups On Mon, Jan 08, 2007 at 11:12:28AM +0800, Li Yang-r58472 wrote: From: Ahmed S. Darwish [mailto:[EMAIL PROTECTED] Hi, A kmalloc casting cleanup patch. Signed-off-by: Ahmed Darwish [EMAIL PROTECTED] [..] - (u32) (kmalloc((u32) (length + align), - GFP_KERNEL)); + kmalloc((u32) (length + align), GFP_KERNEL); + if (ugeth-tx_bd_ring_offset[j] != 0) ugeth-p_tx_bd_ring[j] = [..] - (u32) (kmalloc((u32) (length + align), GFP_KERNEL)); + kmalloc((u32) (length + align), GFP_KERNEL); NACK about the 2 clean-ups above. Cast from pointer to integer is required here. Are the casts from pointer to integer just needed to suppress gcc warnings or there's something technically important about them ? It is to suppress the warnings. IMHO, most type casts are not technically important but for sanity check. - Leo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver
-Original Message- From: Kumar Gala [mailto:[EMAIL PROTECTED] Sent: Thursday, July 06, 2006 9:45 PM To: Li Yang-r58472 Cc: Andrew Morton; [EMAIL PROTECTED]; netdev@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 1/3] Freescale QE UCC gigabit ethernet driver Nack. Here are some high level things that should be addressed: * There is a ton of debug code that should probably be stripped out, such as dump_regs, etc.. The reason I left these debug code in is that, QE is very flexible. User probably needs to fine tune parameters for their own application. It will be good to give them some clue doing that. It's fine to remove them, if you do think they are too verbose. * The phy support should use the phylib instead I'll do this when I do get some time. ;) * convert from being a platform_device to of_device I'm not up against moving to use of_device. But why are we putting extra effort in closing the door to backward compatibility with ppc arch, and doesn't provide any new feature. ;) - kumar - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html