Hi Miguel, On 28/11/2019 13:43, Miguel Ojeda wrote: > Hi Kieran, > > On Thu, Nov 28, 2019 at 11:55 AM <kbingham@xxxxxxxxxx> wrote: >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 8f075b866aaf..640f099ff7fb 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -8837,6 +8837,10 @@ S: Maintained >> F: Documentation/admin-guide/jfs.rst >> F: fs/jfs/ >> >> +JHD1313 LCD Dispaly driver > > Typo (and it should be all uppercase; and perhaps "Display" is not > needed given LCD is there; but see also comments on the title below). Good spot, and good point "Liquid Crystal Display Display" is a bit redundant. > Also missing the "S:" entry. Ah yes, I think we can add the following here: S: Maintained >> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig >> index b8313a04422d..cfc61c1abdee 100644 >> --- a/drivers/auxdisplay/Kconfig >> +++ b/drivers/auxdisplay/Kconfig >> @@ -27,6 +27,18 @@ config HD44780 >> kernel and started at boot. >> If you don't understand what all this is about, say N. >> >> +config JHD1313 >> + tristate "JHD1313 Character LCD support" >> + depends on I2C >> + select CHARLCD >> + ---help--- >> + Enable support for Character LCDs using a JHD1313 controller on I2C. >> + The LCD is accessible through the /dev/lcd char device (10, 156). >> + This code can either be compiled as a module, or linked into the >> + kernel and started at boot. >> + This supports the LCD panel on the Grove 16x2 LCD series. >> + If you don't understand what all this is about, say N. > > Would it be useful/worth for users to put "Grove series" and/or "I2C" > in the tristate title? (i.e. like the help section explains and also > like the MODULE_DESCRIPTION says). I have struggled with the difference between the definition of this driver (which supports a 'JHD1313') vs the 'product' that uses it (the Grove display), and I also suspect that as it's just an implementation of a more generic part, so I'm also contemplating renaming this. For instance, the products at: http://www.jhdlcd.com.cn/162character.html All seem to reference an SPLC780D controller, and have varying properties of backlight colour, and text colour. Thus I highly suspect that the JHD1313 is just a specific variant/implementation of the range which is utilised by the Grove LCD board. (which makes jhd1313 a bad name for this driver...) Do you have any experience in these various part numbers, to suggest perhaps a better naming? Or I wonder if anyone has any relevant contacts at either Seeed, or JHD or any other related part here... Now that I track down the SPLC780D, I see: https://www.newhavendisplay.com/app_notes/SPLC780D.pdf Which leads to 'yet another vendor', and actually I already see newhaven,.* in vendor-prefixes.yaml ... however this looks like it relates to just the 'LCD driver', and does not provide the I2C interface - so the device is of course more complex than a single part. Anyway, certainly adding in I2C would be beneficial though. >> diff --git a/drivers/auxdisplay/jhd1313.c b/drivers/auxdisplay/jhd1313.c >> new file mode 100644 >> index 000000000000..abf270e128ac >> --- /dev/null >> +++ b/drivers/auxdisplay/jhd1313.c >> @@ -0,0 +1,111 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> + > > Unconventional (AFAIK) empty line. Ooops, I can drop that one :-D > Thanks for the driver! You're welcome. The charlcd_ framework makes it easy to add an LCD over I2C, and I hope this can be useful to others. > Cheers, > Miguel -- Kieran
