| Message ID | 1420513785-23660-1-git-send-email-peter@hurleysoftware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday 05 January 2015 22:09:45 Peter Hurley wrote: > Some arches have no need to create unprobed 8250 ports; these phantom > ports are primarily required for ISA ports which have no probe > mechanism or to provide non-operational ports for userspace to > configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). > > Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port > registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers > probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). > > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Cc: Tony Lindgren <tony@atomide.com> > Cc: Grant Likely <grant.likely@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Peter Hurley <peter@hurleysoftware.com> The intent is definitely right, but I think a better approach is possible. I haven't tried it here, but how about moving the serial8250_init function into a separate module, along with all the other parts that are only used for ISA devices, but leaving the actual core (all exported symbols) in this file? At the same time, the serial8250_pnp_init/serial8250_pnp_exit calls can be removed from the serial8250_init function and become standalone initcalls. Arnd
On 01/06/2015 08:13 AM, Arnd Bergmann wrote: > On Monday 05 January 2015 22:09:45 Peter Hurley wrote: >> Some arches have no need to create unprobed 8250 ports; these phantom >> ports are primarily required for ISA ports which have no probe >> mechanism or to provide non-operational ports for userspace to >> configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). >> >> Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port >> registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers >> probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). >> >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> Cc: Tony Lindgren <tony@atomide.com> >> Cc: Grant Likely <grant.likely@linaro.org> >> Cc: Arnd Bergmann <arnd@arndb.de> >> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> > > The intent is definitely right, but I think a better approach is > possible. > > I haven't tried it here, but how about moving the serial8250_init > function into a separate module, along with all the other parts > that are only used for ISA devices, but leaving the actual core > (all exported symbols) in this file? Unfortunately, I don't see a way to remove the stacked initialization without risking tons of breakage. Since later probes can "find" an already-existing port and re-initialize it, the probe order is crucial. For example, a PCI probe can "find" an existing "serial8250" platform device port, resulting in only one device node. And the configuration knob will be required on all arches anyway because that's how user-configurable device nodes are created. > At the same time, the serial8250_pnp_init/serial8250_pnp_exit calls > can be removed from the serial8250_init function and become > standalone initcalls. PNP probe must occur before the phantom ports are registered. See commit 835d844d1a28efba81d5aca7385e24c29d3a6db2 ("8250_pnp: do pnp probe before legacy probe"). Regards, Peter Hurley
On Tuesday 06 January 2015 09:32:02 Peter Hurley wrote: > On 01/06/2015 08:13 AM, Arnd Bergmann wrote: > > On Monday 05 January 2015 22:09:45 Peter Hurley wrote: > >> Some arches have no need to create unprobed 8250 ports; these phantom > >> ports are primarily required for ISA ports which have no probe > >> mechanism or to provide non-operational ports for userspace to > >> configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). > >> > >> Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port > >> registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers > >> probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). > >> > >> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > >> Cc: Tony Lindgren <tony@atomide.com> > >> Cc: Grant Likely <grant.likely@linaro.org> > >> Cc: Arnd Bergmann <arnd@arndb.de> > >> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> > > > > The intent is definitely right, but I think a better approach is > > possible. > > > > I haven't tried it here, but how about moving the serial8250_init > > function into a separate module, along with all the other parts > > that are only used for ISA devices, but leaving the actual core > > (all exported symbols) in this file? > > Unfortunately, I don't see a way to remove the stacked initialization > without risking tons of breakage. > > Since later probes can "find" an already-existing port and > re-initialize it, the probe order is crucial. For example, a PCI > probe can "find" an existing "serial8250" platform device port, > resulting in only one device node. I'm probably missing something important, by why would that be any different if the PCI driver gets loaded first and the ISA driver second? > And the configuration knob will be required on all arches anyway because > that's how user-configurable device nodes are created. I think that's fine: The user-configurable ports are the same as the "ISA" or "phantom" ports we were talking about above, right? If those are part of a separate (possibly loadable) module, having a configuration knob is the obvious way to do it. A lot of architectures can just turn it off because they know exactly which ports are present and there is no need for user-configurability. The ones that don't know can load the module. > > At the same time, the serial8250_pnp_init/serial8250_pnp_exit calls > > can be removed from the serial8250_init function and become > > standalone initcalls. > > PNP probe must occur before the phantom ports are registered. > See commit 835d844d1a28efba81d5aca7385e24c29d3a6db2 > ("8250_pnp: do pnp probe before legacy probe"). Makes sense. Arnd
On 01/06/2015 02:43 PM, Arnd Bergmann wrote: > On Tuesday 06 January 2015 09:32:02 Peter Hurley wrote: >> On 01/06/2015 08:13 AM, Arnd Bergmann wrote: >>> On Monday 05 January 2015 22:09:45 Peter Hurley wrote: >>>> Some arches have no need to create unprobed 8250 ports; these phantom >>>> ports are primarily required for ISA ports which have no probe >>>> mechanism or to provide non-operational ports for userspace to >>>> configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). >>>> >>>> Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port >>>> registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers >>>> probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). >>>> >>>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >>>> Cc: Tony Lindgren <tony@atomide.com> >>>> Cc: Grant Likely <grant.likely@linaro.org> >>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> >>> >>> The intent is definitely right, but I think a better approach is >>> possible. >>> >>> I haven't tried it here, but how about moving the serial8250_init >>> function into a separate module, along with all the other parts >>> that are only used for ISA devices, but leaving the actual core >>> (all exported symbols) in this file? >> >> Unfortunately, I don't see a way to remove the stacked initialization >> without risking tons of breakage. >> >> Since later probes can "find" an already-existing port and >> re-initialize it, the probe order is crucial. For example, a PCI >> probe can "find" an existing "serial8250" platform device port, >> resulting in only one device node. > > I'm probably missing something important, by why would that > be any different if the PCI driver gets loaded first and the > ISA driver second? Well, the PCI driver would have the proper irq, for one. So, if the the platform driver re-initialized the port to the wrong irq... >> And the configuration knob will be required on all arches anyway because >> that's how user-configurable device nodes are created. > > I think that's fine: The user-configurable ports are the same as > the "ISA" or "phantom" ports we were talking about above, right? Yes. > If those are part of a separate (possibly loadable) module, having > a configuration knob is the obvious way to do it. A lot of architectures > can just turn it off because they know exactly which ports are present > and there is no need for user-configurability. The ones that don't know > can load the module. Let me give this some more thought. Regards, Peter Hurley
On Tuesday 06 January 2015 16:47:55 Peter Hurley wrote: > On 01/06/2015 02:43 PM, Arnd Bergmann wrote: > > On Tuesday 06 January 2015 09:32:02 Peter Hurley wrote: > >> On 01/06/2015 08:13 AM, Arnd Bergmann wrote: > >>> On Monday 05 January 2015 22:09:45 Peter Hurley wrote: > >>>> Some arches have no need to create unprobed 8250 ports; these phantom > >>>> ports are primarily required for ISA ports which have no probe > >>>> mechanism or to provide non-operational ports for userspace to > >>>> configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). > >>>> > >>>> Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port > >>>> registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers > >>>> probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). > >>>> > >>>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > >>>> Cc: Tony Lindgren <tony@atomide.com> > >>>> Cc: Grant Likely <grant.likely@linaro.org> > >>>> Cc: Arnd Bergmann <arnd@arndb.de> > >>>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> > >>> > >>> The intent is definitely right, but I think a better approach is > >>> possible. > >>> > >>> I haven't tried it here, but how about moving the serial8250_init > >>> function into a separate module, along with all the other parts > >>> that are only used for ISA devices, but leaving the actual core > >>> (all exported symbols) in this file? > >> > >> Unfortunately, I don't see a way to remove the stacked initialization > >> without risking tons of breakage. > >> > >> Since later probes can "find" an already-existing port and > >> re-initialize it, the probe order is crucial. For example, a PCI > >> probe can "find" an existing "serial8250" platform device port, > >> resulting in only one device node. > > > > I'm probably missing something important, by why would that > > be any different if the PCI driver gets loaded first and the > > ISA driver second? > > Well, the PCI driver would have the proper irq, for one. So, if the > the platform driver re-initialized the port to the wrong irq... I see. So we must still ensure that the ISA driver either gets loaded before the PCI driver, or never. It is already closely coupled with the PNP driver, but I think we can still get to the point where you don't load the ISA driver at all if you only have platform 8250 ports that are not supposed to be reconfigurable. On most non-x86 machines, I would think we also want a way to build the PCI driver without that dependency. Arnd
On Mon, 5 Jan 2015 22:09:45 -0500 Peter Hurley <peter@hurleysoftware.com> wrote: > Some arches have no need to create unprobed 8250 ports; these phantom > ports are primarily required for ISA ports which have no probe > mechanism or to provide non-operational ports for userspace to > configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). > > Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port > registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers > probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). Just #define serial8250_isa_devs to 0 on such platforms. gcc should then be bright enough to do the rest for you.
On 01/08/2015 08:10 AM, One Thousand Gnomes wrote: > On Mon, 5 Jan 2015 22:09:45 -0500 > Peter Hurley <peter@hurleysoftware.com> wrote: > >> Some arches have no need to create unprobed 8250 ports; these phantom >> ports are primarily required for ISA ports which have no probe >> mechanism or to provide non-operational ports for userspace to >> configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). >> >> Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port >> registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers >> probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). > > Just #define serial8250_isa_devs to 0 on such platforms. gcc should then > be bright enough to do the rest for you. Well, this patch has the "advantage" of allowing the user to override the config and create blank ports anyway. That said, I'm exploring a completely different approach: I've split the driver from the port operations. The idea being that the existing "universal" driver supports everything it does now, while allowing for a new driver(s) to only support a subset. At this point, I've only done the actual split. So the "universal" driver module has: * ISA/"serial8250" platform driver * PNP init * uart driver and console definitions * all the existing module parameters * all the globals (except the const uart_config[] table) * RSA port handling * Chained interrupt handling * port i/o by timer handling * early_serial_setup() * sun ttyS shared registration * serial8250_ports[] table and how it re-uses existing ports The "universal" driver also has the sub-driver interface, which is: * serial8250_register_8250_port * serial8250_unregister_port * serial8250_suspend/resume_port * serial8250_find_port (hack for 8250_early) * serial8250_get_port (hack for runtime pm) This interface is just storage and minor allocation, since the port-reuse behavior will be limited to the "universal" driver. From a sub-driver perspective, the shared storage is actually a hindrance, so that reduces the requirement to minor allocation. And that's where I'm stuck at the moment -- how to share ttyS minor allocation. ttyS console is a related problem. Regards, Peter Hurley
On Thursday 08 January 2015 11:11:51 Peter Hurley wrote: > > This interface is just storage and minor allocation, since the > port-reuse behavior will be limited to the "universal" driver. > From a sub-driver perspective, the shared storage is actually > a hindrance, so that reduces the requirement to minor allocation. > > And that's where I'm stuck at the moment -- how to share ttyS > minor allocation. ttyS console is a related problem. One idea that has come up in the past but never saw an implementation is to make the ttyS namespace and minor numbers completely generic and let any serial port driver use it. This would be a major rework, but have the added advantage of cleaning up a number of other namespace issues as well. There also lots of open question, in particular how to maintain compatibility with existing drivers. One could imagine that each uart always gets a ttyS device and optionally also gets a device node for the same port with a driver specific chardev as most of them do today. Or it could be an either/or decision that is made at compile time or as a module parameter. Arnd
> One idea that has come up in the past but never saw an implementation > is to make the ttyS namespace and minor numbers completely generic and > let any serial port driver use it. This would be a major rework, but > have the added advantage of cleaning up a number of other namespace > issues as well. There also lots of open question, in particular how > to maintain compatibility with existing drivers. One could imagine > that each uart always gets a ttyS device and optionally also gets > a device node for the same port with a driver specific chardev as > most of them do today. Or it could be an either/or decision that is > made at compile time or as a module parameter. I'm not sure we should use ttyS for it. ttyS means 8250, a lot of existing code knows what ttyS is and what sort of ioctls and the like are assumed. A generic namespace needs not to be attached to a legacy naming (eg also put all serial ports in /dev/serial/0 /dev/serial/1 etc either via the tty layer or via systemd/udev) Alan
On 01/08/2015 05:36 PM, One Thousand Gnomes wrote: >> One idea that has come up in the past but never saw an implementation >> is to make the ttyS namespace and minor numbers completely generic and >> let any serial port driver use it. This would be a major rework, but >> have the added advantage of cleaning up a number of other namespace >> issues as well. There also lots of open question, in particular how >> to maintain compatibility with existing drivers. One could imagine >> that each uart always gets a ttyS device and optionally also gets >> a device node for the same port with a driver specific chardev as >> most of them do today. Or it could be an either/or decision that is >> made at compile time or as a module parameter. > > I'm not sure we should use ttyS for it. ttyS means 8250, a lot of > existing code knows what ttyS is and what sort of ioctls and the like are > assumed. That genie has already escaped the bottle. ./arch/powerpc/platforms/powermac/setup.c: char *devname = "ttyS"; ./arch/powerpc/platforms/chrp/setup.c: add_preferred_console("ttyS", 0, NULL); ./arch/powerpc/kernel/legacy_serial.c: return add_preferred_console("ttyS", offset, opt); ./arch/xtensa/platforms/iss/console.c: serial_driver->name = "ttyS"; ./arch/mips/sni/setup.c: add_preferred_console("ttyS", port, ./arch/mips/sgi-ip32/ip32-setup.c: add_preferred_console("ttyS", *(con + 1) == '2' ? 1 : 0, ./arch/mips/jazz/setup.c: add_preferred_console("ttyS", 0, "9600"); ./arch/mips/sgi-ip22/ip22-setup.c: add_preferred_console("ttyS", *(ctype + 1) == '2' ? 1 : 0, ./arch/arm/mach-davinci/board-da830-evm.c: return add_preferred_console("ttyS", 2, "115200"); ./arch/arm/mach-davinci/board-mityomapl138.c: return add_preferred_console("ttyS", 1, "115200"); ./arch/arm/mach-davinci/board-da850-evm.c: return add_preferred_console("ttyS", 2, "115200"); ./arch/arm/mach-davinci/board-omapl138-hawk.c: return add_preferred_console("ttyS", 2, "115200"); ./arch/cris/arch-v10/kernel/debugport.c: name : "ttyS", ./arch/ia64/hp/sim/simserial.c: hp_simserial_driver->name = "ttyS"; ./arch/um/drivers/ssl.c: .device_name = "ttyS", ./arch/s390/kernel/setup.c: add_preferred_console("ttyS", 1, NULL); ./drivers/tty/amiserial.c: .name = "ttyS", ./drivers/tty/serial/serial_txx9.c:#define TXX9_TTY_NAME "ttyS" ./drivers/tty/serial/sunsab.c: .dev_name = "ttyS", ./drivers/tty/serial/mcf.c: .dev_name = "ttyS", ./drivers/tty/serial/atmel_serial.c:#define ATMEL_DEVICENAME "ttyS" ./drivers/tty/serial/bcm63xx_uart.c: .dev_name = "ttyS", ./drivers/tty/serial/sunsu.c: .dev_name = "ttyS", ./drivers/tty/serial/mrst_max3110.c: .dev_name = "ttyS", ./drivers/tty/serial/apbuart.c: .name = "ttyS", ./drivers/tty/serial/apbuart.c: .dev_name = "ttyS", ./drivers/tty/serial/tilegx.c:#define TILEGX_UART_NAME "ttyS" ./drivers/tty/serial/pmac_zilog.c:#define PMACZILOG_NAME "ttyS" ./drivers/tty/serial/pnx8xxx_uart.c: .dev_name = "ttyS", ./drivers/tty/serial/m32r_sio.c: .dev_name = "ttyS", ./drivers/tty/serial/ip22zilog.c: .dev_name = "ttyS", ./drivers/tty/serial/zs.c: .dev_name = "ttyS", ./drivers/tty/serial/68328serial.c: serial_driver->name = "ttyS"; ./drivers/tty/serial/crisv10.c: driver->name = "ttyS"; /drivers/tty/serial/8250/8250_core.c: .dev_name = "ttyS", ./drivers/tty/serial/sunzilog.c: .dev_name = "ttyS", ./drivers/tty/serial/pxa.c: .dev_name = "ttyS", ./drivers/tty/serial/dz.c: .dev_name = "ttyS", ./drivers/tty/serial/sunhv.c: .dev_name = "ttyS", ./drivers/s390/char/sclp_vt220.c:#define SCLP_VT220_CONSOLE_NAME "ttyS" ./drivers/s390/char/sclp_con.c:#define sclp_console_name "ttyS" ./drivers/s390/char/con3215.c: driver->name = "ttyS"; Besides, despite its pedigree, omap is no closer to 8250 than anything else in drivers/tty/serial/ > A generic namespace needs not to be attached to a legacy naming > > (eg also put all serial ports in /dev/serial/0 /dev/serial/1 etc either > via the tty layer or via systemd/udev) This doesn't address pre-init. Regards, Peter Hurley
On 01/08/2015 05:05 PM, Arnd Bergmann wrote: > On Thursday 08 January 2015 11:11:51 Peter Hurley wrote: >> >> This interface is just storage and minor allocation, since the >> port-reuse behavior will be limited to the "universal" driver. >> From a sub-driver perspective, the shared storage is actually >> a hindrance, so that reduces the requirement to minor allocation. >> >> And that's where I'm stuck at the moment -- how to share ttyS >> minor allocation. ttyS console is a related problem. > > One idea that has come up in the past but never saw an implementation > is to make the ttyS namespace and minor numbers completely generic and > let any serial port driver use it. This would be a major rework, but > have the added advantage of cleaning up a number of other namespace > issues as well. There also lots of open question, in particular how > to maintain compatibility with existing drivers. One could imagine > that each uart always gets a ttyS device and optionally also gets > a device node for the same port with a driver specific chardev as > most of them do today. Or it could be an either/or decision that is > made at compile time or as a module parameter. I'm not totally convinced that sharing minor allocation is the right solution. ISTM that the main goal is for at least one serial driver to represent ttyS (assuming that it is a suitable work-alike to the 8250 driver) while other ttyS serial drivers must still be able to load and operate. So for example, a UART that is ttyS in a typical configuration, might be ttyX when another driver has been configured as ttyS instead. Assuming that every uart driver has a unique, alternate base name, then the problem is reduced to selecting the appropriate driver for ttyS. Some use cases would require selecting the ttyS driver at build time. Others would probably require the notion of a preferred driver. What got me thinking about this approach instead of sharing minor allocation was based on the 'first-come, first-served' question I asked back in Dec. What happens when probe order varies and console=ttyS0/login prompt doesn't show up where it's supposed to? I've had this experience with multiple ethernet adapters and it's not fun. Regards, Peter Hurley
On Friday 09 January 2015 00:13:28 Peter Hurley wrote: > On 01/08/2015 05:05 PM, Arnd Bergmann wrote: > > On Thursday 08 January 2015 11:11:51 Peter Hurley wrote: > >> > >> This interface is just storage and minor allocation, since the > >> port-reuse behavior will be limited to the "universal" driver. > >> From a sub-driver perspective, the shared storage is actually > >> a hindrance, so that reduces the requirement to minor allocation. > >> > >> And that's where I'm stuck at the moment -- how to share ttyS > >> minor allocation. ttyS console is a related problem. > > > > One idea that has come up in the past but never saw an implementation > > is to make the ttyS namespace and minor numbers completely generic and > > let any serial port driver use it. This would be a major rework, but > > have the added advantage of cleaning up a number of other namespace > > issues as well. There also lots of open question, in particular how > > to maintain compatibility with existing drivers. One could imagine > > that each uart always gets a ttyS device and optionally also gets > > a device node for the same port with a driver specific chardev as > > most of them do today. Or it could be an either/or decision that is > > made at compile time or as a module parameter. > > I'm not totally convinced that sharing minor allocation is the right > solution. ISTM that the main goal is for at least one serial driver > to represent ttyS (assuming that it is a suitable work-alike to the > 8250 driver) while other ttyS serial drivers must still be able to > load and operate. Unfortunately, we have already seen a case where we need to have two drivers enabled at the same time that both use the ttyS namespace and minor numbers at the moment. > So for example, a UART that is ttyS in a typical configuration, might > be ttyX when another driver has been configured as ttyS instead. > Assuming that every uart driver has a unique, alternate base name, > then the problem is reduced to selecting the appropriate driver for > ttyS. > > Some use cases would require selecting the ttyS driver at build time. > Others would probably require the notion of a preferred driver. Build-time selection looks problematic to me in the case of an ARM distro kernel that wants to support dozens of different uart drivers. At the moment this works for the most part because the conflicts are rare, but it's getting worse. > What got me thinking about this approach instead of sharing minor > allocation was based on the 'first-come, first-served' question > I asked back in Dec. What happens when probe order varies and > console=ttyS0/login prompt doesn't show up where it's supposed to? > I've had this experience with multiple ethernet adapters and it's > not fun. In a lot of cases, we can solve the problem for a particular board by hardcoding the 'aliases' in the board specific dts file, at least when devicetree is used (as it is for the majority of the cases that are causing problems now). Using the aliases correctly at the moment is really awkward when you have multiple drivers for uarts on the same system and your serial0 and serial1 uart turn into ttyS0 and ttyX1. Arnd
On 01/09/2015 03:48 AM, Arnd Bergmann wrote: > On Friday 09 January 2015 00:13:28 Peter Hurley wrote: >> On 01/08/2015 05:05 PM, Arnd Bergmann wrote: >>> On Thursday 08 January 2015 11:11:51 Peter Hurley wrote: >>>> >>>> This interface is just storage and minor allocation, since the >>>> port-reuse behavior will be limited to the "universal" driver. >>>> From a sub-driver perspective, the shared storage is actually >>>> a hindrance, so that reduces the requirement to minor allocation. >>>> >>>> And that's where I'm stuck at the moment -- how to share ttyS >>>> minor allocation. ttyS console is a related problem. >>> >>> One idea that has come up in the past but never saw an implementation >>> is to make the ttyS namespace and minor numbers completely generic and >>> let any serial port driver use it. This would be a major rework, but >>> have the added advantage of cleaning up a number of other namespace >>> issues as well. There also lots of open question, in particular how >>> to maintain compatibility with existing drivers. One could imagine >>> that each uart always gets a ttyS device and optionally also gets >>> a device node for the same port with a driver specific chardev as >>> most of them do today. Or it could be an either/or decision that is >>> made at compile time or as a module parameter. >> >> I'm not totally convinced that sharing minor allocation is the right >> solution. ISTM that the main goal is for at least one serial driver >> to represent ttyS (assuming that it is a suitable work-alike to the >> 8250 driver) while other ttyS serial drivers must still be able to >> load and operate. > > Unfortunately, we have already seen a case where we need to have > two drivers enabled at the same time that both use the ttyS > namespace and minor numbers at the moment. Maybe you misunderstood me here. I'm suggesting that every ttyS-capable driver would be able to load into a pre-determined, unique alternate namespace. So at least one ttyS driver would be present, without naming conflicts, and every other ttyS-conflicting driver would be loaded in their respective alternate namespace. Unless what you mean is there is some userspace requirement for the ttyS namespace to be shared. If so, would you please expand on this use case so I can better understand the requirements. >> So for example, a UART that is ttyS in a typical configuration, might >> be ttyX when another driver has been configured as ttyS instead. >> Assuming that every uart driver has a unique, alternate base name, >> then the problem is reduced to selecting the appropriate driver for >> ttyS. >> >> Some use cases would require selecting the ttyS driver at build time. >> Others would probably require the notion of a preferred driver. > > Build-time selection looks problematic to me in the case of an > ARM distro kernel that wants to support dozens of different > uart drivers. At the moment this works for the most part because > the conflicts are rare, but it's getting worse. A dt property could indicate which uart is the preferred ttyS. >> What got me thinking about this approach instead of sharing minor >> allocation was based on the 'first-come, first-served' question >> I asked back in Dec. What happens when probe order varies and >> console=ttyS0/login prompt doesn't show up where it's supposed to? >> I've had this experience with multiple ethernet adapters and it's >> not fun. > > In a lot of cases, we can solve the problem for a particular board > by hardcoding the 'aliases' in the board specific dts file, at least > when devicetree is used (as it is for the majority of the cases that > are causing problems now). > > Using the aliases correctly at the moment is really awkward when you > have multiple drivers for uarts on the same system and your serial0 > and serial1 uart turn into ttyS0 and ttyX1. I'm concerned that this will devolve into some attempt at a persistent naming scheme from kernel-space; which has repeatedly been shown in other subsystems to be fruitless. That was my point about the ethernet adapters. I have no problem sequentially numbering uarts as ttyS, but it would suck if that amount of work ultimately proved useless, when udev rules would have sufficed or became necessary anyway. Regards, Peter Hurley
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h index b008368..bda82f7 100644 --- a/drivers/tty/serial/8250/8250.h +++ b/drivers/tty/serial/8250/8250.h @@ -96,6 +96,12 @@ struct serial8250_config { #define SERIAL8250_SHARE_IRQS 0 #endif +#ifdef CONFIG_SERIAL_8250_PHANTOM_UARTS +#define SERIAL8250_PHANTOM_UARTS 1 +#else +#define SERIAL8250_PHANTOM_UARTS 0 +#endif + static inline int serial_in(struct uart_8250_port *up, int offset) { return up->port.serial_in(&up->port, offset); diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index 3bfcfdb..1b27b2f 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -52,10 +52,13 @@ * Configuration: * share_irqs - whether we pass IRQF_SHARED to request_irq(). This option * is unsafe when used on edge-triggered interrupts. + * nr_uarts - max # of ports which can be registered + * phantom_uarts - whether we pre-register "nr_uarts". Required for ISA ports + * and providing unprobed ports for userspace to configure. */ static unsigned int share_irqs = SERIAL8250_SHARE_IRQS; - static unsigned int nr_uarts = CONFIG_SERIAL_8250_RUNTIME_UARTS; +static unsigned int phantom_uarts = SERIAL8250_PHANTOM_UARTS; static struct uart_driver serial8250_reg; @@ -3155,6 +3158,9 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev) { int i; + if (!phantom_uarts) + return; + for (i = 0; i < nr_uarts; i++) { struct uart_8250_port *up = &serial8250_ports[i]; @@ -3662,7 +3668,7 @@ void serial8250_unregister_port(int line) mutex_lock(&serial_mutex); uart_remove_one_port(&serial8250_reg, &uart->port); - if (serial8250_isa_devs) { + if (serial8250_isa_devs && phantom_uarts) { uart->port.flags &= ~UPF_BOOT_AUTOCONF; uart->port.type = PORT_UNKNOWN; uart->port.dev = &serial8250_isa_devs->dev; @@ -3769,6 +3775,9 @@ MODULE_PARM_DESC(share_irqs, "Share IRQs with other non-8250/16x50 devices" module_param(nr_uarts, uint, 0644); MODULE_PARM_DESC(nr_uarts, "Maximum number of UARTs supported. (1-" __MODULE_STRING(CONFIG_SERIAL_8250_NR_UARTS) ")"); +module_param(phantom_uarts, uint, 0644); +MODULE_PARM_DESC(phantom_uarts, "Enable UARTs with no hardware"); + module_param(skip_txen_test, uint, 0644); MODULE_PARM_DESC(skip_txen_test, "Skip checking for the TXEN bug at init time"); diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 0fcbcd2..87e649b 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -145,15 +145,29 @@ config SERIAL_8250_NR_UARTS via hot-plug, or any ISA multi-port serial cards. config SERIAL_8250_RUNTIME_UARTS - int "Number of 8250/16550 serial ports to register at runtime" + int "Maximum number of 8250/16550 serial ports to enable" depends on SERIAL_8250 range 0 SERIAL_8250_NR_UARTS default "4" help - Set this to the maximum number of serial ports you want - the kernel to register at boot time. This can be overridden - with the module parameter "nr_uarts", or boot-time parameter - 8250.nr_uarts + Set this to the maximum number of 8250/16550 serial ports you + want the kernel to allow. This can be overridden with the module + parameter "nr_uarts", or boot-time parameter, 8250.nr_uarts, + up to the maximum. + +config SERIAL_8250_PHANTOM_UARTS + bool + prompt "Enable phantom 8250/16550 serial ports" if !ISA + depends on SERIAL_8250 + default y + ---help--- + Say Y here to create all 8250/16550 serial ports at module load time. + Saying N here prevents the creation of unprobed ports, but also + disables userspace probe/configure (since no device node is created + for userspace to open). This can be overridden with the module + parameter "phantom_uarts", or boot-time parameter, 8250.phantom_uarts + + If unsure, say Y. config SERIAL_8250_EXTENDED bool "Extended 8250/16550 serial driver options"
Some arches have no need to create unprobed 8250 ports; these phantom ports are primarily required for ISA ports which have no probe mechanism or to provide non-operational ports for userspace to configure (via TIOCSSERIAL and TIOCSERCONFIG ioctls). Provide CONFIG_SERIAL_8250_PHANTOM_UARTS knob to disable phantom port registration; ie., CONFIG_SERIAL_8250_PHANTOM_UARTS=N only registers probed ports (ACPI/PNP, "serial8250" platform devices, PCI, etc). Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: Tony Lindgren <tony@atomide.com> Cc: Grant Likely <grant.likely@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> --- drivers/tty/serial/8250/8250.h | 6 ++++++ drivers/tty/serial/8250/8250_core.c | 13 +++++++++++-- drivers/tty/serial/8250/Kconfig | 24 +++++++++++++++++++----- 3 files changed, 36 insertions(+), 7 deletions(-)