From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Neukum Subject: Re: [PATCH v3 1/4] Simplify usbnet_cdc_update_filter Date: Mon, 02 Jul 2018 10:25:44 +0200 Message-ID: <1530519944.18402.10.camel@suse.com> References: <20180701081550.GA7048@kroah.com> <20180701090553.7776-1-miguel@det.uvigo.gal> <20180701090553.7776-2-miguel@det.uvigo.gal> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit To: Miguel =?ISO-8859-1?Q?Rodr=EDguez_P=E9rez?= , gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, netdev@vger.kernel.org Return-path: Received: from mx2.suse.de ([195.135.220.15]:45022 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754102AbeGBIZ7 (ORCPT ); Mon, 2 Jul 2018 04:25:59 -0400 In-Reply-To: <20180701090553.7776-2-miguel@det.uvigo.gal> Sender: netdev-owner@vger.kernel.org List-ID: On So, 2018-07-01 at 11:05 +0200, Miguel Rodríguez Pérez wrote: > Remove some unneded varibles to make the code easier to read > and, replace the generic usb_control_msg function for the > more specific usbnet_write_cmd. > > Signed-off-by: Miguel Rodríguez Pérez No, sorry, but this is not good. The reason is a bit subtle. Drivers need to reset the filters when handling post_reset() [ and reset_resume() ] usbnet_write_cmd() falls back to kmemdup() with GFP_KERNEL. Usbnet is a framework with class drivers and some of the devices we drive have a storage interface. Thence we are on the block error handling path here. The simplest solution is to leave out this patch in the sequence. Regards Oliver NACKED-BY: Oliver Neukum > --- > drivers/net/usb/cdc_ether.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c > index 178b956501a7..815ed0dc18fe 100644 > --- a/drivers/net/usb/cdc_ether.c > +++ b/drivers/net/usb/cdc_ether.c > @@ -77,9 +77,7 @@ static const u8 mbm_guid[16] = { > > static void usbnet_cdc_update_filter(struct usbnet *dev) > { > - struct cdc_state *info = (void *) &dev->data; > - struct usb_interface *intf = info->control; > - struct net_device *net = dev->net; > + struct net_device *net = dev->net; > > u16 cdc_filter = USB_CDC_PACKET_TYPE_DIRECTED > | USB_CDC_PACKET_TYPE_BROADCAST; > @@ -93,16 +91,13 @@ static void usbnet_cdc_update_filter(struct usbnet *dev) > if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI)) > cdc_filter |= USB_CDC_PACKET_TYPE_ALL_MULTICAST; > > - usb_control_msg(dev->udev, > - usb_sndctrlpipe(dev->udev, 0), > + usbnet_write_cmd(dev, > USB_CDC_SET_ETHERNET_PACKET_FILTER, > - USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE, > cdc_filter, > - intf->cur_altsetting->desc.bInterfaceNumber, > + dev->intf->cur_altsetting->desc.bInterfaceNumber, > NULL, > - 0, > - USB_CTRL_SET_TIMEOUT > - ); > + 0); > } > > /* probes control interface, claims data interface, collects the bulk