2020-11-03 10:05:16

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH v6 00/25] Make charlcd device independent

From: Lars Poeschel <[email protected]>

This tries to make charlcd device independent. At the moment hd44780
device specific code is contained deep in charlcd. This moves this out
into a hd44780_common module, where the two hd44780 drivers we have at
the moment (hd44780 and panel) can use this from. The goal is that at
the end other drivers can use the charlcd interface.
I add one such driver for a modtronix lcd displau with the last patch.
I submitted this already some time ago, where the wish was so split
this into smaller chunks what I try to do with this patchset.

This is v6 of the patchset. I address a two review comments from Miguel.
I fixed the Kconfig menu of auxdisplay. It does now present a submenu
again. And I fixed some typos in the commit message of patch 23.

As a note to patch 23:
This might slightly change behaviour.
On hd44780 displays with one or two lines the previous implementation
did still write characters to the buffer of the display even if they are
currently not visible. The shift_display command could be used so set
the "viewing window" to a new position in the buffer and then you could
see the characters previously written.
This described behaviour does not work for hd44780 displays with more
than two display lines. There simply is not enough buffer.
So the behaviour was a bit inconsistens across different displays.
The new behaviour is to stop writing character at the end of a visible
line, even if there would be room in the buffer. This allows us to have
an easy implementation, that should behave equal on all supported
displays. This is not hd44780 hardware dependent anymore.

Link: https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/lkml/CANiq72kS-u_Xd_m+2CQVh-JCncPf1XNXrXAZ=4z+mze8fwv2kw@mail.gmail.com/

Changes in v6:
- patch 2: Fix Kconfig so that auxdisplay is now a submenu again
- patch 23: Fix some typos in commit message

Changes in v5:
- patch 1: Fix a commit message typo: of -> on
- patch 2: Remove some unnecessary newlines
- patch 8: Fix some typos
- patch 14: Fix commit message typo: it's -> its
- patch 15: this patch is squashed together from the former individual
hd44780_common_ function patches
- patch 16: combined two cleanup patches
- patch 17: I did previously undo commit 3f03b6498 which was a mistake.
This is now corrected.
- patch 24: Picked up Robs Reviewed-by
- patch 25: use hex_to_bin like in commit 3f03b6498 but for the lcd2s.c
file

Changes in v4:
- modtronix -> Modtronix in new lcd2s driver
- Kconfig: remove "default n" in new lcd2s driver

Changes in v3:
- Fix some typos in Kconfig stuff
- Fix kernel test robot reported error: Missed EXPORT_SYMBOL_GPL
- new patch to reduce display timeout
- better patch description to why not move cursor beyond end of a line
- Fixed make dt_binding_doc errors

Changes in v2:
- split whole patch into many smaller chunks
- device tree doc in yaml

Lars Poeschel (25):
auxdisplay: Use an enum for charlcd backlight on/off ops
auxdisplay: Introduce hd44780_common.[ch]
auxdisplay: Move hwidth and bwidth to struct hd44780_common
auxdisplay: Move ifwidth to struct hd44780_common
auxdisplay: Move write_data pointer to hd44780_common
auxdisplay: Move write_cmd pointers to hd44780 drivers
auxdisplay: Move addr out of charlcd_priv
auxdisplay: hd44780_common_print
auxdisplay: provide hd44780_common_gotoxy
auxdisplay: add home to charlcd_ops
auxdisplay: Move clear_display to hd44780_common
auxdisplay: make charlcd_backlight visible to hd44780_common
auxdisplay: Make use of enum for backlight on / off
auxdisplay: Move init_display to hd44780_common
auxdisplay: implement various hd44780_common_ functions
auxdisplay: cleanup unnecessary hd44780 code in charlcd
auxdisplay: Move char redefine code to hd44780_common
auxdisplay: Call charlcd_backlight in place
auxdisplay: hd44780_common: Reduce clear_display timeout
auxdisplay: hd44780: Remove clear_fast
auxdisplay: charlcd: replace last device specific stuff
auxdisplay: Change gotoxy calling interface
auxdisplay: charlcd: Do not print chars at end of line
auxdisplay: lcd2s DT binding doc
auxdisplay: add a driver for lcd2s character display

