Received-SPF: pass (google.com: domain of linux-kernel+bounces-66594-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240212170423.2860895-1-andriy.shevchenko@linux.intel.com> <20240212170423.2860895-8-andriy.shevchenko@linux.intel.com>
In-Reply-To: <20240212170423.2860895-8-andriy.shevchenko@linux.intel.com>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Thu, 15 Feb 2024 11:05:00 +0100
Message-ID: <CAMuHMdW=gTH12vLALAU_ioCKX9E2mJ3uR+8Q6GT0brc4yg83_A@mail.gmail.com>
Subject: Re: [PATCH v2 07/15] auxdisplay: linedisp: Group line display drivers together
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Andy Shevchenko <andy@kernel.org>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Robin van der Gracht <robin@protonic.nl>, Paul Burton <paulburton@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Andy,

On Mon, Feb 12, 2024 at 6:04=E2=80=AFPM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> For better usability group the line display drivers together in Kconfig
> and Makefile.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks for your patch!

> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -25,12 +25,6 @@ config CHARLCD
>           This is some character LCD core interface that multiple drivers=
 can
>           use.
>
> -config LINEDISP
> -       tristate "Character line display core support" if COMPILE_TEST
> -       help
> -         This is the core support for single-line character displays, to=
 be
> -         selected by drivers that use it.
> -
>  config HD44780_COMMON
>         tristate "Common functions for HD44780 (and compatibles) LCD disp=
lays" if COMPILE_TEST
>         select CHARLCD
> @@ -52,6 +46,16 @@ config HD44780
>           kernel and started at boot.
>           If you don't understand what all this is about, say N.
>
> +config LCD2S
> +       tristate "lcd2s 20x4 character display over I2C console"
> +       depends on I2C
> +       select CHARLCD
> +       help
> +         This is a driver that lets you use the lcd2s 20x4 character dis=
play
> +         from Modtronix engineering as a console output device. The disp=
lay
> +         is a simple single color character display. You have to connect=
 it
> +         to an I2C bus.
> +

I think it would be good to have "# <display type> section" comments,
to make the grouping clear.
Else I wonder why "L" is sorted before "K" ;-)

>  config KS0108
>         tristate "KS0108 LCD Controller"
>         depends on PARPORT_PC
> @@ -153,6 +157,12 @@ config CFAG12864B_RATE
>           If you compile this as a module, you can still override this
>           value using the module parameters.
>
> +config LINEDISP
> +       tristate "Character line display core support" if COMPILE_TEST
> +       help
> +         This is the core support for single-line character displays, to=
 be
> +         selected by drivers that use it.
> +
>  config IMG_ASCII_LCD
>         tristate "Imagination Technologies ASCII LCD Display"
>         depends on HAS_IOMEM
> @@ -177,16 +187,6 @@ config HT16K33
>           Say yes here to add support for Holtek HT16K33, RAM mapping 16*=
8
>           LED controller driver with keyscan.
>
> -config LCD2S
> -       tristate "lcd2s 20x4 character display over I2C console"
> -       depends on I2C
> -       select CHARLCD
> -       help
> -         This is a driver that lets you use the lcd2s 20x4 character dis=
play
> -         from Modtronix engineering as a console output device. The disp=
lay
> -         is a simple single color character display. You have to connect=
 it
> -         to an I2C bus.
> -
>  config ARM_CHARLCD
>         bool "ARM Ltd. Character LCD Driver"
>         depends on PLAT_VERSATILE

Shouldn't this (and PARPORT_PANEL and friends) be moved up, to the
character LCD section?

> diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
> index 6968ed4d3f0a..43bad850481c 100644
> --- a/drivers/auxdisplay/Makefile
> +++ b/drivers/auxdisplay/Makefile
> @@ -5,12 +5,12 @@
>
>  obj-$(CONFIG_CHARLCD)          +=3D charlcd.o
>  obj-$(CONFIG_HD44780_COMMON)   +=3D hd44780_common.o
> -obj-$(CONFIG_ARM_CHARLCD)      +=3D arm-charlcd.o
> +obj-$(CONFIG_HD44780)          +=3D hd44780.o
> +obj-$(CONFIG_LCD2S)            +=3D lcd2s.o
>  obj-$(CONFIG_KS0108)           +=3D ks0108.o
>  obj-$(CONFIG_CFAG12864B)       +=3D cfag12864b.o cfag12864bfb.o
> -obj-$(CONFIG_IMG_ASCII_LCD)    +=3D img-ascii-lcd.o
> -obj-$(CONFIG_HD44780)          +=3D hd44780.o
> -obj-$(CONFIG_HT16K33)          +=3D ht16k33.o
> -obj-$(CONFIG_PARPORT_PANEL)    +=3D panel.o
> -obj-$(CONFIG_LCD2S)            +=3D lcd2s.o
>  obj-$(CONFIG_LINEDISP)         +=3D line-display.o
> +obj-$(CONFIG_IMG_ASCII_LCD)    +=3D img-ascii-lcd.o
> +obj-$(CONFIG_HT16K33)          +=3D ht16k33.o
> +obj-$(CONFIG_ARM_CHARLCD)      +=3D arm-charlcd.o
> +obj-$(CONFIG_PARPORT_PANEL)    +=3D panel.o

IMHO it hurts to not sort Makefile entries alphabetically.

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. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds