[PATCH] auxdisplay: linedisp: Support configuring the boot message

Chris Packham posted 1 patch 1 year, 6 months ago
drivers/auxdisplay/Kconfig        |  2 +-
drivers/auxdisplay/line-display.c | 10 +++++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
[PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Chris Packham 1 year, 6 months ago
Like we do for charlcd, allow the configuration of the initial message
on line-display devices.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/auxdisplay/Kconfig        |  2 +-
 drivers/auxdisplay/line-display.c | 10 +++++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 69d2138d7efb..21545ffba065 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -316,7 +316,7 @@ endif # PARPORT_PANEL
 
 config PANEL_CHANGE_MESSAGE
 	bool "Change LCD initialization message ?"
-	depends on CHARLCD
+	depends on CHARLCD || LINEDISP
 	help
 	  This allows you to replace the boot message indicating the kernel version
 	  and the driver version with a custom message. This is useful on appliances
diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index e2b546210f8d..837ca63c8368 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -8,7 +8,9 @@
  * Copyright (C) 2021 Glider bv
  */
 
+#ifndef CONFIG_PANEL_BOOT_MESSAGE
 #include <generated/utsrelease.h>
+#endif
 
 #include <linux/container_of.h>
 #include <linux/device.h>
@@ -312,6 +314,12 @@ static int linedisp_init_map(struct linedisp *linedisp)
 	return 0;
 }
 
+#ifdef CONFIG_PANEL_BOOT_MESSAGE
+#define LINE_DISP_INIT_TEXT CONFIG_PANEL_BOOT_MESSAGE
+#else
+#define LINE_DISP_INIT_TEXT "Linux " UTS_RELEASE "       "
+#endif
+
 /**
  * linedisp_register - register a character line display
  * @linedisp: pointer to character line display structure
@@ -359,7 +367,7 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 		goto out_del_timer;
 
 	/* display a default message */
-	err = linedisp_display(linedisp, "Linux " UTS_RELEASE "       ", -1);
+	err = linedisp_display(linedisp, LINE_DISP_INIT_TEXT, -1);
 	if (err)
 		goto out_del_dev;
 
-- 
2.45.1
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Fri, May 31, 2024 at 11:20:54AM +1200, Chris Packham wrote:
> Like we do for charlcd, allow the configuration of the initial message
> on line-display devices.

Pushed to my review and testing queue, thanks!

I tweaked the define to be LINEDISP as Geert suggested.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Chris Packham 1 year, 6 months ago
On 1/06/24 03:35, Andy Shevchenko wrote:
> On Fri, May 31, 2024 at 11:20:54AM +1200, Chris Packham wrote:
>> Like we do for charlcd, allow the configuration of the initial message
>> on line-display devices.
> Pushed to my review and testing queue, thanks!
>
> I tweaked the define to be LINEDISP as Geert suggested.
>
Actually did you? I just checked what's in 
andy/linux-auxdisplay/review-andy and it still uses LINE_DISP.
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Tue, Jun 4, 2024 at 12:59 AM Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
>
>
> On 1/06/24 03:35, Andy Shevchenko wrote:
> > On Fri, May 31, 2024 at 11:20:54AM +1200, Chris Packham wrote:
> >> Like we do for charlcd, allow the configuration of the initial message
> >> on line-display devices.
> > Pushed to my review and testing queue, thanks!
> >
> > I tweaked the define to be LINEDISP as Geert suggested.
> >
> Actually did you? I just checked what's in
> andy/linux-auxdisplay/review-andy and it still uses LINE_DISP.

Oh, it seems I forgot to squash the change, thank you for the catch!


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Tue, Jun 4, 2024 at 10:27 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jun 4, 2024 at 12:59 AM Chris Packham
> <Chris.Packham@alliedtelesis.co.nz> wrote:
> > On 1/06/24 03:35, Andy Shevchenko wrote:
> > > On Fri, May 31, 2024 at 11:20:54AM +1200, Chris Packham wrote:
> > >> Like we do for charlcd, allow the configuration of the initial message
> > >> on line-display devices.
> > > Pushed to my review and testing queue, thanks!
> > >
> > > I tweaked the define to be LINEDISP as Geert suggested.
> > >
> > Actually did you? I just checked what's in
> > andy/linux-auxdisplay/review-andy and it still uses LINE_DISP.
>
> Oh, it seems I forgot to squash the change, thank you for the catch!

Now updated.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Chris Packham 1 year, 6 months ago
On 1/06/24 03:35, Andy Shevchenko wrote:
> On Fri, May 31, 2024 at 11:20:54AM +1200, Chris Packham wrote:
>> Like we do for charlcd, allow the configuration of the initial message
>> on line-display devices.
> Pushed to my review and testing queue, thanks!
>
> I tweaked the define to be LINEDISP as Geert suggested.

Thanks a lot. Enjoy your well earned vacation.
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Geert Uytterhoeven 1 year, 6 months ago
Hi Chris,

On Fri, May 31, 2024 at 7:28 AM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
> Like we do for charlcd, allow the configuration of the initial message
> on line-display devices.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Thanks for your patch!

> --- a/drivers/auxdisplay/line-display.c
> +++ b/drivers/auxdisplay/line-display.c
> @@ -8,7 +8,9 @@
>   * Copyright (C) 2021 Glider bv
>   */
>
> +#ifndef CONFIG_PANEL_BOOT_MESSAGE
>  #include <generated/utsrelease.h>
> +#endif

The #ifndef/#endif is not really needed.

