Hi Chris,
On Fri, May 31, 2024 at 7:28 AM Chris Packham
<[email protected]> wrote:
> Like we do for charlcd, allow the configuration of the initial message
> on line-display devices.
>
> Signed-off-by: Chris Packham <[email protected]>
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 <[email protected]>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
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
On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
<[email protected]> wrote:
> On Fri, May 31, 2024 at 7:28 AM Chris Packham
> <[email protected]> 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 <[email protected]>
I believe you agree with leaving ifdeffery above.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Fri, May 31, 2024 at 10:16 AM Andy Shevchenko
<[email protected]> wrote:
> On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
> <[email protected]> wrote:
> > On Fri, May 31, 2024 at 7:28 AM Chris Packham
> > <[email protected]> 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 <[email protected]>
>
> 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-m68korg
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
On Fri, May 31, 2024 at 10:22:02AM +0200, Geert Uytterhoeven wrote:
> On Fri, May 31, 2024 at 10:16 AM Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, May 31, 2024 at 10:45 AM Geert Uytterhoeven
> > <[email protected]> wrote:
> > > On Fri, May 31, 2024 at 7:28 AM Chris Packham
> > > <[email protected]> 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 <[email protected]>
> >
> > 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
Hi Andy,
On Fri, May 31, 2024 at 3:43 PM Andy Shevchenko
<[email protected]> 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-m68korg
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
On Fri, May 31, 2024 at 10:12 PM Geert Uytterhoeven
<[email protected]> wrote:
> On Fri, May 31, 2024 at 3:43 PM Andy Shevchenko
> <[email protected]> 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