Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1737619 > unrolled thread

[PATCH] auxdisplay: fix broken menus

Started byRandy Dunlap <rdunlap@infradead.org>
First post2017-09-22 18:30 +0200
Last post2017-09-22 22:50 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] auxdisplay: fix broken menus Randy Dunlap <rdunlap@infradead.org> - 2017-09-22 18:30 +0200
    Re: [PATCH] auxdisplay: fix broken menus Geert Uytterhoeven <geert@linux-m68k.org> - 2017-09-22 20:20 +0200
      Re: [PATCH] auxdisplay: fix broken menus Randy Dunlap <rdunlap@infradead.org> - 2017-09-22 20:30 +0200
        Re: [PATCH] auxdisplay: fix broken menus Geert Uytterhoeven <geert@linux-m68k.org> - 2017-09-22 21:50 +0200
          Re: [PATCH] auxdisplay: fix broken menus Randy Dunlap <rdunlap@infradead.org> - 2017-09-22 22:10 +0200
            Re: [PATCH] auxdisplay: fix broken menus Geert Uytterhoeven <geert@linux-m68k.org> - 2017-09-22 22:20 +0200
              Re: [PATCH] auxdisplay: fix broken menus Randy Dunlap <rdunlap@infradead.org> - 2017-09-22 22:50 +0200

#1737619 — [PATCH] auxdisplay: fix broken menus

FromRandy Dunlap <rdunlap@infradead.org>
Date2017-09-22 18:30 +0200
Subject[PATCH] auxdisplay: fix broken menus
Message-ID<usyZZ-8kh-23@gated-at.bofh.it>
From: Randy Dunlap <rdunlap@infradead.org>

Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
CHARLCD Kconfig symbol above the "menuconfig" so that the menu
display is continuous.

Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)

Cc: stable@vger.kernel.org # v4.12
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 drivers/auxdisplay/Kconfig |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
+++ lnx-414-rc1/drivers/auxdisplay/Kconfig
@@ -5,6 +5,9 @@
 # Auxiliary display drivers configuration.
 #
 
+config CHARLCD
+	tristate "Character LCD core support" if COMPILE_TEST
+
 menuconfig AUXDISPLAY
 	bool "Auxiliary Display support"
 	---help---
@@ -13,9 +16,6 @@ menuconfig AUXDISPLAY
 
 	  If you say N, all options in this submenu will be skipped and disabled.
 
-config CHARLCD
-	tristate "Character LCD core support" if COMPILE_TEST
-
 if AUXDISPLAY
 
 config HD44780

[toc] | [next] | [standalone]

#1737725

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-09-22 20:20 +0200
Message-ID<usAIq-WQ-7@gated-at.bofh.it>
In reply to#1737619
Hi Randy,

On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
> display is continuous.
>
> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)

Oops, sorry for that.

> Cc: stable@vger.kernel.org # v4.12
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
>  drivers/auxdisplay/Kconfig |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
> @@ -5,6 +5,9 @@
>  # Auxiliary display drivers configuration.
>  #
>
> +config CHARLCD
> +       tristate "Character LCD core support" if COMPILE_TEST
> +

What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?

>  menuconfig AUXDISPLAY
>         bool "Auxiliary Display support"
>         ---help---
> @@ -13,9 +16,6 @@ menuconfig AUXDISPLAY
>
>           If you say N, all options in this submenu will be skipped and disabled.
>
> -config CHARLCD
> -       tristate "Character LCD core support" if COMPILE_TEST
> -
>  if AUXDISPLAY
>
>  config HD44780

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

[toc] | [prev] | [next] | [standalone]

#1737729

FromRandy Dunlap <rdunlap@infradead.org>
Date2017-09-22 20:30 +0200
Message-ID<usAS6-ZW-19@gated-at.bofh.it>
In reply to#1737725
On 09/22/17 11:15, Geert Uytterhoeven wrote:
> Hi Randy,
> 
> On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
>> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
>> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
>> display is continuous.
>>
>> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)
> 
> Oops, sorry for that.
> 
>> Cc: stable@vger.kernel.org # v4.12
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>>  drivers/auxdisplay/Kconfig |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
>> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
>> @@ -5,6 +5,9 @@
>>  # Auxiliary display drivers configuration.
>>  #
>>
>> +config CHARLCD
>> +       tristate "Character LCD core support" if COMPILE_TEST
>> +
> 
> What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?

That works.
But why?  PANEL also selects CHARLCD, so it isn't specific to AUXDISPLAY.

>>  menuconfig AUXDISPLAY
>>         bool "Auxiliary Display support"
>>         ---help---
>> @@ -13,9 +16,6 @@ menuconfig AUXDISPLAY
>>
>>           If you say N, all options in this submenu will be skipped and disabled.
>>
>> -config CHARLCD
>> -       tristate "Character LCD core support" if COMPILE_TEST
>> -
>>  if AUXDISPLAY
>>
>>  config HD44780


