| Message ID | 20230112100100.180708-1-bjorn@mork.no (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [net-next] r8152; preserve device list format | expand |
On Thu, Jan 12, 2023 at 11:01:00AM +0100, Bjørn Mork wrote: > This is a partial revert of commit ec51fbd1b8a2 ("r8152: > add USB device driver for config selection") > > Keep a simplified version of the REALTEK_USB_DEVICE macro > to avoid unnecessary reformatting of the device list. This > makes new device ID additions apply cleanly across driver > versions. > > Fixes: ec51fbd1b8a2 ("r8152: add USB device driver for config selection") > Signed-off-by: Bjørn Mork <bjorn@mork.no> > --- > The patch in > https://lore.kernel.org/lkml/20230111133228.190801-1-andre.przywara@arm.com/ > will apply cleanly on top of this. > > This fix will also prevent a lot of stable backporting hassle. No need for this, just backport the original change to older kernels and all will be fine. Don't live with stuff you don't want to because of stable kernels, that's not how this whole process works at all :) thanks, greg k-h
Greg KH <greg@kroah.com> writes: > On Thu, Jan 12, 2023 at 11:01:00AM +0100, Bjørn Mork wrote: >> This is a partial revert of commit ec51fbd1b8a2 ("r8152: >> add USB device driver for config selection") >> >> Keep a simplified version of the REALTEK_USB_DEVICE macro >> to avoid unnecessary reformatting of the device list. This >> makes new device ID additions apply cleanly across driver >> versions. >> >> Fixes: ec51fbd1b8a2 ("r8152: add USB device driver for config selection") >> Signed-off-by: Bjørn Mork <bjorn@mork.no> >> --- >> The patch in >> https://lore.kernel.org/lkml/20230111133228.190801-1-andre.przywara@arm.com/ >> will apply cleanly on top of this. >> >> This fix will also prevent a lot of stable backporting hassle. > > No need for this, just backport the original change to older kernels and > all will be fine. > > Don't live with stuff you don't want to because of stable kernels, > that's not how this whole process works at all :) OK, thanks. Will prepare a patch for stable instead then. But I guess the original patch is unacceptable for stable as-is? It changes how Linux react to these devces, and includes a completely new USB device driver (i.e not interface driver). Bjørn
On Thu, Jan 12, 2023 at 11:18:59AM +0100, Bjørn Mork wrote: > Greg KH <greg@kroah.com> writes: > > On Thu, Jan 12, 2023 at 11:01:00AM +0100, Bjørn Mork wrote: > >> This is a partial revert of commit ec51fbd1b8a2 ("r8152: > >> add USB device driver for config selection") > >> > >> Keep a simplified version of the REALTEK_USB_DEVICE macro > >> to avoid unnecessary reformatting of the device list. This > >> makes new device ID additions apply cleanly across driver > >> versions. > >> > >> Fixes: ec51fbd1b8a2 ("r8152: add USB device driver for config selection") > >> Signed-off-by: Bjørn Mork <bjorn@mork.no> > >> --- > >> The patch in > >> https://lore.kernel.org/lkml/20230111133228.190801-1-andre.przywara@arm.com/ > >> will apply cleanly on top of this. > >> > >> This fix will also prevent a lot of stable backporting hassle. > > > > No need for this, just backport the original change to older kernels and > > all will be fine. > > > > Don't live with stuff you don't want to because of stable kernels, > > that's not how this whole process works at all :) > > OK, thanks. Will prepare a patch for stable instead then. > > But I guess the original patch is unacceptable for stable as-is? It > changes how Linux react to these devces, and includes a completely new > USB device driver (i.e not interface driver). That's up to you all. We don't add new support for new hardware to older kernels _UNLESS_ it's just a new device id. Otherwise it's just better to tell people to upgrade to the newer kernel. If you split things up and added a whole new driver, then just leave it alone, no need to backport anything, sorry, I didn't realize that. greg k-h
Bjørn Mork <bjorn@mork.no> writes: > Greg KH <greg@kroah.com> writes: > >> No need for this, just backport the original change to older kernels and >> all will be fine. >> >> Don't live with stuff you don't want to because of stable kernels, >> that's not how this whole process works at all :) > > OK, thanks. Will prepare a patch for stable instead then. > > But I guess the original patch is unacceptable for stable as-is? It > changes how Linux react to these devces, and includes a completely new > USB device driver (i.e not interface driver). Doh! I gotta start thinking before I send email. Will start right after sending this one ;-) We cannot backport the device-id table change to stable without taking the rest of the patch. The strategy used by the old driver needs two entries per device ID, which is why the macro was there. So the question is: Can commit ec51fbd1b8a2 ("r8152: add USB device driver for config selection") be accepted in stable? ( Direct link for convenience since it's not yet in mainline: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/usb/r8152.c?id=ec51fbd1b8a2bca2948dede99c14ec63dc57ff6b ) This is not within the rules as I read them, but it's your call... Bjørn
On Thu, Jan 12, 2023 at 11:29:40AM +0100, Bjørn Mork wrote: > Bjørn Mork <bjorn@mork.no> writes: > > Greg KH <greg@kroah.com> writes: > > > >> No need for this, just backport the original change to older kernels and > >> all will be fine. > >> > >> Don't live with stuff you don't want to because of stable kernels, > >> that's not how this whole process works at all :) > > > > OK, thanks. Will prepare a patch for stable instead then. > > > > But I guess the original patch is unacceptable for stable as-is? It > > changes how Linux react to these devces, and includes a completely new > > USB device driver (i.e not interface driver). > > Doh! I gotta start thinking before I send email. Will start right > after sending this one ;-) > > We cannot backport the device-id table change to stable without taking > the rest of the patch. The strategy used by the old driver needs two > entries per device ID, which is why the macro was there. > > So the question is: Can commit ec51fbd1b8a2 ("r8152: add USB device > driver for config selection") be accepted in stable? > > ( Direct link for convenience since it's not yet in mainline: > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/usb/r8152.c?id=ec51fbd1b8a2bca2948dede99c14ec63dc57ff6b > ) > > This is not within the rules as I read them, but it's your call... Ah, yeah, that's simple enough, I'd take it if you send it to me :) greg k-h
Greg KH <greg@kroah.com> writes: > On Thu, Jan 12, 2023 at 11:29:40AM +0100, Bjørn Mork wrote: >> Bjørn Mork <bjorn@mork.no> writes: >> > Greg KH <greg@kroah.com> writes: >> > >> >> No need for this, just backport the original change to older kernels and >> >> all will be fine. >> >> >> >> Don't live with stuff you don't want to because of stable kernels, >> >> that's not how this whole process works at all :) >> > >> > OK, thanks. Will prepare a patch for stable instead then. >> > >> > But I guess the original patch is unacceptable for stable as-is? It >> > changes how Linux react to these devces, and includes a completely new >> > USB device driver (i.e not interface driver). >> >> Doh! I gotta start thinking before I send email. Will start right >> after sending this one ;-) >> >> We cannot backport the device-id table change to stable without taking >> the rest of the patch. The strategy used by the old driver needs two >> entries per device ID, which is why the macro was there. >> >> So the question is: Can commit ec51fbd1b8a2 ("r8152: add USB device >> driver for config selection") be accepted in stable? >> >> ( Direct link for convenience since it's not yet in mainline: >> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/usb/r8152.c?id=ec51fbd1b8a2bca2948dede99c14ec63dc57ff6b >> ) >> >> This is not within the rules as I read them, but it's your call... > > Ah, yeah, that's simple enough, I'd take it if you send it to me :) Great! There is no point backporting to anything older than v5.15 since the patch depend on significant driver changes between v5.10 and v5.15. The good news is that those changes also modified the macro in question so any device ID patch for v5.10 or older will have to be fixed up in any case. So we don't lose anything by ignoring the older longterm kernels here. IIUC the special netdev stable rules are gone. But if this is going to stable, then I believe it still has to go to "net" first. David/Jakub - Would you please pick ec51fbd1b8a2 ("r8152: add USB device driver for config selection") 69649ef84053 ("cdc_ether: no need to blacklist any r8152 devices") from net-next to net? With a "CC: stable" preferably. Or do you prefer some other solution? Bjørn
On Fri, 13 Jan 2023 11:16:47 +0100 Bjørn Mork wrote: > There is no point backporting to anything older than v5.15 since the > patch depend on significant driver changes between v5.10 and v5.15. The > good news is that those changes also modified the macro in question so > any device ID patch for v5.10 or older will have to be fixed up in any > case. So we don't lose anything by ignoring the older longterm kernels > here. > > IIUC the special netdev stable rules are gone. But if this is going to > stable, then I believe it still has to go to "net" first. > > David/Jakub - Would you please pick > > ec51fbd1b8a2 ("r8152: add USB device driver for config selection") > 69649ef84053 ("cdc_ether: no need to blacklist any r8152 devices") > > from net-next to net? With a "CC: stable" preferably. Or do you prefer > some other solution? Well.. we already shipped the patch from this thread as is to Linus. Greg will be able to take be53771c87f4 into stable directly, with no dependencies. And now the refactoring won't cherry-pick cleanly :( Maybe let's leave it be? I'll keep in mind that Greg is okay with taking this sort of refactoring in in the future. I made an unnecessary commotion here.
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 66e70b5f8417..3d4631dae00f 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -9817,34 +9817,36 @@ static void rtl8152_disconnect(struct usb_interface *intf) } } +#define REALTEK_USB_DEVICE(vend, prod) { USB_DEVICE(vend, prod) } + /* table of devices that work with this driver */ static const struct usb_device_id rtl8152_table[] = { /* Realtek */ - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8050) }, - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8053) }, - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8152) }, - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8153) }, - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8155) }, - { USB_DEVICE(VENDOR_ID_REALTEK, 0x8156) }, + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8050), + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8053), + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8152), + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8153), + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8155), + REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8156), /* Microsoft */ - { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07ab) }, - { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07c6) }, - { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0927) }, - { USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x304f) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x3054) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x3062) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x3069) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x3082) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x7205) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x720c) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x7214) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0x721e) }, - { USB_DEVICE(VENDOR_ID_LENOVO, 0xa387) }, - { USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041) }, - { USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff) }, - { USB_DEVICE(VENDOR_ID_TPLINK, 0x0601) }, + REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07ab), + REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07c6), + REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0927), + REALTEK_USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3054), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3082), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x721e), + REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0xa387), + REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041), + REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff), + REALTEK_USB_DEVICE(VENDOR_ID_TPLINK, 0x0601), {} };
This is a partial revert of commit ec51fbd1b8a2 ("r8152: add USB device driver for config selection") Keep a simplified version of the REALTEK_USB_DEVICE macro to avoid unnecessary reformatting of the device list. This makes new device ID additions apply cleanly across driver versions. Fixes: ec51fbd1b8a2 ("r8152: add USB device driver for config selection") Signed-off-by: Bjørn Mork <bjorn@mork.no> --- The patch in https://lore.kernel.org/lkml/20230111133228.190801-1-andre.przywara@arm.com/ will apply cleanly on top of this. This fix will also prevent a lot of stable backporting hassle. Sorry for not thinking the change completely through before submitting. I should never have touched the rtl8152_table. Bjørn drivers/net/usb/r8152.c | 48 +++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 23 deletions(-)