* drivers/net/usb/asix: bug in asix_get_wol
@ 2011-12-11 1:02 Eugene
2011-12-11 23:29 ` Grant Grundler
0 siblings, 1 reply; 8+ messages in thread
From: Eugene @ 2011-12-11 1:02 UTC (permalink / raw)
To: netdev, grundler
Dear kernel devs,
Thanks for the commit at
http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378,
It successfully stops my adapter from dying when wake-on-lan gets
enabled. However, I've noticed that it has broken asix_get_wol - the
lines
if (opt & AX_MONITOR_LINK)
wolinfo->wolopts |= WAKE_PHY;
if (opt & AX_MONITOR_MAGIC)
wolinfo->wolopts |= WAKE_MAGIC;
have been accidentally removed. The vendor driver has them, and I've
successfully tested a kernel with these lines included. The change is
too small for me to bother sending in a properly formatted patch...
Cheers,
Eugene
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: drivers/net/usb/asix: bug in asix_get_wol 2011-12-11 1:02 drivers/net/usb/asix: bug in asix_get_wol Eugene @ 2011-12-11 23:29 ` Grant Grundler 2011-12-13 13:03 ` Eugene 0 siblings, 1 reply; 8+ messages in thread From: Grant Grundler @ 2011-12-11 23:29 UTC (permalink / raw) To: Eugene; +Cc: netdev, Freddy Xin, Allan Chou [+freddy/allan @ ASIX] On Sat, Dec 10, 2011 at 5:02 PM, Eugene <elubarsky@gmail.com> wrote: > Dear kernel devs, > > Thanks for the commit at > http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378, > It successfully stops my adapter from dying when wake-on-lan gets > enabled. Hi Eugene! thanks for the "it works!" report. > However, I've noticed that it has broken asix_get_wol - the > lines > > if (opt & AX_MONITOR_LINK) > wolinfo->wolopts |= WAKE_PHY; > if (opt & AX_MONITOR_MAGIC) > wolinfo->wolopts |= WAKE_MAGIC; > > have been accidentally removed. This wasn't by accident. This comment in the commit log perhaps doesn't explain sufficiently: | Remove MONITOR_MODE. In this mode, Received packets are not buffered when | the remote wakeup is enabled. > The vendor driver has them, and I've > successfully tested a kernel with these lines included. The change is > too small for me to bother sending in a properly formatted patch... "Too small"? No such thing. :) cheers, grant ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/net/usb/asix: bug in asix_get_wol 2011-12-11 23:29 ` Grant Grundler @ 2011-12-13 13:03 ` Eugene 2011-12-15 16:48 ` Grant Grundler 0 siblings, 1 reply; 8+ messages in thread From: Eugene @ 2011-12-13 13:03 UTC (permalink / raw) To: Grant Grundler; +Cc: netdev, Freddy Xin, Allan Chou Hi Grant, The problem is that, as it's currently written, asix_get_wol always returns that wake-on-lan is disabled. Cheers, Eugene On 12 December 2011 10:29, Grant Grundler <grundler@chromium.org> wrote: > [+freddy/allan @ ASIX] > > On Sat, Dec 10, 2011 at 5:02 PM, Eugene <elubarsky@gmail.com> wrote: >> Dear kernel devs, >> >> Thanks for the commit at >> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378, >> It successfully stops my adapter from dying when wake-on-lan gets >> enabled. > > Hi Eugene! > thanks for the "it works!" report. > >> However, I've noticed that it has broken asix_get_wol - the >> lines >> >> if (opt & AX_MONITOR_LINK) >> wolinfo->wolopts |= WAKE_PHY; >> if (opt & AX_MONITOR_MAGIC) >> wolinfo->wolopts |= WAKE_MAGIC; >> >> have been accidentally removed. > > This wasn't by accident. This comment in the commit log perhaps > doesn't explain sufficiently: > | Remove MONITOR_MODE. In this mode, Received packets are not buffered when > | the remote wakeup is enabled. > >> The vendor driver has them, and I've >> successfully tested a kernel with these lines included. The change is >> too small for me to bother sending in a properly formatted patch... > > "Too small"? No such thing. :) > > cheers, > grant ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/net/usb/asix: bug in asix_get_wol 2011-12-13 13:03 ` Eugene @ 2011-12-15 16:48 ` Grant Grundler 2011-12-16 3:37 ` ASIX Allan Email [office] 2011-12-16 5:15 ` ASIX Allan Email [office] 0 siblings, 2 replies; 8+ messages in thread From: Grant Grundler @ 2011-12-15 16:48 UTC (permalink / raw) To: Eugene, Allan Chou; +Cc: netdev, Freddy Xin On Tue, Dec 13, 2011 at 5:03 AM, Eugene <elubarsky@gmail.com> wrote: > Hi Grant, > > > The problem is that, as it's currently written, asix_get_wol always > returns that wake-on-lan is disabled. I think that was the intent. Allan, can you please confirm? thanks, grant > > > Cheers, > Eugene > > On 12 December 2011 10:29, Grant Grundler <grundler@chromium.org> wrote: >> [+freddy/allan @ ASIX] >> >> On Sat, Dec 10, 2011 at 5:02 PM, Eugene <elubarsky@gmail.com> wrote: >>> Dear kernel devs, >>> >>> Thanks for the commit at >>> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378, >>> It successfully stops my adapter from dying when wake-on-lan gets >>> enabled. >> >> Hi Eugene! >> thanks for the "it works!" report. >> >>> However, I've noticed that it has broken asix_get_wol - the >>> lines >>> >>> if (opt & AX_MONITOR_LINK) >>> wolinfo->wolopts |= WAKE_PHY; >>> if (opt & AX_MONITOR_MAGIC) >>> wolinfo->wolopts |= WAKE_MAGIC; >>> >>> have been accidentally removed. >> >> This wasn't by accident. This comment in the commit log perhaps >> doesn't explain sufficiently: >> | Remove MONITOR_MODE. In this mode, Received packets are not buffered when >> | the remote wakeup is enabled. >> >>> The vendor driver has them, and I've >>> successfully tested a kernel with these lines included. The change is >>> too small for me to bother sending in a properly formatted patch... >> >> "Too small"? No such thing. :) >> >> cheers, >> grant ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: drivers/net/usb/asix: bug in asix_get_wol 2011-12-15 16:48 ` Grant Grundler @ 2011-12-16 3:37 ` ASIX Allan Email [office] 2011-12-16 5:15 ` ASIX Allan Email [office] 1 sibling, 0 replies; 8+ messages in thread From: ASIX Allan Email [office] @ 2011-12-16 3:37 UTC (permalink / raw) To: 'Grant Grundler', 'Eugene' Cc: netdev, 'Freddy Xin', ASIX Louis [蘇威陸] [-- Attachment #1: Type: text/plain, Size: 2784 bytes --] Dear Grant and Eugene, Please refer to the attached file and below statements to modify the asix_get_wol() routine and let us know if this suggestion can solve your issues or not? Thanks a lot. ================ static void asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo) { struct usbnet *dev = netdev_priv(net); u8 opt; if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) { wolinfo->supported = 0; wolinfo->wolopts = 0; return; } wolinfo->supported = WAKE_PHY | WAKE_MAGIC; wolinfo->wolopts = 0; if (opt & AX_MONITOR_LINK) wolinfo->wolopts |= WAKE_PHY; if (opt & AX_MONITOR_MAGIC) wolinfo->wolopts |= WAKE_MAGIC; } --- Best regards, Allan Chou Technical Support Division ASIX Electronics Corporation TEL: 886-3-5799500 ext.228 FAX: 886-3-5799558 E-mail: allan@asix.com.tw http://www.asix.com.tw/ -----Original Message----- From: grundler@google.com [mailto:grundler@google.com] On Behalf Of Grant Grundler Sent: Friday, December 16, 2011 12:48 AM To: Eugene; Allan Chou Cc: netdev@vger.kernel.org; Freddy Xin Subject: Re: drivers/net/usb/asix: bug in asix_get_wol On Tue, Dec 13, 2011 at 5:03 AM, Eugene <elubarsky@gmail.com> wrote: > Hi Grant, > > > The problem is that, as it's currently written, asix_get_wol always > returns that wake-on-lan is disabled. I think that was the intent. Allan, can you please confirm? thanks, grant > > > Cheers, > Eugene > > On 12 December 2011 10:29, Grant Grundler <grundler@chromium.org> wrote: >> [+freddy/allan @ ASIX] >> >> On Sat, Dec 10, 2011 at 5:02 PM, Eugene <elubarsky@gmail.com> wrote: >>> Dear kernel devs, >>> >>> Thanks for the commit at >>> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378, >>> It successfully stops my adapter from dying when wake-on-lan gets >>> enabled. >> >> Hi Eugene! >> thanks for the "it works!" report. >> >>> However, I've noticed that it has broken asix_get_wol - the >>> lines >>> >>> if (opt & AX_MONITOR_LINK) >>> wolinfo->wolopts |= WAKE_PHY; >>> if (opt & AX_MONITOR_MAGIC) >>> wolinfo->wolopts |= WAKE_MAGIC; >>> >>> have been accidentally removed. >> >> This wasn't by accident. This comment in the commit log perhaps >> doesn't explain sufficiently: >> | Remove MONITOR_MODE. In this mode, Received packets are not buffered when >> | the remote wakeup is enabled. >> >>> The vendor driver has them, and I've >>> successfully tested a kernel with these lines included. The change is >>> too small for me to bother sending in a properly formatted patch... >> >> "Too small"? No such thing. :) >> >> cheers, >> grant [-- Attachment #2: asix_driver_from_Google_Submit_20111216.tar.gz --] [-- Type: application/octet-stream, Size: 253437 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: drivers/net/usb/asix: bug in asix_get_wol 2011-12-15 16:48 ` Grant Grundler 2011-12-16 3:37 ` ASIX Allan Email [office] @ 2011-12-16 5:15 ` ASIX Allan Email [office] 2011-12-16 22:15 ` Grant Grundler 1 sibling, 1 reply; 8+ messages in thread From: ASIX Allan Email [office] @ 2011-12-16 5:15 UTC (permalink / raw) To: 'Grant Grundler', 'Eugene' Cc: netdev, 'Freddy Xin', ASIX Louis [蘇威陸] Resend without attachment due to below email server error. ======== ----- The following addresses had permanent fatal errors ----- <grundler@chromium.org> (reason: 552-5.7.0 Our system detected an illegal attachment on your message. Please) <elubarsky@gmail.com> (reason: 552-5.7.0 Our system detected an illegal attachment on your message. Please) --- Best regards, Allan Chou Technical Support Division ASIX Electronics Corporation TEL: 886-3-5799500 ext.228 FAX: 886-3-5799558 E-mail: allan@asix.com.tw http://www.asix.com.tw/ -----Original Message----- From: ASIX Allan Email [office] [mailto:allan@asix.com.tw] Sent: Friday, December 16, 2011 11:38 AM To: 'Grant Grundler'; 'Eugene' Cc: 'netdev@vger.kernel.org'; 'Freddy Xin'; ASIX Louis [蘇威陸] Subject: RE: drivers/net/usb/asix: bug in asix_get_wol Importance: High Dear Grant and Eugene, Please refer to the attached file and below statements to modify the asix_get_wol() routine and let us know if this suggestion can solve your issues or not? Thanks a lot. ================ static void asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo) { struct usbnet *dev = netdev_priv(net); u8 opt; if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) { wolinfo->supported = 0; wolinfo->wolopts = 0; return; } wolinfo->supported = WAKE_PHY | WAKE_MAGIC; wolinfo->wolopts = 0; if (opt & AX_MONITOR_LINK) wolinfo->wolopts |= WAKE_PHY; if (opt & AX_MONITOR_MAGIC) wolinfo->wolopts |= WAKE_MAGIC; } --- Best regards, Allan Chou Technical Support Division ASIX Electronics Corporation TEL: 886-3-5799500 ext.228 FAX: 886-3-5799558 E-mail: allan@asix.com.tw http://www.asix.com.tw/ -----Original Message----- From: grundler@google.com [mailto:grundler@google.com] On Behalf Of Grant Grundler Sent: Friday, December 16, 2011 12:48 AM To: Eugene; Allan Chou Cc: netdev@vger.kernel.org; Freddy Xin Subject: Re: drivers/net/usb/asix: bug in asix_get_wol On Tue, Dec 13, 2011 at 5:03 AM, Eugene <elubarsky@gmail.com> wrote: > Hi Grant, > > > The problem is that, as it's currently written, asix_get_wol always > returns that wake-on-lan is disabled. I think that was the intent. Allan, can you please confirm? thanks, grant > > > Cheers, > Eugene > > On 12 December 2011 10:29, Grant Grundler <grundler@chromium.org> wrote: >> [+freddy/allan @ ASIX] >> >> On Sat, Dec 10, 2011 at 5:02 PM, Eugene <elubarsky@gmail.com> wrote: >>> Dear kernel devs, >>> >>> Thanks for the commit at >>> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=4ad1438f025ed8d1e4e95a796ca7f0ad5a22c378, >>> It successfully stops my adapter from dying when wake-on-lan gets >>> enabled. >> >> Hi Eugene! >> thanks for the "it works!" report. >> >>> However, I've noticed that it has broken asix_get_wol - the >>> lines >>> >>> if (opt & AX_MONITOR_LINK) >>> wolinfo->wolopts |= WAKE_PHY; >>> if (opt & AX_MONITOR_MAGIC) >>> wolinfo->wolopts |= WAKE_MAGIC; >>> >>> have been accidentally removed. >> >> This wasn't by accident. This comment in the commit log perhaps >> doesn't explain sufficiently: >> | Remove MONITOR_MODE. In this mode, Received packets are not buffered when >> | the remote wakeup is enabled. >> >>> The vendor driver has them, and I've >>> successfully tested a kernel with these lines included. The change is >>> too small for me to bother sending in a properly formatted patch... >> >> "Too small"? No such thing. :) >> >> cheers, >> grant ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/net/usb/asix: bug in asix_get_wol 2011-12-16 5:15 ` ASIX Allan Email [office] @ 2011-12-16 22:15 ` Grant Grundler 2011-12-17 2:35 ` allan 0 siblings, 1 reply; 8+ messages in thread From: Grant Grundler @ 2011-12-16 22:15 UTC (permalink / raw) To: ASIX Allan Email [office] Cc: Eugene, netdev, Freddy Xin, ASIX Louis [蘇威陸] On Thu, Dec 15, 2011 at 9:15 PM, ASIX Allan Email [office] <allan@asix.com.tw> wrote: > Resend without attachment due to below email server error. ... > -----Original Message----- > From: ASIX Allan Email [office] [mailto:allan@asix.com.tw] > Sent: Friday, December 16, 2011 11:38 AM > To: 'Grant Grundler'; 'Eugene' > Cc: 'netdev@vger.kernel.org'; 'Freddy Xin'; ASIX Louis [蘇威陸] > Subject: RE: drivers/net/usb/asix: bug in asix_get_wol > Importance: High > > Dear Grant and Eugene, > > Please refer to the attached file and below statements to modify the > asix_get_wol() routine and let us know if this suggestion can solve > your issues or not? Thanks a lot. Allan, Thanks for the response but it doesn't answer my previous question. Let me ask the same question differently. Does WOL support in asix driver need more than the four lines of code? Ie does MONITOR_MODE need to be enabled or anything like that? > ================ > static void > asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo) > { > struct usbnet *dev = netdev_priv(net); > u8 opt; > > if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) { > wolinfo->supported = 0; > wolinfo->wolopts = 0; > return; > } > wolinfo->supported = WAKE_PHY | WAKE_MAGIC; > wolinfo->wolopts = 0; > if (opt & AX_MONITOR_LINK) > wolinfo->wolopts |= WAKE_PHY; > if (opt & AX_MONITOR_MAGIC) > wolinfo->wolopts |= WAKE_MAGIC; > } This looks remarkably similar to the code Eugene said enables WOL for him (and it works). I not able to find any difference. If you believe it was a mistake to remove these four lines of code, please submit a patch (See Documentation/SubmittingPatches) to add them back. You can add a "Tested-by: Eugene <elubarsky@gmail.com>" line after your own "Signed-off-by:" in the patch. thanks! grant ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: drivers/net/usb/asix: bug in asix_get_wol 2011-12-16 22:15 ` Grant Grundler @ 2011-12-17 2:35 ` allan 0 siblings, 0 replies; 8+ messages in thread From: allan @ 2011-12-17 2:35 UTC (permalink / raw) To: 'Grant Grundler' Cc: 'Eugene', netdev, 'Freddy Xin', 'ASIX Louis [蘇威陸]' Dear Grant, I will double check this issue and submit the revised patch to Linux kernel mainline source next week. Thanks a lot for your great helps. --- Best regards, Allan Chou Technical Support Division ASIX Electronics Corporation TEL: 886-3-5799500 ext.228 FAX: 886-3-5799558 E-mail: allan@asix.com.tw http://www.asix.com.tw/ -----Original Message----- From: grundler@google.com [mailto:grundler@google.com] On Behalf Of Grant Grundler Sent: Saturday, December 17, 2011 6:16 AM To: ASIX Allan Email [office] Cc: Eugene; netdev@vger.kernel.org; Freddy Xin; ASIX Louis [蘇威陸] Subject: Re: drivers/net/usb/asix: bug in asix_get_wol On Thu, Dec 15, 2011 at 9:15 PM, ASIX Allan Email [office] <allan@asix.com.tw> wrote: > Resend without attachment due to below email server error. ... > -----Original Message----- > From: ASIX Allan Email [office] [mailto:allan@asix.com.tw] > Sent: Friday, December 16, 2011 11:38 AM > To: 'Grant Grundler'; 'Eugene' > Cc: 'netdev@vger.kernel.org'; 'Freddy Xin'; ASIX Louis [蘇威陸] > Subject: RE: drivers/net/usb/asix: bug in asix_get_wol > Importance: High > > Dear Grant and Eugene, > > Please refer to the attached file and below statements to modify the > asix_get_wol() routine and let us know if this suggestion can solve > your issues or not? Thanks a lot. Allan, Thanks for the response but it doesn't answer my previous question. Let me ask the same question differently. Does WOL support in asix driver need more than the four lines of code? Ie does MONITOR_MODE need to be enabled or anything like that? > ================ > static void > asix_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo) > { > struct usbnet *dev = netdev_priv(net); > u8 opt; > > if (asix_read_cmd(dev, AX_CMD_READ_MONITOR_MODE, 0, 0, 1, &opt) < 0) { > wolinfo->supported = 0; > wolinfo->wolopts = 0; > return; > } > wolinfo->supported = WAKE_PHY | WAKE_MAGIC; > wolinfo->wolopts = 0; > if (opt & AX_MONITOR_LINK) > wolinfo->wolopts |= WAKE_PHY; > if (opt & AX_MONITOR_MAGIC) > wolinfo->wolopts |= WAKE_MAGIC; > } This looks remarkably similar to the code Eugene said enables WOL for him (and it works). I not able to find any difference. If you believe it was a mistake to remove these four lines of code, please submit a patch (See Documentation/SubmittingPatches) to add them back. You can add a "Tested-by: Eugene <elubarsky@gmail.com>" line after your own "Signed-off-by:" in the patch. thanks! grant ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-12-17 2:36 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-11 1:02 drivers/net/usb/asix: bug in asix_get_wol Eugene 2011-12-11 23:29 ` Grant Grundler 2011-12-13 13:03 ` Eugene 2011-12-15 16:48 ` Grant Grundler 2011-12-16 3:37 ` ASIX Allan Email [office] 2011-12-16 5:15 ` ASIX Allan Email [office] 2011-12-16 22:15 ` Grant Grundler 2011-12-17 2:35 ` allan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).