-- 
~Randy

[toc] | [prev] | [next] | [standalone]

#1737781

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-09-22 21:50 +0200
Message-ID<usC7v-1Fr-7@gated-at.bofh.it>
In reply to#1737729
Hi Randy,

On Fri, Sep 22, 2017 at 8:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 09/22/17 11:15, Geert Uytterhoeven wrote:
>> On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>> From: Randy Dunlap <rdunlap@infradead.org>
>>>
>>> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
>>> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
>>> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
>>> display is continuous.
>>>
>>> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)
>>
>> Oops, sorry for that.
>>
>>> Cc: stable@vger.kernel.org # v4.12
>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> ---
>>>  drivers/auxdisplay/Kconfig |    6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
>>> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
>>> @@ -5,6 +5,9 @@
>>>  # Auxiliary display drivers configuration.
>>>  #
>>>
>>> +config CHARLCD
>>> +       tristate "Character LCD core support" if COMPILE_TEST
>>> +
>>
>> What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?
>
> That works.
> But why?  PANEL also selects CHARLCD, so it isn't specific to AUXDISPLAY.

OK, I thought PANEL was under AUXDISPLAY, but apparently it isn't.
Perhaps it should be, just like ARM_CHARLCD?
I.e. move everything inside the big if AUXDISPLAY block?

>>>  menuconfig AUXDISPLAY
>>>         bool "Auxiliary Display support"
>>>         ---help---
>>> @@ -13,9 +16,6 @@ menuconfig AUXDISPLAY
>>>
>>>           If you say N, all options in this submenu will be skipped and disabled.
>>>
>>> -config CHARLCD
>>> -       tristate "Character LCD core support" if COMPILE_TEST
>>> -
>>>  if AUXDISPLAY
>>>
>>>  config HD44780

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

[toc] | [prev] | [next] | [standalone]

#1737804

FromRandy Dunlap <rdunlap@infradead.org>
Date2017-09-22 22:10 +0200
Message-ID<usCqT-21j-27@gated-at.bofh.it>
In reply to#1737781
On 09/22/17 12:46, Geert Uytterhoeven wrote:
> Hi Randy,
> 
> On Fri, Sep 22, 2017 at 8:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> On 09/22/17 11:15, Geert Uytterhoeven wrote:
>>> On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> From: Randy Dunlap <rdunlap@infradead.org>
>>>>
>>>> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
>>>> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
>>>> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
>>>> display is continuous.
>>>>
>>>> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)
>>>
>>> Oops, sorry for that.
>>>
>>>> Cc: stable@vger.kernel.org # v4.12
>>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>> ---
>>>>  drivers/auxdisplay/Kconfig |    6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
>>>> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
>>>> @@ -5,6 +5,9 @@
>>>>  # Auxiliary display drivers configuration.
>>>>  #
>>>>
>>>> +config CHARLCD
>>>> +       tristate "Character LCD core support" if COMPILE_TEST
>>>> +
>>>
>>> What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?
>>
>> That works.
>> But why?  PANEL also selects CHARLCD, so it isn't specific to AUXDISPLAY.
> 
> OK, I thought PANEL was under AUXDISPLAY, but apparently it isn't.
> Perhaps it should be, just like ARM_CHARLCD?
> I.e. move everything inside the big if AUXDISPLAY block?

ARM_CHARLCD does not depend on AUXDISPLAY either.

I wouldn't mind if all of them were inside the AUXDISPLAY if/endif
for menu cleanness, but AFAIK it's not necessary for code dependencies.

I was just working on a separate patch to change PANEL from config to
menuconfig. That makes the PANEL sub-menu presentation cleaner.

> 
>>>>  menuconfig AUXDISPLAY
>>>>         bool "Auxiliary Display support"
>>>>         ---help---
>>>> @@ -13,9 +16,6 @@ menuconfig AUXDISPLAY
>>>>
>>>>           If you say N, all options in this submenu will be skipped and disabled.
>>>>
>>>> -config CHARLCD
>>>> -       tristate "Character LCD core support" if COMPILE_TEST
>>>> -
>>>>  if AUXDISPLAY
>>>>
>>>>  config HD44780


-- 
~Randy

[toc] | [prev] | [next] | [standalone]

#1737836

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-09-22 22:20 +0200
Message-ID<usCAy-24P-7@gated-at.bofh.it>
In reply to#1737804
Hi Randy,

