[PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Luca Tettamanti]

Hi Bjorn,
your patch:

commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26
Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
Date:   Tue May 8 00:36:07 2007 -0700

    x86, serial: convert legacy COM ports to platform devices

disables probing using legacy code in favour of ACPI/PNP to avoid double
discovery.

However if SERIAL_8250_PNP is not enabled the user is left without
serial ports (been there, done that - my kernel config suddenly wasn't
working anymore). So:

Enable probing of serial ports using legacy code when PNP code
(SERIAL_8250_PNP) is not enabled.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>

---

 arch/i386/kernel/legacy_serial.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/i386/kernel/legacy_serial.c b/arch/i386/kernel/legacy_serial.c
index 2151011..457d307 100644
--- a/arch/i386/kernel/legacy_serial.c
+++ b/arch/i386/kernel/legacy_serial.c
@@ -48,7 +48,11 @@ static struct platform_device x86_com_device = {
 	},
 };
 
+#ifndef CONFIG_SERIAL_8250_PNP
+static int force_legacy_probe = 1;
+#else
 static int force_legacy_probe;
+#endif
 module_param_named(force, force_legacy_probe, bool, 0);
 MODULE_PARM_DESC(force, "Force legacy serial port probe");
 


Luca
-- 
La vispa candela bruciava l'erbetta
creando un'essenza alquanto sospetta;
sembrava l'odore di una sacrestia
ma presto mi accorsi che era maria.

Re: [PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Bjorn Helgaas]

On Saturday 07 July 2007 05:33:00 pm Luca Tettamanti wrote:
> your patch:
> 
> commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26
> Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Date:   Tue May 8 00:36:07 2007 -0700
> 
>     x86, serial: convert legacy COM ports to platform devices
> 
> disables probing using legacy code in favour of ACPI/PNP to avoid double
> discovery.
> 
> However if SERIAL_8250_PNP is not enabled the user is left without
> serial ports (been there, done that - my kernel config suddenly wasn't
> working anymore). So:

Yes, that's true.  Kconfig should automatically turn on SERIAL_8250_PNP
if PNP and SERIAL_8250 are selected, but it does give you a choice if
you have EMBEDDED set.  So I guess you must have this:

  CONFIG_EMBEDDED=y
  CONFIG_PNP=y
  CONFIG_SERIAL_8250=y
  CONFIG_SERIAL_8250_PNP=n

Why do you want PNP support, but not 8250_PNP support?  If you turn off
PNP completely, you will have "pnp_platform_devices == 0", and the
existing code will do what you need.

> Enable probing of serial ports using legacy code when PNP code
> (SERIAL_8250_PNP) is not enabled.
> 
> Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
> 
> ---
> 
>  arch/i386/kernel/legacy_serial.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/i386/kernel/legacy_serial.c b/arch/i386/kernel/legacy_serial.c
> index 2151011..457d307 100644
> --- a/arch/i386/kernel/legacy_serial.c
> +++ b/arch/i386/kernel/legacy_serial.c
> @@ -48,7 +48,11 @@ static struct platform_device x86_com_device = {
>  	},
>  };
>  
> +#ifndef CONFIG_SERIAL_8250_PNP
> +static int force_legacy_probe = 1;
> +#else
>  static int force_legacy_probe;
> +#endif
>  module_param_named(force, force_legacy_probe, bool, 0);
>  MODULE_PARM_DESC(force, "Force legacy serial port probe");
>  
> 
> 
> Luca

Re: [PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Luca]

On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Saturday 07 July 2007 05:33:00 pm Luca Tettamanti wrote:
> > your patch:
> >
> > commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26
> > Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > Date:   Tue May 8 00:36:07 2007 -0700
> >
> >     x86, serial: convert legacy COM ports to platform devices
> >
> > disables probing using legacy code in favour of ACPI/PNP to avoid double
> > discovery.
> >
> > However if SERIAL_8250_PNP is not enabled the user is left without
> > serial ports (been there, done that - my kernel config suddenly wasn't
> > working anymore). So:
>
> Yes, that's true.  Kconfig should automatically turn on SERIAL_8250_PNP
> if PNP and SERIAL_8250 are selected, but it does give you a choice if
> you have EMBEDDED set.  So I guess you must have this:
>
>   CONFIG_EMBEDDED=y
>   CONFIG_PNP=y
>   CONFIG_SERIAL_8250=y
>   CONFIG_SERIAL_8250_PNP=n
>
> Why do you want PNP support, but not 8250_PNP support?

That's a good question :-) IIRC the serial PNP code used to mess up
the mode of the port on my laptop, making it useless for debugging the
boot. Bug was fixed long ago, but I didn't re-enabled that option.

