From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Bj=C3=B8rn_Mork?= Subject: Re: [PATCH v3 1/4] Simplify usbnet_cdc_update_filter Date: Mon, 02 Jul 2018 19:13:13 +0200 Message-ID: <87h8lhwogm.fsf@miraculix.mork.no> References: <20180701081550.GA7048@kroah.com> <20180701090553.7776-1-miguel@det.uvigo.gal> <20180701090553.7776-2-miguel@det.uvigo.gal> <1530519944.18402.10.camel@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Miguel =?utf-8?Q?Rodr=C3=ADguez_P=C3=A9rez?= , gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, netdev@vger.kernel.org To: Oliver Neukum Return-path: Received: from canardo.mork.no ([148.122.252.1]:60077 "EHLO canardo.mork.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752137AbeGBRN1 (ORCPT ); Mon, 2 Jul 2018 13:13:27 -0400 In-Reply-To: <1530519944.18402.10.camel@suse.com> (Oliver Neukum's message of "Mon, 02 Jul 2018 10:25:44 +0200") Sender: netdev-owner@vger.kernel.org List-ID: Oliver Neukum writes: > On So, 2018-07-01 at 11:05 +0200, Miguel Rodr=C3=ADguez P=C3=A9rez = 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. >>=20 >> Signed-off-by: Miguel Rodr=C3=ADguez P=C3=A9rez > > 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. Right. I knew there had to be some reason this was left out when the rest of these were converted. But I don't think the reason is valid... usbnet_write_cmd() will never call kmemdup when data is NULL. There is of course also little advantage in using usbnet_write_cmd when we don't need to allocate a buffer. But I'd still prefer to see it for consistency, power management and debug logging. Or if it is left as-is: Maybe add a comment so that I don't ask the same stupid questions in 3 weeks time? :-) My memory is los^Husy... > The simplest solution is to leave out this patch in the sequence. As Miguel noted: That won't work. The switch from dev->data->control to dev->intf is necessary to make this function reusable by other minidrivers. Bj=C3=B8rn