[PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Paul Bolle]

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) This patch is untested: I have neither the hardware nor the toolchain
needed. It should be correct (though it makes an already too long line
even longer). Nevertheless I think a proper solution is a patch that
drops this warning entirely. I've CC'd the m68k people for further
feedback.

1) If SERIAL_8250_HP300 is set but neither HPDCA nor HPAPCI are set we
end up with an elaborate nop, don't we? Initialization should always
fail in that case. So effectively SERIAL_8250_HP300 depends on HPDCA
and/or HPAPCI. Was there perhaps some problem in translating that
dependency into a Kconfig dependency?

2) Related question: is it useful to have both HPDCA and HPAPCI set?

 drivers/tty/serial/8250_hp300.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250_hp300.c b/drivers/tty/serial/8250_hp300.c
index c13438c..dc41fbb 100644
--- a/drivers/tty/serial/8250_hp300.c
+++ b/drivers/tty/serial/8250_hp300.c
@@ -21,7 +21,7 @@
 #include "8250.h"
 
 #if !defined(CONFIG_HPDCA) && !defined(CONFIG_HPAPCI)
-#warning CONFIG_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
+#warning CONFIG_SERIAL_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
 #endif
 
 #ifdef CONFIG_HPAPCI
-- 
1.7.4.4

Re: [PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Jiri Kosina]

On Mon, 7 Nov 2011, Paul Bolle wrote:

> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) This patch is untested: I have neither the hardware nor the toolchain
> needed. It should be correct (though it makes an already too long line
> even longer). Nevertheless I think a proper solution is a patch that
> drops this warning entirely. I've CC'd the m68k people for further
> feedback.
> 
> 1) If SERIAL_8250_HP300 is set but neither HPDCA nor HPAPCI are set we
> end up with an elaborate nop, don't we? Initialization should always
> fail in that case. So effectively SERIAL_8250_HP300 depends on HPDCA
> and/or HPAPCI. Was there perhaps some problem in translating that
> dependency into a Kconfig dependency?
> 
> 2) Related question: is it useful to have both HPDCA and HPAPCI set?
> 
>  drivers/tty/serial/8250_hp300.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250_hp300.c b/drivers/tty/serial/8250_hp300.c
> index c13438c..dc41fbb 100644
> --- a/drivers/tty/serial/8250_hp300.c
> +++ b/drivers/tty/serial/8250_hp300.c
> @@ -21,7 +21,7 @@
>  #include "8250.h"
>  
>  #if !defined(CONFIG_HPDCA) && !defined(CONFIG_HPAPCI)
> -#warning CONFIG_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
> +#warning CONFIG_SERIAL_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
>  #endif

What is the point of this warning anyway? Shouldn't everything necessary 
be taken care of by Kconfig rules?

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Paul Bolle]

On Tue, 2011-11-08 at 10:26 +0100, Jiri Kosina wrote:
> On Mon, 7 Nov 2011, Paul Bolle wrote:
> > 1) If SERIAL_8250_HP300 is set but neither HPDCA nor HPAPCI are set we
> > end up with an elaborate nop, don't we? Initialization should always
> > fail in that case. So effectively SERIAL_8250_HP300 depends on HPDCA
> > and/or HPAPCI. Was there perhaps some problem in translating that
> > dependency into a Kconfig dependency?

[...]

> > --- a/drivers/tty/serial/8250_hp300.c
> > +++ b/drivers/tty/serial/8250_hp300.c
> > @@ -21,7 +21,7 @@
> >  #include "8250.h"
> >  
> >  #if !defined(CONFIG_HPDCA) && !defined(CONFIG_HPAPCI)
> > -#warning CONFIG_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
> > +#warning CONFIG_SERIAL_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
> >  #endif
> 
> What is the point of this warning anyway? Shouldn't everything necessary 
> be taken care of by Kconfig rules?

That's exactly what I'm trying to find out with my questions (aimed at
the m68k people, see 1) above).


Paul Bolle

Re: [PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Geert Uytterhoeven]

On Tue, Nov 8, 2011 at 10:34, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Tue, 2011-11-08 at 10:26 +0100, Jiri Kosina wrote:
>> On Mon, 7 Nov 2011, Paul Bolle wrote:
>> > 1) If SERIAL_8250_HP300 is set but neither HPDCA nor HPAPCI are set we
>> > end up with an elaborate nop, don't we? Initialization should always
>> > fail in that case. So effectively SERIAL_8250_HP300 depends on HPDCA
>> > and/or HPAPCI. Was there perhaps some problem in translating that
>> > dependency into a Kconfig dependency?
>
> [...]
>
>> > --- a/drivers/tty/serial/8250_hp300.c
>> > +++ b/drivers/tty/serial/8250_hp300.c
>> > @@ -21,7 +21,7 @@
>> >  #include "8250.h"
>> >
>> >  #if !defined(CONFIG_HPDCA) && !defined(CONFIG_HPAPCI)
>> > -#warning CONFIG_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
>> > +#warning CONFIG_SERIAL_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
>> >  #endif
>>
>> What is the point of this warning anyway? Shouldn't everything necessary
>> be taken care of by Kconfig rules?
>
> That's exactly what I'm trying to find out with my questions (aimed at
> the m68k people, see 1) above).

