Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

> What I propose is a way for the arch to provide it's own table along
> with the size of it via a function call. It's optional, based on a
> #ifdef defined by the arch in it's asm/serial.h. The only remaining
> tricky point is the fact that you used to size your static array of
> UART's based on the size of the table. So with my path, an arch
> that defines ARCH_HAS_GET_LEGACY_SERIAL_PORTS is supposed to provide
> both the new get_legacy_serial_ports() function, but also to define
> UART_NR to something sensible. I hope one day, we'll be able to
> convert 8250 to more dynamic allocation though.

This looks like a reasonable short-term fix, but I think the whole
serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
of an 8250_platform.c that could use register_serial() for each port
in some platform-supplied old_serial_port[] table, which is probably
what you mean by moving to a more dynamic allocation.

AFAICS, the only reason for doing serial8250_isa_init_ports() early
is for early serial consoles, and I think those should be done along
the lines of this:
 http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/1034.html
where the platform can specify a device by its MMIO or IO port address,
and we automatically switch to the corresponding ttyS device later.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Benjamin Herrenschmidt]

On Fri, 2004-10-01 at 02:14, Bjorn Helgaas wrote:

> This looks like a reasonable short-term fix, but I think the whole
> serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
> of an 8250_platform.c that could use register_serial() for each port
> in some platform-supplied old_serial_port[] table, which is probably
> what you mean by moving to a more dynamic allocation.

What would this file look like ? a bunch of platform #ifdef's with
different implementations each time ? 

> AFAICS, the only reason for doing serial8250_isa_init_ports() early
> is for early serial consoles, and I think those should be done along
> the lines of this:
>  http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/1034.html
> where the platform can specify a device by its MMIO or IO port address,
> and we automatically switch to the corresponding ttyS device later.

Eventually...

In the meantime, that patch would fix urgent problems for ppc now so
I'd appreciate if Russell would consider it for inclusion asap. This
is the kind of subject on which everybody comes up with a different
"better" way to do it and no code, and nothign ever gets implemented
and we end up with no progress...

Ben.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

On Thursday 30 September 2004 5:53 pm, Benjamin Herrenschmidt wrote:
> On Fri, 2004-10-01 at 02:14, Bjorn Helgaas wrote:
> 
> > This looks like a reasonable short-term fix, but I think the whole
> > serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
> > of an 8250_platform.c that could use register_serial() for each port
> > in some platform-supplied old_serial_port[] table, which is probably
> > what you mean by moving to a more dynamic allocation.
> 
> What would this file look like ? a bunch of platform #ifdef's with
> different implementations each time ? 

My main point is that I think the early init stuff (i.e.,
serial8250_isa_init_ports()) should go away, so we don't have the
dichotomy of having the compiled-in stuff handled differently than
the run-time enumerated stuff.

It really doesn't matter for ia64, since we discover all the ports
via either PCI enumeration or ACPI.  We don't put anything in
old_serial_port[] at all.  For platforms that can't do that,
there'd still have to be compiled-in tables, but the entries
should be added using the standard register_serial() interface
just like everything else.

> > AFAICS, the only reason for doing serial8250_isa_init_ports() early
> > is for early serial consoles, and I think those should be done along
> > the lines of this:
> >  http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/1034.html
> > where the platform can specify a device by its MMIO or IO port address,
> > and we automatically switch to the corresponding ttyS device later.
> 
> Eventually...
> 
> In the meantime, that patch would fix urgent problems for ppc now so
> I'd appreciate if Russell would consider it for inclusion asap. This
> is the kind of subject on which everybody comes up with a different
> "better" way to do it and no code, and nothign ever gets implemented
> and we end up with no progress...

We've both posted working code that are at least baby steps toward
a better solution, so I hope we can make some progress.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Benjamin Herrenschmidt]

On Sat, 2004-10-02 at 00:58, Bjorn Helgaas wrote:

> My main point is that I think the early init stuff (i.e.,
> serial8250_isa_init_ports()) should go away, so we don't have the
> dichotomy of having the compiled-in stuff handled differently than
> the run-time enumerated stuff.

Right. But that's a long term goal ;)

> It really doesn't matter for ia64, since we discover all the ports
> via either PCI enumeration or ACPI.  We don't put anything in
> old_serial_port[] at all.  For platforms that can't do that,
> there'd still have to be compiled-in tables, but the entries
> should be added using the standard register_serial() interface
> just like everything else.

I discover everything at boot time via Open Firmware, though on
ppc32, embedded platforms have gazillion different ways of getting
to them.