On Fri, Sep 22, 2017 at 10:08 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 09/22/17 12:46, Geert Uytterhoeven wrote:
>> On Fri, Sep 22, 2017 at 8:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>> On 09/22/17 11:15, Geert Uytterhoeven wrote:
>>>> On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>>> From: Randy Dunlap <rdunlap@infradead.org>
>>>>>
>>>>> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
>>>>> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
>>>>> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
>>>>> display is continuous.
>>>>>
>>>>> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)
>>>>
>>>> Oops, sorry for that.
>>>>
>>>>> Cc: stable@vger.kernel.org # v4.12
>>>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>>>> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>> ---
>>>>>  drivers/auxdisplay/Kconfig |    6 +++---
>>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
>>>>> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
>>>>> @@ -5,6 +5,9 @@
>>>>>  # Auxiliary display drivers configuration.
>>>>>  #
>>>>>
>>>>> +config CHARLCD
>>>>> +       tristate "Character LCD core support" if COMPILE_TEST
>>>>> +
>>>>
>>>> What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?
>>>
>>> That works.
>>> But why?  PANEL also selects CHARLCD, so it isn't specific to AUXDISPLAY.
>>
>> OK, I thought PANEL was under AUXDISPLAY, but apparently it isn't.
>> Perhaps it should be, just like ARM_CHARLCD?
>> I.e. move everything inside the big if AUXDISPLAY block?
>
> ARM_CHARLCD does not depend on AUXDISPLAY either.

Technically it's an auxiliary display, AFAIK.

> I wouldn't mind if all of them were inside the AUXDISPLAY if/endif
> for menu cleanness, but AFAIK it's not necessary for code dependencies.

Indeed.

> I was just working on a separate patch to change PANEL from config to
> menuconfig. That makes the PANEL sub-menu presentation cleaner.

OK. So let's make progress with your patch for now.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

[toc] | [prev] | [next] | [standalone]

#1737850

FromRandy Dunlap <rdunlap@infradead.org>
Date2017-09-22 22:50 +0200
Message-ID<usD3A-2i1-11@gated-at.bofh.it>
In reply to#1737836
On 09/22/17 13:17, Geert Uytterhoeven wrote:
> Hi Randy,
> 
> On Fri, Sep 22, 2017 at 10:08 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>> On 09/22/17 12:46, Geert Uytterhoeven wrote:
>>> On Fri, Sep 22, 2017 at 8:23 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>> On 09/22/17 11:15, Geert Uytterhoeven wrote:
>>>>> On Fri, Sep 22, 2017 at 6:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>>>> From: Randy Dunlap <rdunlap@infradead.org>
>>>>>>
>>>>>> Having the CHARLCD Kconfig symbol between "menuconfig AUXDISPLAY"
>>>>>> and "if AUXDISPLAY" breaks the AUXDISPLAY submenus, so move the
>>>>>> CHARLCD Kconfig symbol above the "menuconfig" so that the menu
>>>>>> display is continuous.
>>>>>>
>>>>>> Fixes: 39f8ea46724e (auxdisplay: charlcd: Extract character LCD core from misc/panel)
>>>>>
>>>>> Oops, sorry for that.
>>>>>
>>>>>> Cc: stable@vger.kernel.org # v4.12
>>>>>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>>>>>> Cc: Miguel Ojeda Sandonis <miguel.ojeda.sandonis@gmail.com>
>>>>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>>>>> ---
>>>>>>  drivers/auxdisplay/Kconfig |    6 +++---
>>>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> --- lnx-414-rc1.orig/drivers/auxdisplay/Kconfig
>>>>>> +++ lnx-414-rc1/drivers/auxdisplay/Kconfig
>>>>>> @@ -5,6 +5,9 @@
>>>>>>  # Auxiliary display drivers configuration.
>>>>>>  #
>>>>>>
>>>>>> +config CHARLCD
>>>>>> +       tristate "Character LCD core support" if COMPILE_TEST
>>>>>> +
>>>>>
>>>>> What about moving it inside (i.e. right below) the "if AUXDISPLAY" instead?
>>>>
>>>> That works.
>>>> But why?  PANEL also selects CHARLCD, so it isn't specific to AUXDISPLAY.
>>>
>>> OK, I thought PANEL was under AUXDISPLAY, but apparently it isn't.
>>> Perhaps it should be, just like ARM_CHARLCD?
>>> I.e. move everything inside the big if AUXDISPLAY block?
>>
>> ARM_CHARLCD does not depend on AUXDISPLAY either.
> 
> Technically it's an auxiliary display, AFAIK.

The commit seems to support your statement. Looks like we should put it inside
AUXDISPLAY then.

commit 00846a4425d373761c70cbe82cceb98d3d4da179
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Tue Apr 4 20:25:22 2017 +0300

    auxdisplay: Move arm-charlcd.c to drivers/auxdisplay folder
    
    It looks like arm-charlcd.c belongs to auxdisplay subsystem.

>> I wouldn't mind if all of them were inside the AUXDISPLAY if/endif
>> for menu cleanness, but AFAIK it's not necessary for code dependencies.
> 
> Indeed.
> 
>> I was just working on a separate patch to change PANEL from config to
>> menuconfig. That makes the PANEL sub-menu presentation cleaner.
> 
> OK. So let's make progress with your patch for now.
> 
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> 
> 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
> 


-- 
~Randy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web