.../bindings/auxdisplay/modtronix,lcd2s.yaml | 58 +++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
drivers/auxdisplay/Kconfig | 30 ++
drivers/auxdisplay/Makefile | 2 +
drivers/auxdisplay/charlcd.c | 412 +++++-------------
drivers/auxdisplay/charlcd.h | 86 +++-
drivers/auxdisplay/hd44780.c | 120 +++--
drivers/auxdisplay/hd44780_common.c | 361 +++++++++++++++
drivers/auxdisplay/hd44780_common.h | 33 ++
drivers/auxdisplay/lcd2s.c | 403 +++++++++++++++++
drivers/auxdisplay/panel.c | 180 ++++----
11 files changed, 1237 insertions(+), 450 deletions(-)
create mode 100644 Documentation/devicetree/bindings/auxdisplay/modtronix,lcd2s.yaml
create mode 100644 drivers/auxdisplay/hd44780_common.c
create mode 100644 drivers/auxdisplay/hd44780_common.h
create mode 100644 drivers/auxdisplay/lcd2s.c

--
2.28.0


2020-11-04 13:32:57

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v6 00/25] Make charlcd device independent

On Tue, Nov 3, 2020 at 10:58 AM <[email protected]> wrote:
>
> Changes in v6:
> - patch 2: Fix Kconfig so that auxdisplay is now a submenu again
> - patch 23: Fix some typos in commit message

Thanks a lot for all the work, Lars. Queued in -next.

Cheers,
Miguel

2020-11-06 10:14:10

by Lars Poeschel

[permalink] [raw]
Subject: Re: [PATCH v6 00/25] Make charlcd device independent

On Wed, Nov 04, 2020 at 02:30:04PM +0100, Miguel Ojeda wrote:
> Thanks a lot for all the work, Lars. Queued in -next.

I got an email [1] with a report about a build failure in
hd44780_common. The fix is simple but I don't know the process from here
on. Should I post a v7 of the whole patchset or only a follow-up patch
for the fix ?

Regards,
Lars

[1]: https://lore.kernel.org/linux-next/[email protected]/T/#u

2020-11-06 12:19:17

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v6 00/25] Make charlcd device independent

On Fri, Nov 6, 2020 at 11:11 AM Lars Poeschel <[email protected]> wrote:
>
> I got an email [1] with a report about a build failure in
> hd44780_common. The fix is simple but I don't know the process from here
> on. Should I post a v7 of the whole patchset or only a follow-up patch
> for the fix ?

Either would work (I can rebase it on my side). However, in order to
give credit to Randy, if the fix is integrated into a previous patch,
then I am not sure where we would put the Reported-by.

Randy, what people usually do for your reports on -next (or what do you prefer)?

Cheers,
Miguel

2020-11-06 16:38:05

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v6 00/25] Make charlcd device independent

On 11/6/20 4:17 AM, Miguel Ojeda wrote:
> On Fri, Nov 6, 2020 at 11:11 AM Lars Poeschel <[email protected]> wrote:
>>
>> I got an email [1] with a report about a build failure in
>> hd44780_common. The fix is simple but I don't know the process from here
>> on. Should I post a v7 of the whole patchset or only a follow-up patch
>> for the fix ?
>
> Either would work (I can rebase it on my side). However, in order to
> give credit to Randy, if the fix is integrated into a previous patch,
> then I am not sure where we would put the Reported-by.
>
> Randy, what people usually do for your reports on -next (or what do you prefer)?

Hi Miguel,

I'm not sure that I understand the question...

Include
Reported-by: Randy Dunlap <[email protected]>
if possible. If not, then don't. It's not a big deal.

Integrate the fix from Lars in whatever way works for you.

cheers. :)

--
~Randy

2020-11-09 09:34:56

by Lars Poeschel

[permalink] [raw]
Subject: [PATCH] auxdisplay: hd44780_common: Fix build error

From: Lars Poeschel <[email protected]>

When building the hd44780_common driver without a driver that actually
uses it like panel or hd44780 you got a build error, because
hd44780_common uses charlcd, but did not select it. It's users did
select it.
This is fixed now. hd4478_common now selects charlcd in Kconfig and
panel and hd44780 do not. They only select hd44780_common.