> > In the meantime, that patch would fix urgent problems for ppc now so
> > I'd appreciate if Russell would consider it for inclusion asap. This
> > is the kind of subject on which everybody comes up with a different
> > "better" way to do it and no code, and nothign ever gets implemented
> > and we end up with no progress...
> 
> We've both posted working code that are at least baby steps toward
> a better solution, so I hope we can make some progress.

Yes. Well, I suppose having a 8250_ppc{64}.c where I do the discovery
a bit like it's done with ACPI would be ok for everything but serial
console... I can do an OF plaform device that gets probed a bit late
though, and that wouldn't help for embedded stuffs, but then, those
people can always still use the hard coded table for a while ... :)

That would mean though having the kernel serial console for early boot
be separate from the 8250 driver... like you proposed. That would work
too... I have to ponder this approach, indeed probably better than
my patch provided you get your early uart stuff upstream :)

Ben.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Russell King]

On Fri, Oct 01, 2004 at 08:58:27AM -0600, Bjorn Helgaas wrote:
> On Thursday 30 September 2004 5:53 pm, Benjamin Herrenschmidt wrote:
> > On Fri, 2004-10-01 at 02:14, Bjorn Helgaas wrote:
> > 
> > > This looks like a reasonable short-term fix, but I think the whole
> > > serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
> > > of an 8250_platform.c that could use register_serial() for each port
> > > in some platform-supplied old_serial_port[] table, which is probably
> > > what you mean by moving to a more dynamic allocation.
> > 
> > What would this file look like ? a bunch of platform #ifdef's with
> > different implementations each time ? 
> 
> My main point is that I think the early init stuff (i.e.,
> serial8250_isa_init_ports()) should go away, so we don't have the
> dichotomy of having the compiled-in stuff handled differently than
> the run-time enumerated stuff.

You're always going to have this.  For instance, the standard ISA serial
ports may not show up in any "enumerated stuff" on an x86 box - and x86
people expect that the port at 0x3f8 is ttyS0, 2f8 is ttyS1 etc.

Change that order and they'll scream at you.

> It really doesn't matter for ia64, since we discover all the ports
> via either PCI enumeration or ACPI.  We don't put anything in
> old_serial_port[] at all.  For platforms that can't do that,
> there'd still have to be compiled-in tables, but the entries
> should be added using the standard register_serial() interface
> just like everything else.

See my previous mail why this doesn't work - x86 serial console
requirements.

I think you'll do better to discuss this problem with Alan so that
he can change his (and maybe others) points of view wrt when the
serial console is initialised.  Until then I'm going to continue
sitting on the fence on this point.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

On Wednesday 06 October 2004 1:32 am, Russell King wrote:
> On Fri, Oct 01, 2004 at 08:58:27AM -0600, Bjorn Helgaas wrote:
> > My main point is that I think the early init stuff (i.e.,
> > serial8250_isa_init_ports()) should go away, so we don't have the
> > dichotomy of having the compiled-in stuff handled differently than
> > the run-time enumerated stuff.
> 
> You're always going to have this.  For instance, the standard ISA serial
> ports may not show up in any "enumerated stuff" on an x86 box - and x86
> people expect that the port at 0x3f8 is ttyS0, 2f8 is ttyS1 etc.
> 
> Change that order and they'll scream at you.

I don't forsee any order changing.  I would expect to use link
ordering so all the 8250_platform ports are registered first, then
all the 8250_acpi, then all the 8250_pci.

The order WOULD change in some cases on ia64, because we'd get rid
of the current wierdness where the device in the HCDP/PCDP firmware
table always becomes ttyS0, regardless of where it lives.  This
would be an improvement, though, because the devices would stop
changing names just because you selected a different firmware
console.

> See my previous mail why this doesn't work - x86 serial console
> requirements.
> 
> I think you'll do better to discuss this problem with Alan so that
> he can change his (and maybe others) points of view wrt when the
> serial console is initialised.  Until then I'm going to continue
> sitting on the fence on this point.

Yeah, I'll poke him about "console=uart".  I sent it to you because I
think a clean solution requires minor 8250 hooks so we can look up
the ttyS device that corresponds to an MMIO or IO address.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

On Wednesday 06 October 2004 1:54 pm, Bjorn Helgaas wrote:
> On Wednesday 06 October 2004 1:32 am, Russell King wrote:
> > I think you'll do better to discuss this problem with Alan so that
> > he can change his (and maybe others) points of view wrt when the
> > serial console is initialised.  Until then I'm going to continue
> > sitting on the fence on this point.
> 
> Yeah, I'll poke him about "console=uart".  I sent it to you because I
> think a clean solution requires minor 8250 hooks so we can look up
> the ttyS device that corresponds to an MMIO or IO address.

On second thought, I want the "console=uart" stuff regardless of
what happens with early port registration, because it fixes the
ia64 problem of port names changing based on firmware configuration.

I'd like to make forward progress on that.  Do you have any comments
on it?  The (whitespace-mangled) patch is here:
    http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/1034.html

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Russell King]

On Thu, Sep 30, 2004 at 10:14:00AM -0600, Bjorn Helgaas wrote:
> > What I propose is a way for the arch to provide it's own table along
> > with the size of it via a function call. It's optional, based on a
> > #ifdef defined by the arch in it's asm/serial.h. The only remaining
> > tricky point is the fact that you used to size your static array of
> > UART's based on the size of the table. So with my path, an arch
> > that defines ARCH_HAS_GET_LEGACY_SERIAL_PORTS is supposed to provide
> > both the new get_legacy_serial_ports() function, but also to define
> > UART_NR to something sensible. I hope one day, we'll be able to
> > convert 8250 to more dynamic allocation though.
> 
> This looks like a reasonable short-term fix, but I think the whole
> serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
> of an 8250_platform.c that could use register_serial() for each port
> in some platform-supplied old_serial_port[] table, which is probably
> what you mean by moving to a more dynamic allocation.

The only reason it exists in its current form is because Alan says
we can't get rid of the serial port initialisation due to the x86
requirement for serial console to be initialised reasonably early.

Unfortunately the early console stuff (afaik) never made it in to
the kernel, so we've had to keep this hanging around.

Maybe once this problem is solved we can consider dwmw2's suggestion.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

On Wednesday 06 October 2004 1:29 am, Russell King wrote:
> On Thu, Sep 30, 2004 at 10:14:00AM -0600, Bjorn Helgaas wrote:
> > This looks like a reasonable short-term fix, but I think the whole
> > serial8250_isa_init_ports() should go away.  I like dwmw2's suggestion
> > of an 8250_platform.c that could use register_serial() for each port
> > in some platform-supplied old_serial_port[] table, which is probably
> > what you mean by moving to a more dynamic allocation.
> 
> The only reason it exists in its current form is because Alan says
> we can't get rid of the serial port initialisation due to the x86
> requirement for serial console to be initialised reasonably early.
> 
> Unfortunately the early console stuff (afaik) never made it in to
> the kernel, so we've had to keep this hanging around.

My "console=uart" patch
 http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/1034.html
is a start, I think.  Relative to the current situation, it
 - requires different syntax ("console=uart" vs "console=ttyS0"),
   though a platform could choose to translate "console=ttyS0"
   into "console=uart,io,0x3f8"
 - can start working much earlier (no interrupts or clock
   calibration required)
 - doesn't deal with all the wierd devices the full driver does
 - transitions automatically to the matching ttyS device after
   the driver is initialized

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[David Woodhouse]

On Wed, 2004-10-06 at 08:29 +0100, Russell King wrote:
> The only reason it exists in its current form is because Alan says
> we can't get rid of the serial port initialisation due to the x86
> requirement for serial console to be initialised reasonably early.
> 
> Unfortunately the early console stuff (afaik) never made it in to
> the kernel, so we've had to keep this hanging around.
> 
> Maybe once this problem is solved we can consider dwmw2's suggestion.

Serial console working really early isn't just a requirement for x86.
It's useful elsewhere too. 

The problem is that 'console=ttySx' doesn't actually do anything unless
port numer 'x' is already registered and working. We should fix that --
we ought to be able to use 'console=ttySx' on the command line and have
the console get registered with the core printk code later, when some
8250 sub-driver (8250_platform, 8250_pci, etc.) actually registers the
port which becomes number 'x'. 

That would allow 'early' serial consoles to have none of the horrible
special-cased 'earlyconsole' crap -- we just call register_serial() (or
early_serial_setup() or whatever) as soon as it's actually possible to
poke at the port.

It also fixes the case of serial console on a PCI device.

-- 
dwmw2

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Bjorn Helgaas]

On Monday 01 November 2004 10:15 am, David Woodhouse wrote:
> The problem is that 'console=ttySx' doesn't actually do anything unless
> port numer 'x' is already registered and working. We should fix that --
> we ought to be able to use 'console=ttySx' on the command line and have
> the console get registered with the core printk code later, when some
> 8250 sub-driver (8250_platform, 8250_pci, etc.) actually registers the
> port which becomes number 'x'. 

See serial8250_late_console_init(); does this do what you want?

> That would allow 'early' serial consoles to have none of the horrible
> special-cased 'earlyconsole' crap -- we just call register_serial() (or
> early_serial_setup() or whatever) as soon as it's actually possible to
> poke at the port.

ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc1/2.6.10-rc1-mm2/broken-out/early-uart-console-support.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.10-rc1/2.6.10-rc1-mm2/broken-out/move-hcdp-pcdp-to-early-uart-console.patch

