[patch] x86, serial: always probe for legacy COM ports

[patch] x86, serial: always probe for legacy COM ports

[Bjorn Helgaas]

Andrew, if you drop
    revert-x86-serial-convert-legacy-com-ports-to-platform-devices.patch
and apply this, we should have the previous behavior.  I think the
conversion to platform devices is still worthwhile.  Obviously, I intend
this as 2.6.23 material.


Always probe for serial ports at legacy addresses, even if we have PNPBIOS
or ACPI that should tell us about those ports.

We have no reports yet of defective firmware that completely omits ports.
However, Sébastien Dugué <sebastien.dugue@bull.net> reported that the
NEC Express5800/120Lh with Phoenix BIOS 6.0.5N52, released 7/12/2005,
reports COM2 first, then COM1 in the ACPI namespace.

8250_pnp currently registers devices in namespace order.  On this machine,
that makes ttyS0=COM2 and ttyS1=COM1, the reverse of what we want.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/arch/i386/kernel/legacy_serial.c
===================================================================
--- w.orig/arch/i386/kernel/legacy_serial.c	2007-07-27 10:57:41.000000000 -0600
+++ w/arch/i386/kernel/legacy_serial.c	2007-07-27 10:58:03.000000000 -0600
@@ -11,7 +11,6 @@
  */
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/pnp.h>
 #include <linux/serial_8250.h>
 
 /* Standard COM flags (except for COM4, because of the 8514 problem) */
@@ -48,15 +47,8 @@
 	},
 };
 
-static int force_legacy_probe;
-module_param_named(force, force_legacy_probe, bool, 0);
-MODULE_PARM_DESC(force, "Force legacy serial port probe");
-
 static int __init serial8250_x86_com_init(void)
 {
-	if (pnp_platform_devices && !force_legacy_probe)
-		return -ENODEV;
-
 	return platform_device_register(&x86_com_device);
 }
 
Index: w/Documentation/kernel-parameters.txt
===================================================================
--- w.orig/Documentation/kernel-parameters.txt	2007-07-27 10:58:12.000000000 -0600
+++ w/Documentation/kernel-parameters.txt	2007-07-27 10:58:19.000000000 -0600
@@ -859,11 +859,6 @@
 	lasi=		[HW,SCSI] PARISC LASI driver for the 53c700 chip
 			Format: addr:<io>,irq:<irq>
 
-	legacy_serial.force [HW,IA-32,X86-64]
-			Probe for COM ports at legacy addresses even
-			if PNPBIOS or ACPI should describe them.  This
-			is for working around firmware defects.
-
 	llsc*=		[IA64] See function print_params() in
 			arch/ia64/sn/kernel/llsc4.c.
 

Re: [patch] x86, serial: always probe for legacy COM ports

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Bjorn Helgaas wrote:
> Andrew, if you drop
>     revert-x86-serial-convert-legacy-com-ports-to-platform-devices.patch
> and apply this, we should have the previous behavior.  I think the
> conversion to platform devices is still worthwhile.  Obviously, I intend
> this as 2.6.23 material.
> 
> 
> Always probe for serial ports at legacy addresses, even if we have PNPBIOS
> or ACPI that should tell us about those ports.
> 
> We have no reports yet of defective firmware that completely omits ports.
> However, Sébastien Dugué <sebastien.dugue@bull.net> reported that the
> NEC Express5800/120Lh with Phoenix BIOS 6.0.5N52, released 7/12/2005,
> reports COM2 first, then COM1 in the ACPI namespace.
> 
> 8250_pnp currently registers devices in namespace order.  On this machine,
> that makes ttyS0=COM2 and ttyS1=COM1, the reverse of what we want.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

This is still incomplete, as repeatedly stated.  Here is the email, again.

	Jeff