Reported-by: Randy Dunlap <[email protected]>
Signed-off-by: Lars Poeschel <[email protected]>
---
drivers/auxdisplay/Kconfig | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index a69623124a26..a2b59b84bb88 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -27,6 +27,7 @@ config CHARLCD

config HD44780_COMMON
tristate "Common functions for HD44780 (and compatibles) LCD displays" if COMPILE_TEST
+ select CHARLCD
help
This is a module with the common symbols for HD44780 (and compatibles)
displays. This is the code that multiple other modules use. It is not
@@ -37,7 +38,6 @@ config HD44780_COMMON
config HD44780
tristate "HD44780 Character LCD support"
depends on GPIOLIB || COMPILE_TEST
- select CHARLCD
select HD44780_COMMON
help
Enable support for Character LCDs using a HD44780 controller.
@@ -196,7 +196,6 @@ config ARM_CHARLCD
menuconfig PARPORT_PANEL
tristate "Parallel port LCD/Keypad Panel support"
depends on PARPORT
- select CHARLCD
select HD44780_COMMON
help
Say Y here if you have an HD44780 or KS-0074 LCD connected to your
--
2.28.0

2020-11-09 09:47:36

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] auxdisplay: hd44780_common: Fix build error

On Mon, Nov 9, 2020 at 10:32 AM <[email protected]> wrote:
>
> From: Lars Poeschel <[email protected]>
>
> When building the hd44780_common driver without a driver that actually
> uses it like panel or hd44780 you got a build error, because
> hd44780_common uses charlcd, but did not select it. It's users did
> select it.
> This is fixed now. hd4478_common now selects charlcd in Kconfig and
> panel and hd44780 do not. They only select hd44780_common.
>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: Lars Poeschel <[email protected]>

Thanks Lars, I'm picking it up.

Cheers,
Miguel

2020-11-09 09:55:27

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v6 00/25] Make charlcd device independent

Hi Randy,

On Fri, Nov 6, 2020 at 5:35 PM Randy Dunlap <[email protected]> wrote:
>
> I'm not sure that I understand the question...
>
> Include
> Reported-by: Randy Dunlap <[email protected]>
> if possible. If not, then don't. It's not a big deal.
>
> Integrate the fix from Lars in whatever way works for you.

Thanks Randy -- I asked Stephen Rothwell and he told me even for -next
patches on top are preferred, unless the bug is bad enough. In which
case, the [] notation can still be used to give credit.

Cheers,
Miguel

2020-11-09 17:32:36

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] auxdisplay: hd44780_common: Fix build error

On 11/9/20 1:31 AM, [email protected] wrote:
> From: Lars Poeschel <[email protected]>
>
> When building the hd44780_common driver without a driver that actually
> uses it like panel or hd44780 you got a build error, because
> hd44780_common uses charlcd, but did not select it. It's users did
> select it.
> This is fixed now. hd4478_common now selects charlcd in Kconfig and
> panel and hd44780 do not. They only select hd44780_common.
>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: Lars Poeschel <[email protected]>

Acked-by: Randy Dunlap <[email protected]> # build-tested

Thanks.

> ---
> drivers/auxdisplay/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
> index a69623124a26..a2b59b84bb88 100644
> --- a/drivers/auxdisplay/Kconfig
> +++ b/drivers/auxdisplay/Kconfig
> @@ -27,6 +27,7 @@ config CHARLCD
>
> config HD44780_COMMON
> tristate "Common functions for HD44780 (and compatibles) LCD displays" if COMPILE_TEST
> + select CHARLCD
> help
> This is a module with the common symbols for HD44780 (and compatibles)
> displays. This is the code that multiple other modules use. It is not
> @@ -37,7 +38,6 @@ config HD44780_COMMON
> config HD44780
> tristate "HD44780 Character LCD support"
> depends on GPIOLIB || COMPILE_TEST
> - select CHARLCD
> select HD44780_COMMON
> help
> Enable support for Character LCDs using a HD44780 controller.
> @@ -196,7 +196,6 @@ config ARM_CHARLCD
> menuconfig PARPORT_PANEL
> tristate "Parallel port LCD/Keypad Panel support"
> depends on PARPORT
> - select CHARLCD
> select HD44780_COMMON
> help
> Say Y here if you have an HD44780 or KS-0074 LCD connected to your
>


--
~Randy