>  #include <linux/container_of.h>
>  #include <linux/device.h>
> @@ -312,6 +314,12 @@ static int linedisp_init_map(struct linedisp *linedisp)
>         return 0;
>  }
>
> +#ifdef CONFIG_PANEL_BOOT_MESSAGE
> +#define LINE_DISP_INIT_TEXT CONFIG_PANEL_BOOT_MESSAGE

So the user has to add extra spaces at the end when needed.
This makes sense, as they are not always needed (e.g. when the length of
the message matches the display size, no scrolling is needed/wanted).
I have verified that Kconfig actually preserves such spaces.

Nit: this is the only definition having an underscore between "line"
and "disp".

> +#else
> +#define LINE_DISP_INIT_TEXT "Linux " UTS_RELEASE "       "
> +#endif

I'd rather move this up, next to the other definitions at the top of
the file.

> +
>  /**
>   * linedisp_register - register a character line display
>   * @linedisp: pointer to character line display structure

As I see no real deficiencies:
Reviewed-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
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, May 31, 2024 at 7:28 AM Chris Packham
> <chris.packham@alliedtelesis.co.nz> wrote:
> > Like we do for charlcd, allow the configuration of the initial message
> > on line-display devices.

...

> > +#ifndef CONFIG_PANEL_BOOT_MESSAGE
> >  #include <generated/utsrelease.h>
> > +#endif
>
> The #ifndef/#endif is not really needed.

It's needed to avoid unnecessary build of the module (in case of m).



> As I see no real deficiencies:
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

I believe you agree with leaving ifdeffery above.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Geert Uytterhoeven 1 year, 6 months ago
Hi Andy,

On Fri, May 31, 2024 at 10:16 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Fri, May 31, 2024 at 7:28 AM Chris Packham
> > <chris.packham@alliedtelesis.co.nz> wrote:
> > > Like we do for charlcd, allow the configuration of the initial message
> > > on line-display devices.
>
> ...
>
> > > +#ifndef CONFIG_PANEL_BOOT_MESSAGE
> > >  #include <generated/utsrelease.h>
> > > +#endif
> >
> > The #ifndef/#endif is not really needed.
>
> It's needed to avoid unnecessary build of the module (in case of m).

OK.

> > As I see no real deficiencies:
> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> I believe you agree with leaving ifdeffery above.

Thanks, I agree to agree ;-)

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] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Fri, May 31, 2024 at 10:22:02AM +0200, Geert Uytterhoeven wrote:
> On Fri, May 31, 2024 at 10:16 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Fri, May 31, 2024 at 7:28 AM Chris Packham
> > > <chris.packham@alliedtelesis.co.nz> wrote:
> > > > Like we do for charlcd, allow the configuration of the initial message
> > > > on line-display devices.

...

> > > > +#ifndef CONFIG_PANEL_BOOT_MESSAGE
> > > >  #include <generated/utsrelease.h>
> > > > +#endif
> > >
> > > The #ifndef/#endif is not really needed.
> >
> > It's needed to avoid unnecessary build of the module (in case of m).
> 
> OK.
> 
> > > As I see no real deficiencies:
> > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> >
> > I believe you agree with leaving ifdeffery above.
> 
> Thanks, I agree to agree ;-)

Btw, I will take a long lasting vacations (ten weeks in a row) and most likely
won't be able to actively participate for this subsystem. Thinking about how
to proceed if something critical appears... Maybe you want a push access to the
same Git repo and in (rare) cases can handle fixes? We may ask Konstantin to
configure that on git.kernel.org.

P.S. This change doesn't seem to me as critical and there is still a chance
that I will have time to proceed, but the situation just motivated me to discuss
the possibilities.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message
Posted by Geert Uytterhoeven 1 year, 6 months ago
Hi Andy,

On Fri, May 31, 2024 at 3:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> Btw, I will take a long lasting vacations (ten weeks in a row) and most likely
> won't be able to actively participate for this subsystem. Thinking about how
> to proceed if something critical appears... Maybe you want a push access to the
> same Git repo and in (rare) cases can handle fixes? We may ask Konstantin to
> configure that on git.kernel.org.
>
> P.S. This change doesn't seem to me as critical and there is still a chance
> that I will have time to proceed, but the situation just motivated me to discuss
> the possibilities.

Thanks for the heads up!

Np, I can take over if there is something critical.
If it is temporary, I can also just create my own linux-auxdisplay.git at
kernel.org and ask Stephen and Linus to pull from that one?

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] auxdisplay: linedisp: Support configuring the boot message
Posted by Andy Shevchenko 1 year, 6 months ago
On Fri, May 31, 2024 at 10:12 PM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, May 31, 2024 at 3:43 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > Btw, I will take a long lasting vacations (ten weeks in a row) and most likely
> > won't be able to actively participate for this subsystem. Thinking about how
> > to proceed if something critical appears... Maybe you want a push access to the
> > same Git repo and in (rare) cases can handle fixes? We may ask Konstantin to
> > configure that on git.kernel.org.
> >
> > P.S. This change doesn't seem to me as critical and there is still a chance
> > that I will have time to proceed, but the situation just motivated me to discuss
> > the possibilities.
>
> Thanks for the heads up!
>
> Np, I can take over if there is something critical.
> If it is temporary, I can also just create my own linux-auxdisplay.git at
> kernel.org and ask Stephen and Linus to pull from that one?

It's temporary, but based on my experience the shared trees work well.

-- 
With Best Regards,
Andy Shevchenko