I already had that on my todo list...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Geert Uytterhoeven]

Hi Paul,

On Tue, Nov 8, 2011 at 10:34, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Tue, 2011-11-08 at 10:26 +0100, Jiri Kosina wrote:
>> On Mon, 7 Nov 2011, Paul Bolle wrote:
>> > 1) If SERIAL_8250_HP300 is set but neither HPDCA nor HPAPCI are set we
>> > end up with an elaborate nop, don't we? Initialization should always
>> > fail in that case. So effectively SERIAL_8250_HP300 depends on HPDCA
>> > and/or HPAPCI. Was there perhaps some problem in translating that
>> > dependency into a Kconfig dependency?
>
> [...]
>
>> > --- a/drivers/tty/serial/8250_hp300.c
>> > +++ b/drivers/tty/serial/8250_hp300.c
>> > @@ -21,7 +21,7 @@
>> >  #include "8250.h"
>> >
>> >  #if !defined(CONFIG_HPDCA) && !defined(CONFIG_HPAPCI)
>> > -#warning CONFIG_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
>> > +#warning CONFIG_SERIAL_8250 defined but neither CONFIG_HPDCA nor CONFIG_HPAPCI defined, are you sure?
>> >  #endif
>>
>> What is the point of this warning anyway? Shouldn't everything necessary
>> be taken care of by Kconfig rules?
>
> That's exactly what I'm trying to find out with my questions (aimed at
> the m68k people, see 1) above).

I have this reminder in my local tree:

diff --git a/arch/m68k/Kconfig.devices b/arch/m68k/Kconfig.devices
index 6033f5d..354ce91 100644
--- a/arch/m68k/Kconfig.devices
+++ b/arch/m68k/Kconfig.devices
@@ -109,6 +109,7 @@ config MULTIFACE_III_TTY
          To compile this driver as a module, choose M here.

 config HPDCA
+       # FIXME Should be bool and select SERIAL_8250_HP300?
        tristate "HP DCA serial support"
        depends on DIO && SERIAL_8250
        help
@@ -116,6 +117,7 @@ config HPDCA
          machine, say Y here.

 config HPAPCI
+       # FIXME Should be bool and select SERIAL_8250_HP300?
        tristate "HP APCI serial support"
        depends on HP300 && SERIAL_8250 && EXPERIMENTAL
        help

However, if these two options are bool, SERIAL_8250_HP300 can never
be m anymore.

Ah, if we just kill SERIAL_8250_HP300, we can put in the Makefile:

obj-$(CONFIG_HPDCA) += 8250_hp300.o
obj-$(CONFIG_HPAPCI) += 8250_hp300.o

Does that make sense?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] [TRIVIAL] 8250_hp300: Fix warning typo 'CONFIG_8250'

[Paul Bolle]

On Wed, 2011-11-09 at 20:52 +0100, Geert Uytterhoeven wrote:
> I have this reminder in my local tree:
> 
> diff --git a/arch/m68k/Kconfig.devices b/arch/m68k/Kconfig.devices
> index 6033f5d..354ce91 100644
> --- a/arch/m68k/Kconfig.devices
> +++ b/arch/m68k/Kconfig.devices
> @@ -109,6 +109,7 @@ config MULTIFACE_III_TTY
>           To compile this driver as a module, choose M here.
> 
>  config HPDCA
> +       # FIXME Should be bool and select SERIAL_8250_HP300?
>         tristate "HP DCA serial support"
>         depends on DIO && SERIAL_8250
>         help
> @@ -116,6 +117,7 @@ config HPDCA
>           machine, say Y here.
> 
>  config HPAPCI
> +       # FIXME Should be bool and select SERIAL_8250_HP300?
>         tristate "HP APCI serial support"
>         depends on HP300 && SERIAL_8250 && EXPERIMENTAL
>         help
> 
> However, if these two options are bool, SERIAL_8250_HP300 can never
> be m anymore.

0) This might turn out not to be relevant, but are HPDCA and HPAPCI
exclusive?

> Ah, if we just kill SERIAL_8250_HP300, we can put in the Makefile:
> 
> obj-$(CONFIG_HPDCA) += 8250_hp300.o
> obj-$(CONFIG_HPAPCI) += 8250_hp300.o
> 
> Does that make sense?

1) Makefiles always make me nervous. What happens if both CONFIG_HPDCA
and CONFIG_HPAPCI are set?

2) This means upgrading both symbols to tristate, doesn't it? It seems
that someone could then end up with this driver builtin instead of as a
module on upgrade (eg, SERIAL_8250_HP300=m and HPDCA=y becomes just
HPDCA=y on upgrade). But I'm not sure whether that is how the config
tools handle symbol upgrades.

3) Does changing the SERIAL_8250_HP300 entry to
    config SERIAL_8250_HP300
            tristate
            depends on HP300 && (HPDCA || HPAPCI)
            default SERIAL_8250

work for you? That only changes the "depends on" line.

(If you make HPDCA depend on HP300, which it seems to do in practice,
the dependency on HP300 could also be dropped here.)


Paul Bolle