For platforms that define SERIAL_PORT_DFNS, early boot code could easily
recognize "console=ttyS0" and use SERIAL_PORT_DFNS to register
"console=uart,io,0x3f8".

If you don't have SERIAL_PORT_DFNS, early boot code doesn't know anything
about 'ttySx', so you need some other mechanism to find the device (the
user could specify it directly, or firmware could supply it).

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Russell King]

On Tue, Nov 02, 2004 at 09:39:25AM -0700, Bjorn Helgaas wrote:
> On Monday 01 November 2004 10:15 am, David Woodhouse wrote:
> > The problem is that 'console=ttySx' doesn't actually do anything unless
> > port numer 'x' is already registered and working. We should fix that --
> > we ought to be able to use 'console=ttySx' on the command line and have
> > the console get registered with the core printk code later, when some
> > 8250 sub-driver (8250_platform, 8250_pci, etc.) actually registers the
> > port which becomes number 'x'. 
> 
> See serial8250_late_console_init(); does this do what you want?

This call can now be removed - for any serial device, we keep registering
the console each time a new port is added until it successfully registers.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[David Woodhouse]

On Wed, 2004-11-03 at 07:43 +0000, Russell King wrote:
> This call can now be removed - for any serial device, we keep registering
> the console each time a new port is added until it successfully registers.

Er, if we already fixed that, then what was it which still needed to be
fixed? If the console is registered when the port is registered, what
stops us from dropping the static table and letting the i386
setup_arch() code register the port?

You'd still get the console working a lot earlier than it does today --
currently I think we wait for the console_initcalls to start the 8250
console on most platforms, including i386.

-- 
dwmw2

[RFC][PATCH] Way for platforms to alter built-in serial ports

[Benjamin Herrenschmidt]

Hi Russel !

I'm back with a finally useable fix for an oooooold problem, which is
the way the serial table is supposed to be hard coded in asm/serial.h
makes it really nasty to deal with for platforms like ppc where a
given kernel can boot machines with different setups or even no
built-in serial port at all.

(I currently have the 3 cases on ppc64: some normal ISA serial, some
with different port/irq settings, and machines with no ports at all,
all in the same kernel image).

The early_serial_setup() thing has never been practical to use, since
it basically consist of "shoving" entries into the table, which is a
bit ugly, requires knowledge of the table size if we want to remove
all entries but N, that sort of thing, but the MAIN issue is that it's
fundamentally incompatible with the driver beeing in a module.

What I propose is a way for the arch to provide it's own table along
with the size of it via a function call. It's optional, based on a
#ifdef defined by the arch in it's asm/serial.h. The only remaining
tricky point is the fact that you used to size your static array of
UART's based on the size of the table. So with my path, an arch
that defines ARCH_HAS_GET_LEGACY_SERIAL_PORTS is supposed to provide
both the new get_legacy_serial_ports() function, but also to define
UART_NR to something sensible. I hope one day, we'll be able to
convert 8250 to more dynamic allocation though.

With this patch, I fix all my problems of properly detecting built-in
serial ports _and_ not munging around with non existing ports on machines
with no legacy HW with a single kernel image on ppc64 (and hopefully on
ppc32 too). I have the ppc64-side of the patch beeing tested on various
hardware at the moment and will submit it if you accept this one.

Regards,
Ben.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

diff -urN linux-2.5/drivers/serial/8250.c linux-maple/drivers/serial/8250.c
--- linux-2.5/drivers/serial/8250.c	2004-09-30 18:31:42.560719874 +1000
+++ linux-maple/drivers/serial/8250.c	2004-09-30 16:27:39.421941590 +1000
@@ -112,11 +112,18 @@
 #define SERIAL_PORT_DFNS
 #endif
 
+#ifndef ARCH_HAS_GET_LEGACY_SERIAL_PORTS
 static struct old_serial_port old_serial_port[] = {
 	SERIAL_PORT_DFNS /* defined in asm/serial.h */
 };
-
+static inline struct old_serial_port *get_legacy_serial_ports(unsigned int *count)
+{
+	*count = ARRAY_SIZE(old_serial_port);
+	return old_serial_port;
+}
 #define UART_NR	(ARRAY_SIZE(old_serial_port) + CONFIG_SERIAL_8250_NR_UARTS)
+#endif /* ARCH_HAS_GET_LEGACY_SERIAL_PORTS */
+
 
 #ifdef CONFIG_SERIAL_8250_RSA
 