> If you turn off
> PNP completely, you will have "pnp_platform_devices == 0", and the
> existing code will do what you need.

Well for me it's a non-issue, I'm able to "work-around" it in many
ways. My point is that a configuration that used to work and is still
valid suddenly doesn't work anymore. I missed the dependacy on
CONFIG_EMBEDDED though, so feel free to ignore the patch ;-)

Luca

Re: [PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Bjorn Helgaas]

On Monday 09 July 2007 11:30:59 am Luca wrote:
> On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Saturday 07 July 2007 05:33:00 pm Luca Tettamanti wrote:
> > > your patch:
> > >
> > > commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26
> > > Author: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > > Date:   Tue May 8 00:36:07 2007 -0700
> > >
> > >     x86, serial: convert legacy COM ports to platform devices
> > >
> > > disables probing using legacy code in favour of ACPI/PNP to avoid double
> > > discovery.
> > >
> > > However if SERIAL_8250_PNP is not enabled the user is left without
> > > serial ports (been there, done that - my kernel config suddenly wasn't
> > > working anymore). So:
> >
> > Yes, that's true.  Kconfig should automatically turn on SERIAL_8250_PNP
> > if PNP and SERIAL_8250 are selected, but it does give you a choice if
> > you have EMBEDDED set.  So I guess you must have this:
> >
> >   CONFIG_EMBEDDED=y
> >   CONFIG_PNP=y
> >   CONFIG_SERIAL_8250=y
> >   CONFIG_SERIAL_8250_PNP=n
> >
> > Why do you want PNP support, but not 8250_PNP support?
> 
> That's a good question :-) IIRC the serial PNP code used to mess up
> the mode of the port on my laptop, making it useless for debugging the
> boot. Bug was fixed long ago, but I didn't re-enabled that option.

It wouldn't surprise me if serial PNP can be tripped up by the
incomplete Linux PNP code or by buggy firmware.  If you find any
issues like that, I'll fix them.

> > If you turn off
> > PNP completely, you will have "pnp_platform_devices == 0", and the
> > existing code will do what you need.
> 
> Well for me it's a non-issue, I'm able to "work-around" it in many
> ways. My point is that a configuration that used to work and is still
> valid suddenly doesn't work anymore.

You're right about that, and it is a legitimate issue.  But given the
fact that (PNP && !SERIAL_8250_PNP) should be relatively unusual, my
opinion is that it's not worth extra ifdefs in legacy_serial.c.

If you have to use any workarounds like "legacy_serial.force", I think
that's a bug, and I'll fix it.

Bjorn

Re: [PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Luca]

On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Monday 09 July 2007 11:30:59 am Luca wrote:
> > On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > If you turn off
> > > PNP completely, you will have "pnp_platform_devices == 0", and the
> > > existing code will do what you need.
> >
> > Well for me it's a non-issue, I'm able to "work-around" it in many
> > ways. My point is that a configuration that used to work and is still
> > valid suddenly doesn't work anymore.
>
> You're right about that, and it is a legitimate issue.  But given the
> fact that (PNP && !SERIAL_8250_PNP) should be relatively unusual, my
> opinion is that it's not worth extra ifdefs in legacy_serial.c.
>
> If you have to use any workarounds like "legacy_serial.force", I think
> that's a bug, and I'll fix it.

Ok, I double-checked: the serial port is usable during boot with PNP
enabled, so no problem with dropping my patch.

Luca

Re: [PATCH] Enable legacy support for serial ports when SERIAL_8250_PNP is disabled

[Bjorn Helgaas]

On Wednesday 11 July 2007 01:40:07 pm Luca wrote:
> On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > On Monday 09 July 2007 11:30:59 am Luca wrote:
> > > On 7/9/07, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > > > If you turn off
> > > > PNP completely, you will have "pnp_platform_devices == 0", and the
> > > > existing code will do what you need.
> > >
> > > Well for me it's a non-issue, I'm able to "work-around" it in many
> > > ways. My point is that a configuration that used to work and is still
> > > valid suddenly doesn't work anymore.
> >
> > You're right about that, and it is a legitimate issue.  But given the
> > fact that (PNP && !SERIAL_8250_PNP) should be relatively unusual, my
> > opinion is that it's not worth extra ifdefs in legacy_serial.c.
> >
> > If you have to use any workarounds like "legacy_serial.force", I think
> > that's a bug, and I'll fix it.
> 
> Ok, I double-checked: the serial port is usable during boot with PNP
> enabled, so no problem with dropping my patch.

Thanks a lot for checking!

Bjorn