| Message ID | 20090221193633.GD16596@n2100.arm.linux.org.uk |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Sat, 21 Feb 2009 19:36:33 +0000 > Further to a71558d, this is round five of fixes to make etherh work > again. As mainline kernels stand, the fixes in b9a9b4b were the wrong > approach. ... > Fix up b9a9b4b by making etherh's net_device_ops refer to the internal > lib8390 functions, and remove the build of the ISA 8390.c driver. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> Applied to net-next-2.6, thanks Russell. You mention this as a fix against mainline, but your patch only applied to net-next-2.6 because the "eth_set_mac_addr" fix to this driver only exists there. Let me know if you want me to propagate both changes to net-2.6 and push to Linus. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 21, 2009 at 11:44:48PM -0800, David Miller wrote: > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > Date: Sat, 21 Feb 2009 19:36:33 +0000 > > > Further to a71558d, this is round five of fixes to make etherh work > > again. As mainline kernels stand, the fixes in b9a9b4b were the wrong > > approach. > ... > > Fix up b9a9b4b by making etherh's net_device_ops refer to the internal > > lib8390 functions, and remove the build of the ISA 8390.c driver. > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > Applied to net-next-2.6, thanks Russell. > > You mention this as a fix against mainline, but your patch > only applied to net-next-2.6 because the "eth_set_mac_addr" > fix to this driver only exists there. Hmm, I don't see the problem. What's currently in mainline is: .ndo_set_mac_address = eth_mac_addr, which is what's in the context lines of this patch. I pushed this myself in a71558d. > Let me know if you want me to propagate both changes to net-2.6 > and push to Linus. Definitely, these patches are fixing regressions, so should be submitted to Linus for 2.6.29. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Sun, 22 Feb 2009 08:19:47 +0000 > On Sat, Feb 21, 2009 at 11:44:48PM -0800, David Miller wrote: > > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > > Date: Sat, 21 Feb 2009 19:36:33 +0000 > > > > > Further to a71558d, this is round five of fixes to make etherh work > > > again. As mainline kernels stand, the fixes in b9a9b4b were the wrong > > > approach. > > ... > > > Fix up b9a9b4b by making etherh's net_device_ops refer to the internal > > > lib8390 functions, and remove the build of the ISA 8390.c driver. > > > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > > Applied to net-next-2.6, thanks Russell. > > > > You mention this as a fix against mainline, but your patch > > only applied to net-next-2.6 because the "eth_set_mac_addr" > > fix to this driver only exists there. > > Hmm, I don't see the problem. What's currently in mainline is: > > .ndo_set_mac_address = eth_mac_addr, Which didn't go in via the net-2.6 tree, sigh... :-/ Russell, pick your transport medium, either send ARM network driver fixes via me or straight to Linus. Not some mixture of both, that's only going to lead to confusion, just like it did here. I put that "eth_mac_addr" fix into net-next-2.6, and you then sent it straight to Linus. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 22, 2009 at 12:24:14AM -0800, David Miller wrote: > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > Date: Sun, 22 Feb 2009 08:19:47 +0000 > > > On Sat, Feb 21, 2009 at 11:44:48PM -0800, David Miller wrote: > > > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > > > Date: Sat, 21 Feb 2009 19:36:33 +0000 > > > > > > > Further to a71558d, this is round five of fixes to make etherh work > > > > again. As mainline kernels stand, the fixes in b9a9b4b were the wrong > > > > approach. > > > ... > > > > Fix up b9a9b4b by making etherh's net_device_ops refer to the internal > > > > lib8390 functions, and remove the build of the ISA 8390.c driver. > > > > > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > > > > > Applied to net-next-2.6, thanks Russell. > > > > > > You mention this as a fix against mainline, but your patch > > > only applied to net-next-2.6 because the "eth_set_mac_addr" > > > fix to this driver only exists there. > > > > Hmm, I don't see the problem. What's currently in mainline is: > > > > .ndo_set_mac_address = eth_mac_addr, > > Which didn't go in via the net-2.6 tree, sigh... :-/ > > Russell, pick your transport medium, either send ARM network driver > fixes via me or straight to Linus. > > Not some mixture of both, that's only going to lead to confusion, > just like it did here. > > I put that "eth_mac_addr" fix into net-next-2.6, and you then sent it > straight to Linus. Hmm, so someone else submitted the same fix for that regression caused by fe96aaa. Given that the eth_mac_addr change is a regression fix, the question has to be asked: why is it queued for the next merge window? In any case, I'm more than willing to push this through the ARM tree, but at the same time I'm aware that people get upset if they're not copied on the patches. That's why I CC'd you with it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Sun, 22 Feb 2009 08:45:58 +0000 > On Sun, Feb 22, 2009 at 12:24:14AM -0800, David Miller wrote: > > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > > Date: Sun, 22 Feb 2009 08:19:47 +0000 > > > > > Hmm, I don't see the problem. What's currently in mainline is: > > > > > > .ndo_set_mac_address = eth_mac_addr, > > > > Which didn't go in via the net-2.6 tree, sigh... :-/ > > > > Russell, pick your transport medium, either send ARM network driver > > fixes via me or straight to Linus. > > > > Not some mixture of both, that's only going to lead to confusion, > > just like it did here. > > > > I put that "eth_mac_addr" fix into net-next-2.6, and you then sent it > > straight to Linus. > > Hmm, so someone else submitted the same fix for that regression caused > by fe96aaa. That someone else was you: commit 5376071069ec8a7e6a8112beab16fc24f5139475 ... Merge master.kernel.org:/home/rmk/linux-2.6-arm * master.kernel.org:/home/rmk/linux-2.6-arm: (22 commits) which brought in: commit a71558d0eca1bbb23737f832297926666f9b36db Author: Russell King <rmk@dyn-67.arm.linux.org.uk> Date: Tue Jan 27 22:32:29 2009 +0000 [ARM] etherh: continue fixing build failure Further to 483a2b3a3182abcb7fcea986d7ea13e793bb00b1, also fix: drivers/net/arm/etherh.c:649: error: 'eth_set_mac_addr' undeclared here (not in a function) Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> This was my entire point. > Given that the eth_mac_addr change is a regression fix, the question has > to be asked: why is it queued for the next merge window? Because I thought the regression only existed in net-next-2.6, probably due to poor communication from the patch submitter :) > In any case, I'm more than willing to push this through the ARM tree, but > at the same time I'm aware that people get upset if they're not copied on > the patches. That's why I CC'd you with it. All you need to do is explicitly tell me where a bug fix goes, and I can get it into Linus's tree in less than a day. It's all about communication and not doing things like submitting changes behind my back after I've explicitly replied with an email saying "Applied" to your patch. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 22, 2009 at 02:39:33AM -0800, David Miller wrote: > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > Date: Sun, 22 Feb 2009 08:45:58 +0000 > > > On Sun, Feb 22, 2009 at 12:24:14AM -0800, David Miller wrote: > > > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > > > Date: Sun, 22 Feb 2009 08:19:47 +0000 > > > > > > > Hmm, I don't see the problem. What's currently in mainline is: > > > > > > > > .ndo_set_mac_address = eth_mac_addr, > > > > > > Which didn't go in via the net-2.6 tree, sigh... :-/ > > > > > > Russell, pick your transport medium, either send ARM network driver > > > fixes via me or straight to Linus. > > > > > > Not some mixture of both, that's only going to lead to confusion, > > > just like it did here. > > > > > > I put that "eth_mac_addr" fix into net-next-2.6, and you then sent it > > > straight to Linus. > > > > Hmm, so someone else submitted the same fix for that regression caused > > by fe96aaa. > > That someone else was you: > > commit 5376071069ec8a7e6a8112beab16fc24f5139475 > ... > Merge master.kernel.org:/home/rmk/linux-2.6-arm > > * master.kernel.org:/home/rmk/linux-2.6-arm: (22 commits) > > which brought in: > > commit a71558d0eca1bbb23737f832297926666f9b36db > Author: Russell King <rmk@dyn-67.arm.linux.org.uk> > Date: Tue Jan 27 22:32:29 2009 +0000 > > [ARM] etherh: continue fixing build failure > > Further to 483a2b3a3182abcb7fcea986d7ea13e793bb00b1, also fix: > > drivers/net/arm/etherh.c:649: error: 'eth_set_mac_addr' undeclared here (not in a function) > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > This was my entire point. Yes, I know I put that into mainline via my tree... > > Given that the eth_mac_addr change is a regression fix, the question has > > to be asked: why is it queued for the next merge window? > > Because I thought the regression only existed in net-next-2.6, > probably due to poor communication from the patch submitter :) > > > In any case, I'm more than willing to push this through the ARM tree, but > > at the same time I'm aware that people get upset if they're not copied on > > the patches. That's why I CC'd you with it. > > All you need to do is explicitly tell me where a bug fix goes, > and I can get it into Linus's tree in less than a day. > > It's all about communication and not doing things like submitting > changes behind my back after I've explicitly replied with an > email saying "Applied" to your patch. ... but I must have forgotten that I'd already sent it to you. I don't seem to have a record of sending it. What I do have is that I raised the issue of the ndo_set_mac_addr in January on netdev. To that I got a reply from Stephen about it, to which I asked whether he wanted me to fix it via my tree. The response I got in private was "yes" (and I won't reveal the rest of it because it refers to his travel around that time.) In reply to that I sent Stephen a patch, which being a reply to a private message couldn't be CC'd back to netdev. However, coincidentally, it seems that you fixed the precise error I raised (thanks) but missed the eth_set_mac_addr mis-spelling, so I dropped my original patch. I didn't get any further response from on the issue, so I'd assumed two weeks later that it was dead and fixed the remaining issue. So, as far as I can see, I never sent you a patch for to fix the eth_set_mac_addr -> eth_mac_addr change. If you have that in your tree, someone else must have sent it to you (maybe Stephen forwarded it?) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Sun, 22 Feb 2009 11:46:01 +0000 > So, as far as I can see, I never sent you a patch for to fix the > eth_set_mac_addr -> eth_mac_addr change. If you have that in your > tree, someone else must have sent it to you (maybe Stephen forwarded > it?) Nevermind, I see what happened, I got your commit when I merged upstream into net-next-2.6 to resolve some merge conflicts. Ok, I'll revert today's patch from my tree and let it propagate via your's. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 22, 2009 at 04:29:52AM -0800, David Miller wrote: > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > Date: Sun, 22 Feb 2009 11:46:01 +0000 > > > So, as far as I can see, I never sent you a patch for to fix the > > eth_set_mac_addr -> eth_mac_addr change. If you have that in your > > tree, someone else must have sent it to you (maybe Stephen forwarded > > it?) > > Nevermind, I see what happened, I got your commit when I merged > upstream into net-next-2.6 to resolve some merge conflicts. > > Ok, I'll revert today's patch from my tree and let it propagate via > your's. I don't mind how it gets in, as long as it does. Since you seem happy for it to propagate via mine, can I assume you're acking it? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Russell King - ARM Linux <linux@arm.linux.org.uk> Date: Sun, 22 Feb 2009 12:34:58 +0000 > On Sun, Feb 22, 2009 at 04:29:52AM -0800, David Miller wrote: > > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > > Date: Sun, 22 Feb 2009 11:46:01 +0000 > > > > > So, as far as I can see, I never sent you a patch for to fix the > > > eth_set_mac_addr -> eth_mac_addr change. If you have that in your > > > tree, someone else must have sent it to you (maybe Stephen forwarded > > > it?) > > > > Nevermind, I see what happened, I got your commit when I merged > > upstream into net-next-2.6 to resolve some merge conflicts. > > > > Ok, I'll revert today's patch from my tree and let it propagate via > > your's. > > I don't mind how it gets in, as long as it does. Since you seem happy > for it to propagate via mine, can I assume you're acking it? Yep Acked-by: David S. Miller <davem@davemloft.net> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 22, 2009 at 04:36:47AM -0800, David Miller wrote: > From: Russell King - ARM Linux <linux@arm.linux.org.uk> > Date: Sun, 22 Feb 2009 12:34:58 +0000 > > I don't mind how it gets in, as long as it does. Since you seem happy > > for it to propagate via mine, can I assume you're acking it? > > Yep > > Acked-by: David S. Miller <davem@davemloft.net> Thanks, applied. Should hit Linus' tree around Thursday. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/arm/Makefile b/drivers/net/arm/Makefile index c69c0cd..811a3cc 100644 --- a/drivers/net/arm/Makefile +++ b/drivers/net/arm/Makefile @@ -4,7 +4,7 @@ # obj-$(CONFIG_ARM_AM79C961A) += am79c961a.o -obj-$(CONFIG_ARM_ETHERH) += etherh.o ../8390.o +obj-$(CONFIG_ARM_ETHERH) += etherh.o obj-$(CONFIG_ARM_ETHER3) += ether3.o obj-$(CONFIG_ARM_ETHER1) += ether1.o obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o diff --git a/drivers/net/arm/etherh.c b/drivers/net/arm/etherh.c index 54b52e5..f52f668 100644 --- a/drivers/net/arm/etherh.c +++ b/drivers/net/arm/etherh.c @@ -641,15 +641,15 @@ static const struct net_device_ops etherh_netdev_ops = { .ndo_open = etherh_open, .ndo_stop = etherh_close, .ndo_set_config = etherh_set_config, - .ndo_start_xmit = ei_start_xmit, - .ndo_tx_timeout = ei_tx_timeout, - .ndo_get_stats = ei_get_stats, - .ndo_set_multicast_list = ei_set_multicast_list, + .ndo_start_xmit = __ei_start_xmit, + .ndo_tx_timeout = __ei_tx_timeout, + .ndo_get_stats = __ei_get_stats, + .ndo_set_multicast_list = __ei_set_multicast_list, .ndo_validate_addr = eth_validate_addr, .ndo_set_mac_address = eth_mac_addr, .ndo_change_mtu = eth_change_mtu, #ifdef CONFIG_NET_POLL_CONTROLLER - .ndo_poll_controller = ei_poll, + .ndo_poll_controller = __ei_poll, #endif };
Further to a71558d, this is round five of fixes to make etherh work again. As mainline kernels stand, the fixes in b9a9b4b were the wrong approach. The 8390 driver was structured by Al Viro to allow the flexibility required by platforms. lib8390.c contains the core code which drivers explicitly include: - 8390.c includes lib8390.c to provide the standard ISA based driver. - etherh.c includes it with the accessors defined for RiscPC platforms, where it is addressed via the MMIO accessors with a device dependent register spacing. Other platform drivers do something similar. However, b9a9b4b caused the kernel to contain not only the etherh private build of lib8390 (included in etherh.c) but also lib8390.c itself, and referred the new net_device_ops methods to the ISA version. The result of this is is not pretty: Unable to handle kernel paging request at virtual address 12032030 pgd = c8330000 [12032030] *pgd=00000000 Internal error: Oops: 18331805 [#1] Modules linked in: ipv6 CPU: 0 Not tainted (2.6.29-rc3 #167) PC is at do_set_multicast_list+0xd0/0x190 LR is at bitrev32+0x28/0x34 pc : [<c017aab4>] lr : [<c0139120>] psr: a0000093 sp : c8321d9c ip : c8321d84 fp : c8321dbc r10: c80c6800 r9 : 00000000 r8 : c80c6b60 r7 : c80c6b80 r6 : cc80c800 r5 : c80c6800 r4 : 00000000 r3 : cc80c80c r2 : 00000004 r1 : 00000007 r0 : e0000000 Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user ... Fix up b9a9b4b by making etherh's net_device_ops refer to the internal lib8390 functions, and remove the build of the ISA 8390.c driver. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html