@@ -1839,22 +1846,28 @@
 {
 	struct uart_8250_port *up;
 	static int first = 1;
+	struct old_serial_port *old_ports;
+	int count;
 	int i;
 
 	if (!first)
 		return;
 	first = 0;
 
-	for (i = 0, up = serial8250_ports; i < ARRAY_SIZE(old_serial_port);
+	old_ports = get_legacy_serial_ports(&count);
+	if (old_ports == NULL)
+		return;
+
+	for (i = 0, up = serial8250_ports; i < count;
 	     i++, up++) {
-		up->port.iobase   = old_serial_port[i].port;
-		up->port.irq      = irq_canonicalize(old_serial_port[i].irq);
-		up->port.uartclk  = old_serial_port[i].baud_base * 16;
-		up->port.flags    = old_serial_port[i].flags;
-		up->port.hub6     = old_serial_port[i].hub6;
-		up->port.membase  = old_serial_port[i].iomem_base;
-		up->port.iotype   = old_serial_port[i].io_type;
-		up->port.regshift = old_serial_port[i].iomem_reg_shift;
+		up->port.iobase   = old_ports[i].port;
+		up->port.irq      = irq_canonicalize(old_ports[i].irq);
+		up->port.uartclk  = old_ports[i].baud_base * 16;
+		up->port.flags    = old_ports[i].flags;
+		up->port.hub6     = old_ports[i].hub6;
+		up->port.membase  = old_ports[i].iomem_base;
+		up->port.iotype   = old_ports[i].io_type;
+		up->port.regshift = old_ports[i].iomem_reg_shift;
 		up->port.ops      = &serial8250_pops;
 		if (share_irqs)
 			up->port.flags |= UPF_SHARE_IRQ;
diff -urN linux-2.5/drivers/serial/8250.h linux-maple/drivers/serial/8250.h
--- linux-2.5/drivers/serial/8250.h	2004-09-30 18:31:42.561719806 +1000
+++ linux-maple/drivers/serial/8250.h	2004-09-30 15:36:55.867448623 +1000
@@ -21,18 +21,6 @@
 void serial8250_suspend_port(int line);
 void serial8250_resume_port(int line);
 
-struct old_serial_port {
-	unsigned int uart;
-	unsigned int baud_base;
-	unsigned int port;
-	unsigned int irq;
-	unsigned int flags;
-	unsigned char hub6;
-	unsigned char io_type;
-	unsigned char *iomem_base;
-	unsigned short iomem_reg_shift;
-};
-
 /*
  * This replaces serial_uart_config in include/linux/serial.h
  */
diff -urN linux-2.5/include/linux/serial.h linux-maple/include/linux/serial.h
--- linux-2.5/include/linux/serial.h	2004-09-30 18:31:55.867785437 +1000
+++ linux-maple/include/linux/serial.h	2004-09-30 15:36:57.981697919 +1000
@@ -14,6 +14,21 @@
 #include <asm/page.h>
 
 /*
+ * Definition of a legacy serial port
+ */
+struct old_serial_port {
+	unsigned int uart;
+	unsigned int baud_base;
+	unsigned int port;
+	unsigned int irq;
+	unsigned int flags;
+	unsigned char hub6;
+	unsigned char io_type;
+	unsigned char *iomem_base;
+	unsigned short iomem_reg_shift;
+};
+
+/*
  * Counters of the input lines (CTS, DSR, RI, CD) interrupts
  */
 
 

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Russell King]

On Thu, Sep 30, 2004 at 06:50:48PM +1000, Benjamin Herrenschmidt wrote:
> +#ifndef ARCH_HAS_GET_LEGACY_SERIAL_PORTS
>  static struct old_serial_port old_serial_port[] = {
>  	SERIAL_PORT_DFNS /* defined in asm/serial.h */
>  };
> -
> +static inline struct old_serial_port *get_legacy_serial_ports(unsigned int *count)
> +{
> +	*count = ARRAY_SIZE(old_serial_port);
> +	return old_serial_port;
> +}
>  #define UART_NR	(ARRAY_SIZE(old_serial_port) + CONFIG_SERIAL_8250_NR_UARTS)
> +#endif /* ARCH_HAS_GET_LEGACY_SERIAL_PORTS */
> +

What happens if 8250.c is built as a module and
ARCH_HAS_GET_LEGACY_SERIAL_PORTS is defined?

> diff -urN linux-2.5/include/linux/serial.h linux-maple/include/linux/serial.h
> --- linux-2.5/include/linux/serial.h	2004-09-30 18:31:55.867785437 +1000
> +++ linux-maple/include/linux/serial.h	2004-09-30 15:36:57.981697919 +1000
> @@ -14,6 +14,21 @@
>  #include <asm/page.h>
>  
>  /*
> + * Definition of a legacy serial port
> + */
> +struct old_serial_port {
> +	unsigned int uart;
> +	unsigned int baud_base;
> +	unsigned int port;
> +	unsigned int irq;
> +	unsigned int flags;
> +	unsigned char hub6;
> +	unsigned char io_type;
> +	unsigned char *iomem_base;
> +	unsigned short iomem_reg_shift;
> +};
> +
> +/*
>   * Counters of the input lines (CTS, DSR, RI, CD) interrupts
>   */

serial.h is used by userspace programs.  We should not expose this
structure to those programs.  Instead, maybe creating an 8250.h
header, or even moving the existing 8250.h header ?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Benjamin Herrenschmidt]

On Wed, 2004-10-06 at 17:26, Russell King wrote:
> On Thu, Sep 30, 2004 at 06:50:48PM +1000, Benjamin Herrenschmidt wrote:
> > +#ifndef ARCH_HAS_GET_LEGACY_SERIAL_PORTS
> >  static struct old_serial_port old_serial_port[] = {
> >  	SERIAL_PORT_DFNS /* defined in asm/serial.h */
> >  };
> > -
> > +static inline struct old_serial_port *get_legacy_serial_ports(unsigned int *count)
> > +{
> > +	*count = ARRAY_SIZE(old_serial_port);
> > +	return old_serial_port;
> > +}
> >  #define UART_NR	(ARRAY_SIZE(old_serial_port) + CONFIG_SERIAL_8250_NR_UARTS)
> > +#endif /* ARCH_HAS_GET_LEGACY_SERIAL_PORTS */
> > +
> 
> What happens if 8250.c is built as a module and
> ARCH_HAS_GET_LEGACY_SERIAL_PORTS is defined?

It well call get_legacy_serial_ports() which is hopefully exported by
the arch code.

> serial.h is used by userspace programs.  We should not expose this
> structure to those programs.  Instead, maybe creating an 8250.h
> header, or even moving the existing 8250.h header ?

Hrm... ok. Or adding a #ifdef __KERNEL__ (sic !) :)

I'll send you a new patch later today as I had to do another fix, we
tend to "force" register_console() apparently even when we have nothing
to register because we set the "ops" to all ports even those who were
never configured and we test "ops" to decide wether to succeed or fail
in the console setup() callback.

Ben.

Re: [RFC][PATCH] Way for platforms to alter built-in serial ports

[Benjamin Herrenschmidt]

On Wed, 2004-10-06 at 17:26, Russell King wrote:

> serial.h is used by userspace programs.  We should not expose this
> structure to those programs.  Instead, maybe creating an 8250.h
> header, or even moving the existing 8250.h header ?

Here's a new version of that patch that moves 8250.h to
include/linux, moves the definition of old_serial_ports there,
and also corrects the problem I told you about with serial console.

Let me know if I can send it to Andrew...

Ben.

diff -urN linux-2.5/drivers/serial/8250.c linux-maple/drivers/serial/8250.c
--- linux-2.5/drivers/serial/8250.c	2004-09-30 18:31:42.000000000 +1000
+++ linux-maple/drivers/serial/8250.c	2004-10-06 19:05:13.042342513 +1000
@@ -41,7 +41,7 @@
 #endif
 
 #include <linux/serial_core.h>
-#include "8250.h"
+#include <linux/8250.h>
 
 /*
  * Configuration:
@@ -112,11 +112,18 @@
 #define SERIAL_PORT_DFNS
 #endif
 
+#ifndef ARCH_HAS_GET_LEGACY_SERIAL_PORTS
 static struct old_serial_port old_serial_port[] = {
 	SERIAL_PORT_DFNS /* defined in asm/serial.h */
 };
