* [PATCH v2 0/2] serial: 8250: Correct basic issues with the PCI blacklist @ 2022-02-12 17:30 Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 1/2] serial: 8250: Correct Kconfig help text for blacklisted PCI devices Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 2/2] serial: 8250: Report which option to enable " Maciej W. Rozycki 0 siblings, 2 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2022-02-12 17:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Andy Shevchenko, linux-serial, linux-kernel Hi, This v2 of the patch series adds missing filler member initialisers for blacklist entries using the PCI_DEVICE rather than PCI_VDEVICE macro that cause "initializer element is not computable at load time" compilation errors in some configurations, which have escaped my verification. An upside is I have noticed PARPORT_SERIAL (or indeed PARPORT_PC) is not selectable with numerous PCI platforms like RISC-V meaning that PC-style PCI parallel port hardware cannot be used with them even though there's no reason for that. The only PCI platforms that actually cannot make use of such hardware are those newer PCIe systems that have no support for I/O cycles in the host bridge with the only actual specimen known to me being the POWER9 PHB4 device, so we ought to enable PARPORT_PC/PARPORT_SERIAL support for PCI systems in the general case. I'll post a fix separately. The original cover letter continues. In the course of investigating whether support code for OxSemi PCIe UARTs could be factored out from the common 8250 PCI UART driver, which has been previously requested by Andy (cc-ed), I have noticed that the Kconfig help text for several device-specific UART drivers previously factored out is incorrect in that it claims that those dedicated drivers are required for extra features of the respective devices, while actually the blacklist entries within the common driver make them require those dedicated drivers even for standard features, as the common driver now refuses to handle them. Also it may be unclear for the user from a specific PCI device ID of an affected PCI UART device which dedicated driver has to be configured in to handle it, so make the blacklist entries include that information to be printed if a device is encountered that cannot be handled because its dedicated driver has been excluded from configuration while the common driver refuses to handle it. See the respective change descriptions for further details. Please apply. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] serial: 8250: Correct Kconfig help text for blacklisted PCI devices 2022-02-12 17:30 [PATCH v2 0/2] serial: 8250: Correct basic issues with the PCI blacklist Maciej W. Rozycki @ 2022-02-12 17:30 ` Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 2/2] serial: 8250: Report which option to enable " Maciej W. Rozycki 1 sibling, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2022-02-12 17:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Andy Shevchenko, linux-serial, linux-kernel Correct the Kconfig help text for SERIAL_8250_LPSS, SERIAL_8250_MID and SERIAL_8250_PERICOM configuration options for dedicated PCI UART drivers that have been blacklisted in the generic PCI 8250 UART driver and as from commit a13e19cf3dc10 ("serial: 8250_lpss: split LPSS driver to separate module"), commit d9eda9bab2372 ("serial: 8250_pci: Intel MID UART support to its own driver"), and commit fcfd3c09f4078 ("serial: 8250_pci: Split out Pericom driver") respectively are not handled by said driver anymore (rather than for extra features only, as the current text indicates), and therefore require the respective dedicated drivers to work at all. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> --- No changes from v1. --- drivers/tty/serial/8250/Kconfig | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) linux-serial-8250-pci-blacklist-help.diff Index: linux-macro/drivers/tty/serial/8250/Kconfig =================================================================== --- linux-macro.orig/drivers/tty/serial/8250/Kconfig +++ linux-macro/drivers/tty/serial/8250/Kconfig @@ -479,11 +479,12 @@ config SERIAL_8250_LPSS select DW_DMAC_PCI if (SERIAL_8250_DMA && X86_INTEL_LPSS) select RATIONAL help - Selecting this option will enable handling of the extra features - present on the UART found on various Intel platforms such as: + Selecting this option will enable handling of the UART found on + various Intel platforms such as: - Intel Baytrail SoC - Intel Braswell SoC - Intel Quark X1000 SoC + that are not covered by the more generic SERIAL_8250_PCI option. config SERIAL_8250_MID tristate "Support for serial ports on Intel MID platforms" @@ -494,17 +495,18 @@ config SERIAL_8250_MID select HSU_DMA_PCI if (HSU_DMA && X86_INTEL_MID) select RATIONAL help - Selecting this option will enable handling of the extra features - present on the UART found on Intel Medfield SOC and various other - Intel platforms. + Selecting this option will enable handling of the UART found on + Intel Medfield SOC and various other Intel platforms that is not + covered by the more generic SERIAL_8250_PCI option. config SERIAL_8250_PERICOM tristate "Support for Pericom and Acces I/O serial ports" default SERIAL_8250 depends on SERIAL_8250 && PCI help - Selecting this option will enable handling of the extra features - present on the Pericom and Acces I/O UARTs. + Selecting this option will enable handling of the Pericom and Acces + I/O UARTs that are not covered by the more generic SERIAL_8250_PCI + option. config SERIAL_8250_PXA tristate "PXA serial port support" ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] serial: 8250: Report which option to enable for blacklisted PCI devices 2022-02-12 17:30 [PATCH v2 0/2] serial: 8250: Correct basic issues with the PCI blacklist Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 1/2] serial: 8250: Correct Kconfig help text for blacklisted PCI devices Maciej W. Rozycki @ 2022-02-12 17:30 ` Maciej W. Rozycki 2022-02-25 9:35 ` Greg Kroah-Hartman 1 sibling, 1 reply; 7+ messages in thread From: Maciej W. Rozycki @ 2022-02-12 17:30 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: Andy Shevchenko, linux-serial, linux-kernel Provide information in the kernel log as to what configuration option to enable for PCI UART devices that have been blacklisted in the generic PCI 8250 UART driver and which have a dedicated driver available to handle that has been disabled. The rationale is there is no easy way for the user to map a specific PCI vendor:device pair to an individual dedicated driver while the generic driver has this information readily available and it will likely be confusing that the generic driver does not register such a port. A message is then printed like: serial 0000:04:00.3: ignoring port, enable SERIAL_8250_PERICOM to handle when an affected device is encountered and the generic driver rejects it. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> --- Hi, Verified in a simulated environment as obviously I don't have a Pericom device. Maciej Changes from v1: - Add missing filler struct member initialisers for PCI_DEVICE entries. --- drivers/tty/serial/8250/8250_pci.c | 67 ++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 27 deletions(-) linux-serial-8250-pci-blacklist-config.diff Index: linux-macro/drivers/tty/serial/8250/8250_pci.c =================================================================== --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c +++ linux-macro/drivers/tty/serial/8250/8250_pci.c @@ -3518,6 +3518,12 @@ static struct pciserial_board pci_boards }, }; +#define REPORT_CONFIG(option) \ + (IS_ENABLED(CONFIG_##option) ? 0 : (kernel_ulong_t)&#option) +#define REPORT_8250_CONFIG(option) \ + (IS_ENABLED(CONFIG_SERIAL_8250_##option) ? \ + 0 : (kernel_ulong_t)&"SERIAL_8250_"#option) + static const struct pci_device_id blacklist[] = { /* softmodems */ { PCI_VDEVICE(AL, 0x5457), }, /* ALi Corporation M5457 AC'97 Modem */ @@ -3525,40 +3531,43 @@ static const struct pci_device_id blackl { PCI_DEVICE(0x1543, 0x3052), }, /* Si3052-based modem, default IDs */ /* multi-io cards handled by parport_serial */ - { PCI_DEVICE(0x4348, 0x7053), }, /* WCH CH353 2S1P */ - { PCI_DEVICE(0x4348, 0x5053), }, /* WCH CH353 1S1P */ - { PCI_DEVICE(0x1c00, 0x3250), }, /* WCH CH382 2S1P */ + /* WCH CH353 2S1P */ + { PCI_DEVICE(0x4348, 0x7053), 0, 0, REPORT_CONFIG(PARPORT_SERIAL), }, + /* WCH CH353 1S1P */ + { PCI_DEVICE(0x4348, 0x5053), 0, 0, REPORT_CONFIG(PARPORT_SERIAL), }, + /* WCH CH382 2S1P */ + { PCI_DEVICE(0x1c00, 0x3250), 0, 0, REPORT_CONFIG(PARPORT_SERIAL), }, /* Intel platforms with MID UART */ - { PCI_VDEVICE(INTEL, 0x081b), }, - { PCI_VDEVICE(INTEL, 0x081c), }, - { PCI_VDEVICE(INTEL, 0x081d), }, - { PCI_VDEVICE(INTEL, 0x1191), }, - { PCI_VDEVICE(INTEL, 0x18d8), }, - { PCI_VDEVICE(INTEL, 0x19d8), }, + { PCI_VDEVICE(INTEL, 0x081b), REPORT_8250_CONFIG(MID), }, + { PCI_VDEVICE(INTEL, 0x081c), REPORT_8250_CONFIG(MID), }, + { PCI_VDEVICE(INTEL, 0x081d), REPORT_8250_CONFIG(MID), }, + { PCI_VDEVICE(INTEL, 0x1191), REPORT_8250_CONFIG(MID), }, + { PCI_VDEVICE(INTEL, 0x18d8), REPORT_8250_CONFIG(MID), }, + { PCI_VDEVICE(INTEL, 0x19d8), REPORT_8250_CONFIG(MID), }, /* Intel platforms with DesignWare UART */ - { PCI_VDEVICE(INTEL, 0x0936), }, - { PCI_VDEVICE(INTEL, 0x0f0a), }, - { PCI_VDEVICE(INTEL, 0x0f0c), }, - { PCI_VDEVICE(INTEL, 0x228a), }, - { PCI_VDEVICE(INTEL, 0x228c), }, - { PCI_VDEVICE(INTEL, 0x4b96), }, - { PCI_VDEVICE(INTEL, 0x4b97), }, - { PCI_VDEVICE(INTEL, 0x4b98), }, - { PCI_VDEVICE(INTEL, 0x4b99), }, - { PCI_VDEVICE(INTEL, 0x4b9a), }, - { PCI_VDEVICE(INTEL, 0x4b9b), }, - { PCI_VDEVICE(INTEL, 0x9ce3), }, - { PCI_VDEVICE(INTEL, 0x9ce4), }, + { PCI_VDEVICE(INTEL, 0x0936), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x0f0a), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x0f0c), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x228a), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x228c), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b96), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b97), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b98), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b99), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b9a), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x4b9b), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x9ce3), REPORT_8250_CONFIG(LPSS), }, + { PCI_VDEVICE(INTEL, 0x9ce4), REPORT_8250_CONFIG(LPSS), }, /* Exar devices */ - { PCI_VDEVICE(EXAR, PCI_ANY_ID), }, - { PCI_VDEVICE(COMMTECH, PCI_ANY_ID), }, + { PCI_VDEVICE(EXAR, PCI_ANY_ID), REPORT_8250_CONFIG(EXAR), }, + { PCI_VDEVICE(COMMTECH, PCI_ANY_ID), REPORT_8250_CONFIG(EXAR), }, /* Pericom devices */ - { PCI_VDEVICE(PERICOM, PCI_ANY_ID), }, - { PCI_VDEVICE(ACCESSIO, PCI_ANY_ID), }, + { PCI_VDEVICE(PERICOM, PCI_ANY_ID), REPORT_8250_CONFIG(PERICOM), }, + { PCI_VDEVICE(ACCESSIO, PCI_ANY_ID), REPORT_8250_CONFIG(PERICOM), }, /* End of the black list */ { } @@ -3840,8 +3849,12 @@ pciserial_init_one(struct pci_dev *dev, board = &pci_boards[ent->driver_data]; exclude = pci_match_id(blacklist, dev); - if (exclude) + if (exclude) { + if (exclude->driver_data) + pci_warn(dev, "ignoring port, enable %s to handle\n", + (const char *)exclude->driver_data); return -ENODEV; + } rc = pcim_enable_device(dev); pci_save_state(dev); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] serial: 8250: Report which option to enable for blacklisted PCI devices 2022-02-12 17:30 ` [PATCH v2 2/2] serial: 8250: Report which option to enable " Maciej W. Rozycki @ 2022-02-25 9:35 ` Greg Kroah-Hartman 2022-02-26 10:32 ` Maciej W. Rozycki 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2022-02-25 9:35 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: Jiri Slaby, Andy Shevchenko, linux-serial, linux-kernel On Sat, Feb 12, 2022 at 05:30:59PM +0000, Maciej W. Rozycki wrote: > Provide information in the kernel log as to what configuration option to > enable for PCI UART devices that have been blacklisted in the generic > PCI 8250 UART driver and which have a dedicated driver available to > handle that has been disabled. The rationale is there is no easy way > for the user to map a specific PCI vendor:device pair to an individual > dedicated driver while the generic driver has this information readily > available and it will likely be confusing that the generic driver does > not register such a port. > > A message is then printed like: > > serial 0000:04:00.3: ignoring port, enable SERIAL_8250_PERICOM to handle > > when an affected device is encountered and the generic driver rejects it. > > Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> I've applied patch 1 of this series, but this is really an odd one. We don't do this for any other driver subsystem, so why is it really needed? What is so special about this driver that distros can't just enable all of the drivers and all is good? What is keeping those drivers fromb eing enabled? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] serial: 8250: Report which option to enable for blacklisted PCI devices 2022-02-25 9:35 ` Greg Kroah-Hartman @ 2022-02-26 10:32 ` Maciej W. Rozycki 2022-02-27 23:06 ` Maciej W. Rozycki 0 siblings, 1 reply; 7+ messages in thread From: Maciej W. Rozycki @ 2022-02-26 10:32 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, Andy Shevchenko, linux-serial, linux-kernel On Fri, 25 Feb 2022, Greg Kroah-Hartman wrote: > On Sat, Feb 12, 2022 at 05:30:59PM +0000, Maciej W. Rozycki wrote: > > Provide information in the kernel log as to what configuration option to > > enable for PCI UART devices that have been blacklisted in the generic > > PCI 8250 UART driver and which have a dedicated driver available to > > handle that has been disabled. The rationale is there is no easy way > > for the user to map a specific PCI vendor:device pair to an individual > > dedicated driver while the generic driver has this information readily > > available and it will likely be confusing that the generic driver does > > not register such a port. > > > > A message is then printed like: > > > > serial 0000:04:00.3: ignoring port, enable SERIAL_8250_PERICOM to handle > > > > when an affected device is encountered and the generic driver rejects it. > > > > Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> > > I've applied patch 1 of this series, but this is really an odd one. Thank you. > We don't do this for any other driver subsystem, so why is it really > needed? What is so special about this driver that distros can't > just enable all of the drivers and all is good? What is keeping those > drivers fromb eing enabled? My justification is we have a supposedly generic PCI 8250 UART driver, except it explicitly and silently refuses to handle a handful of devices chosen by their PCI IDs based on that they may have extra features, even though they are otherwise fully compatible with a generic 8250. For distributions it probably does not matter as long as the packager does not forget to enable an option, which itself might be a problem (I've seen distributions missing drivers randomly). A user who configures their kernel on their own may simply not be aware that for one card enabling SERIAL_8250_PCI will do while for another almost identical card they need to use SERIAL_8250_foo instead even though it's just another PCI 8250 UART. Consequently someone may well waste a day trying to figure out why their card does not work (is it faulty perhaps, is there a configuration error with the hardware?). Even if they actually realise it's a kernel config issue, they may still have to go through a trial-and-error experience trying to figure out which driver to enable. While the generic driver knows perfectly well. Then why not make people's life easier and let them know as well what is going on? I don't think we have another case like this, do we? Hence my proposal. Have I made myself clear now? What are your actual arguments against my reasoning? Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] serial: 8250: Report which option to enable for blacklisted PCI devices 2022-02-26 10:32 ` Maciej W. Rozycki @ 2022-02-27 23:06 ` Maciej W. Rozycki 2022-03-31 7:12 ` Maciej W. Rozycki 0 siblings, 1 reply; 7+ messages in thread From: Maciej W. Rozycki @ 2022-02-27 23:06 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, Andy Shevchenko, linux-serial, linux-kernel On Sat, 26 Feb 2022, Maciej W. Rozycki wrote: > On Fri, 25 Feb 2022, Greg Kroah-Hartman wrote: > > > We don't do this for any other driver subsystem, so why is it really > > needed? What is so special about this driver that distros can't > > just enable all of the drivers and all is good? What is keeping those > > drivers fromb eing enabled? > > My justification is we have a supposedly generic PCI 8250 UART driver, > except it explicitly and silently refuses to handle a handful of devices > chosen by their PCI IDs based on that they may have extra features, even > though they are otherwise fully compatible with a generic 8250. Actually as it happens we do have a precedent too, as here's what I have just spotted on my laptop by chance when hibernating: psmouse serio1: synaptics: The touchpad can support a better bus than the too old PS/2 protocol. Make sure MOUSE_PS2_SYNAPTICS_SMBUS and RMI4_SMB are enabled to get a better touchpad experience. (with a distribution kernel, so clearly whoever packaged that has not enabled what might be needed). Someone else wanted to be helpful too as it seems. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] serial: 8250: Report which option to enable for blacklisted PCI devices 2022-02-27 23:06 ` Maciej W. Rozycki @ 2022-03-31 7:12 ` Maciej W. Rozycki 0 siblings, 0 replies; 7+ messages in thread From: Maciej W. Rozycki @ 2022-03-31 7:12 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jiri Slaby, Andy Shevchenko, linux-serial, linux-kernel On Sun, 27 Feb 2022, Maciej W. Rozycki wrote: > > > We don't do this for any other driver subsystem, so why is it really > > > needed? What is so special about this driver that distros can't > > > just enable all of the drivers and all is good? What is keeping those > > > drivers fromb eing enabled? > > > > My justification is we have a supposedly generic PCI 8250 UART driver, > > except it explicitly and silently refuses to handle a handful of devices > > chosen by their PCI IDs based on that they may have extra features, even > > though they are otherwise fully compatible with a generic 8250. > > Actually as it happens we do have a precedent too, as here's what I have > just spotted on my laptop by chance when hibernating: > > psmouse serio1: synaptics: The touchpad can support a better bus than the too old PS/2 protocol. Make sure MOUSE_PS2_SYNAPTICS_SMBUS and RMI4_SMB are enabled to get a better touchpad experience. > > (with a distribution kernel, so clearly whoever packaged that has not > enabled what might be needed). Someone else wanted to be helpful too as > it seems. I have now posted v3 with these clarifications included in the change descriptions. Please review. Maciej ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-31 7:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-12 17:30 [PATCH v2 0/2] serial: 8250: Correct basic issues with the PCI blacklist Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 1/2] serial: 8250: Correct Kconfig help text for blacklisted PCI devices Maciej W. Rozycki 2022-02-12 17:30 ` [PATCH v2 2/2] serial: 8250: Report which option to enable " Maciej W. Rozycki 2022-02-25 9:35 ` Greg Kroah-Hartman 2022-02-26 10:32 ` Maciej W. Rozycki 2022-02-27 23:06 ` Maciej W. Rozycki 2022-03-31 7:12 ` Maciej W. Rozycki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.