[-- Attachment #2: Attached Message --]
[-- Type: message/rfc822, Size: 2056 bytes --]

From: Jeff Garzik <jeff@garzik.org>
To: Russell King <rmk@arm.linux.org.uk>
Cc: akpm@linux-foundation.org,  mm-commits@vger.kernel.org,  ambx1@neo.rr.com,  bjorn.helgaas@hp.com,  lenb@kernel.org,  mjg59@srcf.ucam.org,  sebastien.dugue@bull.net
Subject: Re: + revert-x86-serial-convert-legacy-com-ports-to-platform-devices.patch added to -mm tree
Date: Tue, 24 Jul 2007 19:21:27 -0400
Message-ID: <46A68977.50800@garzik.org>

Russell King wrote:
> So we shouldn't even try to remove the legacy "legacy port" probing in
> the serial driver like other platforms have done, which this patch minus
> those bits I mentioned actually achieves?

Move [to a new driver / location in tree]?  Sure.  As long as there are 
no probe order issues, module naming issues (I note distinct lack of 
MODULE_ALIAS for example) or other transitions issues.

Remove?  Definitely not.

Even if we apply your suggested solution, updates are still needed:
* fix kernel-parameters.txt

* address transition issues related to module naming.  other transition 
issues?  has anybody even given this any thought?

* no apparent module unload support (regression or just a bug?)

* add docs for x86/x86-64 users (or point to docs in the source). 
Overall this was a highly "stealthy" change over to the new platform 
driver, without much notice at all.  Not optimum, for a popular device 
on Linux's most popular platform.

I have no complaints about platform_device direction, certainly.  But 
this particular commit was incomplete, poorly described, and broke 
decade+ behavior.

	Jeff

Re: [patch] x86, serial: always probe for legacy COM ports

[Bjorn Helgaas]

On Friday 27 July 2007 12:05:51 pm Jeff Garzik wrote:
> Bjorn Helgaas wrote:
> > Andrew, if you drop
> >     revert-x86-serial-convert-legacy-com-ports-to-platform-devices.patch
> > and apply this, we should have the previous behavior.  I think the
> > conversion to platform devices is still worthwhile.  Obviously, I intend
> > this as 2.6.23 material.
> > 
> > 
> > Always probe for serial ports at legacy addresses, even if we have PNPBIOS
> > or ACPI that should tell us about those ports.
> > 
> > We have no reports yet of defective firmware that completely omits ports.
> > However, Sébastien Dugué <sebastien.dugue@bull.net> reported that the
> > NEC Express5800/120Lh with Phoenix BIOS 6.0.5N52, released 7/12/2005,
> > reports COM2 first, then COM1 in the ACPI namespace.
> > 
> > 8250_pnp currently registers devices in namespace order.  On this machine,
> > that makes ttyS0=COM2 and ttyS1=COM1, the reverse of what we want.
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> This is still incomplete, as repeatedly stated.  Here is the email, again.

OK, uncle!  I'll work on your helpful advice (thanks for that), but
it feels too risky for this stage of 2.6.23.  Maybe I can come up with
something for a future release.

Bjorn

Re: [patch] x86, serial: always probe for legacy COM ports

[Jeff Garzik]

Bjorn Helgaas wrote:
> On Friday 27 July 2007 12:05:51 pm Jeff Garzik wrote:
>> This is still incomplete, as repeatedly stated.  Here is the email, again.

> OK, uncle!  I'll work on your helpful advice (thanks for that), but
> it feels too risky for this stage of 2.6.23.  Maybe I can come up with
> something for a future release.

The changeset in the kernel is too risky _without_ the changes I described.

Hence the repeated use of the word "incomplete."

* The patch has transition issues, when you shuffle users from one 
device driver to another.  Existing change should not be deployed 
without ensuring breakage is unlikely.

* The new driver is missing module unload support.  Existing change 
should not be deployed without this.

* DOCUMENT the user visible changes.  Existing change should not be 
deployed without this.

	Jeff

Re: [patch] x86, serial: always probe for legacy COM ports

[Bjorn Helgaas]

On Friday 27 July 2007 02:21:55 pm Jeff Garzik wrote:
> Bjorn Helgaas wrote:
> > On Friday 27 July 2007 12:05:51 pm Jeff Garzik wrote:
> >> This is still incomplete, as repeatedly stated.  Here is the email, again.
> 
> > OK, uncle!  I'll work on your helpful advice (thanks for that), but
> > it feels too risky for this stage of 2.6.23.  Maybe I can come up with
> > something for a future release.
> 
> The changeset in the kernel is too risky _without_ the changes I described.

That's what I meant.  Andrew already has a revert patch in -mm, and
the safest path seems like putting the reversion in 2.6.23 and working
on your advice post-2.6.23.

Bjorn

Re: [patch] x86, serial: always probe for legacy COM ports

[Andrew Morton]

On Fri, 27 Jul 2007 14:26:30 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> On Friday 27 July 2007 02:21:55 pm Jeff Garzik wrote:
> > Bjorn Helgaas wrote:
> > > On Friday 27 July 2007 12:05:51 pm Jeff Garzik wrote:
> > >> This is still incomplete, as repeatedly stated.  Here is the email, again.
> > 
> > > OK, uncle!  I'll work on your helpful advice (thanks for that), but
> > > it feels too risky for this stage of 2.6.23.  Maybe I can come up with
> > > something for a future release.
> > 
> > The changeset in the kernel is too risky _without_ the changes I described.
> 
> That's what I meant.  Andrew already has a revert patch in -mm, and
> the safest path seems like putting the reversion in 2.6.23 and working
> on your advice post-2.6.23.
> 

ok, thanks, I'll pull that trigger.

Re: [patch] x86, serial: always probe for legacy COM ports

[Jeff Garzik]

Bjorn Helgaas wrote:
> On Friday 27 July 2007 02:21:55 pm Jeff Garzik wrote:
>> Bjorn Helgaas wrote:
>>> On Friday 27 July 2007 12:05:51 pm Jeff Garzik wrote:
>>>> This is still incomplete, as repeatedly stated.  Here is the email, again.

>>> OK, uncle!  I'll work on your helpful advice (thanks for that), but
>>> it feels too risky for this stage of 2.6.23.  Maybe I can come up with
>>> something for a future release.

>> The changeset in the kernel is too risky _without_ the changes I described.
> 
> That's what I meant.  Andrew already has a revert patch in -mm, and
> the safest path seems like putting the reversion in 2.6.23 and working
> on your advice post-2.6.23.

Ah, my apologies!

I read "I'll work on your advice, but it feels too risky" as describing 
my advice as too risky, rather than the changeset currently in the kernel.

IMO the changeset should be resubmitted as two changes, separating the 
probe behavior changes from the platform driver conversion.  They are 
two distinct changes, and that would allow git-bisect to identify the 
culprit easily if future breakage occurs.

	Jeff