-
+static inline struct old_serial_port *get_legacy_serial_ports(unsigned int *count)
+{
+	*count = ARRAY_SIZE(old_serial_port);
+	return old_serial_port;
+}
 #define UART_NR	(ARRAY_SIZE(old_serial_port) + CONFIG_SERIAL_8250_NR_UARTS)
+#endif /* ARCH_HAS_DYNAMIC_LEGACY_SERIAL_PORTS */
+
 
 #ifdef CONFIG_SERIAL_8250_RSA
 
@@ -1839,22 +1846,28 @@
 {
 	struct uart_8250_port *up;
 	static int first = 1;
+	struct old_serial_port *old_ports;
+	int count;
 	int i;
 
 	if (!first)
 		return;
 	first = 0;
 
-	for (i = 0, up = serial8250_ports; i < ARRAY_SIZE(old_serial_port);
+	old_ports = get_legacy_serial_ports(&count);
+	if (old_ports == NULL)
+		return;
+
+	for (i = 0, up = serial8250_ports; i < count;
 	     i++, up++) {
-		up->port.iobase   = old_serial_port[i].port;
-		up->port.irq      = irq_canonicalize(old_serial_port[i].irq);
-		up->port.uartclk  = old_serial_port[i].baud_base * 16;
-		up->port.flags    = old_serial_port[i].flags;
-		up->port.hub6     = old_serial_port[i].hub6;
-		up->port.membase  = old_serial_port[i].iomem_base;
-		up->port.iotype   = old_serial_port[i].io_type;
-		up->port.regshift = old_serial_port[i].iomem_reg_shift;
+		up->port.iobase   = old_ports[i].port;
+		up->port.irq      = irq_canonicalize(old_ports[i].irq);
+		up->port.uartclk  = old_ports[i].baud_base * 16;
+		up->port.flags    = old_ports[i].flags;
+		up->port.hub6     = old_ports[i].hub6;
+		up->port.membase  = old_ports[i].iomem_base;
+		up->port.iotype   = old_ports[i].io_type;
+		up->port.regshift = old_ports[i].iomem_reg_shift;
 		up->port.ops      = &serial8250_pops;
 		if (share_irqs)
 			up->port.flags |= UPF_SHARE_IRQ;
@@ -1870,6 +1883,9 @@
 	for (i = 0; i < UART_NR; i++) {
 		struct uart_8250_port *up = &serial8250_ports[i];
 
+		if (!up->port.iobase)
+			continue;
+
 		up->port.line = i;
 		up->port.ops = &serial8250_pops;
 		init_timer(&up->timer);
diff -urN linux-2.5/drivers/serial/8250.h linux-maple/drivers/serial/8250.h
--- linux-2.5/drivers/serial/8250.h	2004-09-30 18:31:42.000000000 +1000
+++ /dev/null	2004-10-05 22:10:47.391719208 +1000
@@ -1,71 +0,0 @@
-/*
- *  linux/drivers/char/8250.h
- *
- *  Driver for 8250/16550-type serial ports
- *
- *  Based on drivers/char/serial.c, by Linus Torvalds, Theodore Ts'o.
- *
- *  Copyright (C) 2001 Russell King.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- *  $Id: 8250.h,v 1.8 2002/07/21 21:32:30 rmk Exp $
- */
-
-#include <linux/config.h>
-
-void serial8250_get_irq_map(unsigned int *map);
-void serial8250_suspend_port(int line);
-void serial8250_resume_port(int line);
-
-struct old_serial_port {
-	unsigned int uart;
-	unsigned int baud_base;
-	unsigned int port;
-	unsigned int irq;
-	unsigned int flags;
-	unsigned char hub6;
-	unsigned char io_type;
-	unsigned char *iomem_base;
-	unsigned short iomem_reg_shift;
-};
-
-/*
- * This replaces serial_uart_config in include/linux/serial.h
- */
-struct serial8250_config {
-	const char	*name;
-	unsigned int	fifo_size;
-	unsigned int	tx_loadsz;
-	unsigned int	flags;
-};
-
-#define UART_CAP_FIFO	(1 << 8)	/* UART has FIFO */
-#define UART_CAP_EFR	(1 << 9)	/* UART has EFR */
-#define UART_CAP_SLEEP	(1 << 10)	/* UART has IER sleep */
-
-#undef SERIAL_DEBUG_PCI
-
-#if defined(__i386__) && (defined(CONFIG_M386) || defined(CONFIG_M486))
-#define SERIAL_INLINE
-#endif
-  
-#ifdef SERIAL_INLINE
-#define _INLINE_ inline
-#else
-#define _INLINE_
-#endif
-
-#define PROBE_RSA	(1 << 0)
-#define PROBE_ANY	(~0)
-
-#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)
-
-#ifdef CONFIG_SERIAL_8250_SHARE_IRQ
-#define SERIAL8250_SHARE_IRQS 1
-#else
-#define SERIAL8250_SHARE_IRQS 0
-#endif
diff -urN linux-2.5/drivers/serial/8250_pci.c linux-maple/drivers/serial/8250_pci.c
--- linux-2.5/drivers/serial/8250_pci.c	2004-09-30 18:31:42.000000000 +1000
+++ linux-maple/drivers/serial/8250_pci.c	2004-10-06 19:05:41.301674308 +1000
@@ -25,13 +25,12 @@
 #include <linux/serial.h>
 #include <linux/serial_core.h>
 #include <linux/8250_pci.h>
+#include <linux/8250.h>
 
 #include <asm/bitops.h>
 #include <asm/byteorder.h>
 #include <asm/io.h>
 
-#include "8250.h"
-
 /*
  * Definitions for PCI support.
  */
diff -urN linux-2.5/drivers/serial/8250_pnp.c linux-maple/drivers/serial/8250_pnp.c
--- linux-2.5/drivers/serial/8250_pnp.c	2004-09-30 18:31:42.000000000 +1000
+++ linux-maple/drivers/serial/8250_pnp.c	2004-10-06 19:05:55.788749883 +1000
@@ -25,13 +25,12 @@
 #include <linux/serial.h>
 #include <linux/serialP.h>
 #include <linux/serial_core.h>
+#include <linux/8250.h>
 
 #include <asm/bitops.h>
 #include <asm/byteorder.h>
 #include <asm/serial.h>
 
-#include "8250.h"
-
 #define UNKNOWN_DEV 0x3000
 
 
diff -urN linux-2.5/drivers/serial/au1x00_uart.c linux-maple/drivers/serial/au1x00_uart.c
--- linux-2.5/drivers/serial/au1x00_uart.c	2004-09-30 18:31:42.000000000 +1000
+++ linux-maple/drivers/serial/au1x00_uart.c	2004-10-06 19:07:39.461032916 +1000
@@ -40,7 +40,7 @@
 #endif
 
 #include <linux/serial_core.h>
-#include "8250.h"
+#include <linux/8250.h>
 
 /*
  * Debugging.
diff -urN linux-2.5/drivers/serial/serial_cs.c linux-maple/drivers/serial/serial_cs.c
--- linux-2.5/drivers/serial/serial_cs.c	2004-09-30 18:31:42.000000000 +1000
+++ linux-maple/drivers/serial/serial_cs.c	2004-10-06 19:07:35.059700476 +1000
@@ -44,6 +44,7 @@
 #include <linux/serial.h>
 #include <linux/serial_core.h>
 #include <linux/major.h>
+#include <linux/8250.h>
 #include <asm/io.h>
 #include <asm/system.h>
 
@@ -55,8 +56,6 @@
 #include <pcmcia/ds.h>
 #include <pcmcia/cisreg.h>
 
-#include "8250.h"
-
 #ifdef PCMCIA_DEBUG
 static int pc_debug = PCMCIA_DEBUG;
 MODULE_PARM(pc_debug, "i");
diff -urN linux-2.5/include/linux/8250.h linux-maple/include/linux/8250.h
--- /dev/null	2004-10-05 22:10:47.391719208 +1000
+++ linux-maple/include/linux/8250.h	2004-10-06 19:06:45.680713598 +1000
@@ -0,0 +1,74 @@
+/*
+ *  linux/drivers/char/8250.h
+ *
+ *  Driver for 8250/16550-type serial ports
+ *
+ *  Based on drivers/char/serial.c, by Linus Torvalds, Theodore Ts'o.
+ *
+ *  Copyright (C) 2001 Russell King.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ *  $Id: 8250.h,v 1.8 2002/07/21 21:32:30 rmk Exp $
+ */
+
+#include <linux/config.h>
+
+void serial8250_get_irq_map(unsigned int *map);
+void serial8250_suspend_port(int line);
+void serial8250_resume_port(int line);
+
+/*
+ * This replaces serial_uart_config in include/linux/serial.h
+ */
+struct serial8250_config {
+	const char	*name;
+	unsigned int	fifo_size;
+	unsigned int	tx_loadsz;
+	unsigned int	flags;
+};
+
+#define UART_CAP_FIFO	(1 << 8)	/* UART has FIFO */
+#define UART_CAP_EFR	(1 << 9)	/* UART has EFR */
+#define UART_CAP_SLEEP	(1 << 10)	/* UART has IER sleep */
+
+/*
+ * Definition of a legacy serial port
+ */
+struct old_serial_port {
+	unsigned int uart;
+	unsigned int baud_base;
+	unsigned int port;
+	unsigned int irq;
+	unsigned int flags;
+	unsigned char hub6;
+	unsigned char io_type;
+	unsigned char *iomem_base;
+	unsigned short iomem_reg_shift;
+};
+
+#undef SERIAL_DEBUG_PCI
+
+#if defined(__i386__) && (defined(CONFIG_M386) || defined(CONFIG_M486))
+#define SERIAL_INLINE
+#endif
+  
+#ifdef SERIAL_INLINE
+#define _INLINE_ inline
+#else
+#define _INLINE_
+#endif
+
+#define PROBE_RSA	(1 << 0)
+#define PROBE_ANY	(~0)
+
+#define HIGH_BITS_OFFSET ((sizeof(long)-sizeof(int))*8)
+
+#ifdef CONFIG_SERIAL_8250_SHARE_IRQ
+#define SERIAL8250_SHARE_IRQS 1
+#else
+#define SERIAL8250_SHARE_IRQS 0
+#endif