* [PATCH 0/3] drivers/auxdisplay: Provide support for JHD1313
@ 2019-11-28 10:55 kbingham
2019-11-28 10:55 ` [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor kbingham
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: kbingham @ 2019-11-28 10:55 UTC (permalink / raw)
To: linux-kernel, devicetree, Rob Herring, Miguel Ojeda Sandonis,
Simon Goda
Cc: Kieran Bingham
From: Kieran Bingham <kbingham@kernel.org>
The JHD1313 is a 16x2 LCD controller with an I2C interface. It is used in the
Seeed RGB Backlight LCD [0] which has the LCD at the I2C address 0x3e. (A
PCA9633 is also available at 0x62, to control the RGB backlight)
This series introduces a new Vendor prefix for JHD, and introduces bindings for
the LCD controller. A driver for the JHD1313 is added to the auxdisplay
subsystem providing a charlcd to control the display.
[0] http://wiki.seeedstudio.com/Grove-LCD_RGB_Backlight/
Because this interface is quite common, and generic - this could be potentially
extended to other similar devices later, possibly with optional bindings to
configure the display width and height. If so - perhaps a more generic naming
for the binding/driver might be appropriate at that time.
Kieran Bingham (3):
dt-bindings: vendor: Add JHD LCD vendor
dt-bindings: auxdisplay: Add JHD1313 bindings
drivers: auxdisplay: Add JHD1313 I2C interface driver
.../bindings/auxdisplay/jhd,jhd1313.yaml | 33 ++++++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 4 +
drivers/auxdisplay/Kconfig | 12 ++
drivers/auxdisplay/Makefile | 1 +
drivers/auxdisplay/jhd1313.c | 111 ++++++++++++++++++
6 files changed, 163 insertions(+)
create mode 100644 Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml
create mode 100644 drivers/auxdisplay/jhd1313.c
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor 2019-11-28 10:55 [PATCH 0/3] drivers/auxdisplay: Provide support for JHD1313 kbingham @ 2019-11-28 10:55 ` kbingham 2019-12-13 21:01 ` Rob Herring 2019-11-28 10:55 ` [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings kbingham 2019-11-28 10:55 ` [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver kbingham 2 siblings, 1 reply; 10+ messages in thread From: kbingham @ 2019-11-28 10:55 UTC (permalink / raw) To: linux-kernel, devicetree, Rob Herring, Miguel Ojeda Sandonis, Simon Goda Cc: Kieran Bingham From: Kieran Bingham <kbingham@kernel.org> Jing Handa Electronics is an LCD manufacturer based in Shenzhen, China. http://www.jhdlcd.com.cn/Company.html Signed-off-by: Kieran Bingham <kbingham@kernel.org> --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 05b3904a995b..5c671263a1f0 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -473,6 +473,8 @@ patternProperties: description: JEDEC Solid State Technology Association "^jesurun,.*": description: Shenzhen Jesurun Electronics Business Dept. + "^jhd,.*": + description: Jing Handa Electronics "^jianda,.*": description: Jiandangjing Technology Co., Ltd. "^karo,.*": -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor 2019-11-28 10:55 ` [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor kbingham @ 2019-12-13 21:01 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2019-12-13 21:01 UTC (permalink / raw) To: kbingham Cc: linux-kernel, devicetree, Miguel Ojeda Sandonis, Simon Goda, Kieran Bingham On Thu, 28 Nov 2019 10:55:06 +0000, kbingham@kernel.org wrote: > From: Kieran Bingham <kbingham@kernel.org> > > Jing Handa Electronics is an LCD manufacturer based in Shenzhen, China. > http://www.jhdlcd.com.cn/Company.html > > Signed-off-by: Kieran Bingham <kbingham@kernel.org> > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings 2019-11-28 10:55 [PATCH 0/3] drivers/auxdisplay: Provide support for JHD1313 kbingham 2019-11-28 10:55 ` [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor kbingham @ 2019-11-28 10:55 ` kbingham 2019-12-13 21:03 ` Rob Herring 2019-11-28 10:55 ` [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver kbingham 2 siblings, 1 reply; 10+ messages in thread From: kbingham @ 2019-11-28 10:55 UTC (permalink / raw) To: linux-kernel, devicetree, Rob Herring, Miguel Ojeda Sandonis, Simon Goda Cc: Kieran Bingham From: Kieran Bingham <kbingham@kernel.org> The JHD1313 is used in the Grove RGB LCD controller [0] and provides an I2C interface to control the LCD. The implementation is based upon the datasheet for the JHD1214 [1], as this is the only datasheet referenced by the documentation for the Grove part. [0] http://wiki.seeedstudio.com/Grove-LCD_RGB_Backlight/ [1] https://seeeddoc.github.io/Grove-LCD_RGB_Backlight/res/JHD1214Y_YG_1.0.pdf Signed-off-by: Simon Goda <simon.goda@doulos.com> Signed-off-by: Kieran Bingham <kbingham@kernel.org> --- .../bindings/auxdisplay/jhd,jhd1313.yaml | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml diff --git a/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml b/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml new file mode 100644 index 000000000000..b799a91846d2 --- /dev/null +++ b/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/auxdisplay/jhd,jhd1313.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: DT bindings for the JHD1313 Character LCD Controller + +description: + The JHD1313 Character LCD Controller is used by the widely available Grove + LCD RGB Backlight display. This currently supports only 16x2 LCD Modules. The + reg property specifies the I2C address of the module, and is expected to be + 0x3e. + +maintainers: + - Kieran Bingham <kbingham@kernel.org> + +properties: + compatible: + const: jhd,jhd1313 + + reg: true + +required: + - compatible + - reg + +examples: + - | + auxdisplay: lcd@3e { + compatible = "jhd,jhd1313"; + reg = <0x3e>; + }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings 2019-11-28 10:55 ` [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings kbingham @ 2019-12-13 21:03 ` Rob Herring 0 siblings, 0 replies; 10+ messages in thread From: Rob Herring @ 2019-12-13 21:03 UTC (permalink / raw) To: kbingham; +Cc: linux-kernel, devicetree, Miguel Ojeda Sandonis, Simon Goda On Thu, Nov 28, 2019 at 10:55:07AM +0000, kbingham@kernel.org wrote: > From: Kieran Bingham <kbingham@kernel.org> > > The JHD1313 is used in the Grove RGB LCD controller [0] and provides > an I2C interface to control the LCD. > > The implementation is based upon the datasheet for the JHD1214 [1], as > this is the only datasheet referenced by the documentation for the Grove > part. > > [0] http://wiki.seeedstudio.com/Grove-LCD_RGB_Backlight/ > [1] https://seeeddoc.github.io/Grove-LCD_RGB_Backlight/res/JHD1214Y_YG_1.0.pdf > > Signed-off-by: Simon Goda <simon.goda@doulos.com> > Signed-off-by: Kieran Bingham <kbingham@kernel.org> > --- > .../bindings/auxdisplay/jhd,jhd1313.yaml | 33 +++++++++++++++++++ > 1 file changed, 33 insertions(+) > create mode 100644 Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml > > diff --git a/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml b/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml > new file mode 100644 > index 000000000000..b799a91846d2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/auxdisplay/jhd,jhd1313.yaml > @@ -0,0 +1,33 @@ > +# SPDX-License-Identifier: GPL-2.0 Dual license new bindings please. (GPL-2.0-only OR BSD-2-Clause) With that, Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver 2019-11-28 10:55 [PATCH 0/3] drivers/auxdisplay: Provide support for JHD1313 kbingham 2019-11-28 10:55 ` [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor kbingham 2019-11-28 10:55 ` [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings kbingham @ 2019-11-28 10:55 ` kbingham 2019-11-28 13:43 ` Miguel Ojeda 2 siblings, 1 reply; 10+ messages in thread From: kbingham @ 2019-11-28 10:55 UTC (permalink / raw) To: linux-kernel, devicetree, Rob Herring, Miguel Ojeda Sandonis, Simon Goda Cc: Kieran Bingham From: Kieran Bingham <kbingham@kernel.org> Provide an auxdisplay driver for the JHD1313 as used by the Grove-LCD RGB Backlight module [0]. A datasheet for the JHD1214 is provided by seeed [1], which assumes that they are similar parts. The backlight for the Grove-LCD is already controllable with the PCA963x driver. [0] http://wiki.seeedstudio.com/Grove-LCD_RGB_Backlight/ [1] https://seeeddoc.github.io/Grove-LCD_RGB_Backlight/res/JHD1214Y_YG_1.0.pdf Signed-off-by: Simon Goda <simon.goda@doulos.com> Signed-off-by: Kieran Bingham <kbingham@kernel.org> --- MAINTAINERS | 4 ++ drivers/auxdisplay/Kconfig | 12 ++++ drivers/auxdisplay/Makefile | 1 + drivers/auxdisplay/jhd1313.c | 111 +++++++++++++++++++++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 drivers/auxdisplay/jhd1313.c 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 +M: Kieran Bingham <kbingham@kernel.org> +F: drivers/auxdisplay/jhd1313.c + JME NETWORK DRIVER M: Guo-Fu Tseng <cooldavid@cooldavid.org> L: netdev@vger.kernel.org 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. + config KS0108 tristate "KS0108 LCD Controller" depends on PARPORT_PC diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile index cf54b5efb07e..6e1405a61925 100644 --- a/drivers/auxdisplay/Makefile +++ b/drivers/auxdisplay/Makefile @@ -9,5 +9,6 @@ obj-$(CONFIG_KS0108) += ks0108.o obj-$(CONFIG_CFAG12864B) += cfag12864b.o cfag12864bfb.o obj-$(CONFIG_IMG_ASCII_LCD) += img-ascii-lcd.o obj-$(CONFIG_HD44780) += hd44780.o +obj-$(CONFIG_JHD1313) += jhd1313.o obj-$(CONFIG_HT16K33) += ht16k33.o obj-$(CONFIG_PARPORT_PANEL) += panel.o 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+ + +/* + * JHD1313 I2C Character LCD driver for Linux. + * + * Kieran Bingham <kbingham@kernel.org> + */ + +#include <linux/module.h> +#include <linux/mod_devicetable.h> + +#include <linux/i2c.h> + +#include "charlcd.h" + +struct jhd1313 { + struct i2c_client *client; +}; + +static void jhd1313_write_cmd(struct charlcd *lcd, int cmd) +{ + struct jhd1313 *jhd = lcd->drvdata; + struct i2c_client *client = jhd->client; + + i2c_smbus_write_byte_data(client, 0x00, cmd); +} + +static void jhd1313_write_data(struct charlcd *lcd, int data) +{ + struct jhd1313 *jhd = lcd->drvdata; + struct i2c_client *client = jhd->client; + + i2c_smbus_write_byte_data(client, 0x40, data); +} + +static const struct charlcd_ops jhd1313_ops = { + .write_cmd = jhd1313_write_cmd, + .write_data = jhd1313_write_data, +}; + +static int jhd1313_probe(struct i2c_client *client) +{ + struct charlcd *lcd; + struct jhd1313 *jhd; + int ret; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) { + dev_err(&client->dev, "i2c_check_functionality error\n"); + return -EIO; + } + + lcd = charlcd_alloc(sizeof(struct jhd1313)); + if (!lcd) + return -ENOMEM; + + jhd = lcd->drvdata; + i2c_set_clientdata(client, lcd); + jhd->client = client; + + lcd->width = 16; + lcd->height = 2; + lcd->ifwidth = 8; + lcd->ops = &jhd1313_ops; + + ret = charlcd_register(lcd); + if (ret) { + charlcd_free(lcd); + dev_err(&client->dev, "Failed to register JHD1313"); + } + + return ret; +} + +static int jhd1313_remove(struct i2c_client *client) +{ + struct charlcd *lcd = i2c_get_clientdata(client); + + charlcd_unregister(lcd); + charlcd_free(lcd); + + return 0; +} + +static const struct i2c_device_id jhd1313_id[] = { + { "jhd1313", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(i2c, jhd1313_id); + +static const struct of_device_id jhd1313_of_table[] = { + { .compatible = "jhd,jhd1313" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, jhd1313_of_table); + +static struct i2c_driver jhd1313_driver = { + .driver = { + .name = "jhd1313", + .of_match_table = jhd1313_of_table, + }, + .probe_new = jhd1313_probe, + .remove = jhd1313_remove, + .id_table = jhd1313_id, +}; + +module_i2c_driver(jhd1313_driver); + +MODULE_DESCRIPTION("JHD1313 I2C Character LCD driver"); +MODULE_AUTHOR("Kieran Bingham <kbingham@kernel.org>"); +MODULE_LICENSE("GPL"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver 2019-11-28 10:55 ` [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver kbingham @ 2019-11-28 13:43 ` Miguel Ojeda 2019-11-28 14:08 ` Kieran Bingham 0 siblings, 1 reply; 10+ messages in thread From: Miguel Ojeda @ 2019-11-28 13:43 UTC (permalink / raw) To: kbingham; +Cc: linux-kernel, devicetree, Rob Herring, Simon Goda Hi Kieran, On Thu, Nov 28, 2019 at 11:55 AM <kbingham@kernel.org> 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). Also missing the "S:" entry. > 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). > 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. Thanks for the driver! Cheers, Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver 2019-11-28 13:43 ` Miguel Ojeda @ 2019-11-28 14:08 ` Kieran Bingham 2019-11-28 14:34 ` Kieran Bingham 2019-11-28 14:37 ` Miguel Ojeda 0 siblings, 2 replies; 10+ messages in thread From: Kieran Bingham @ 2019-11-28 14:08 UTC (permalink / raw) To: Miguel Ojeda; +Cc: linux-kernel, devicetree, Rob Herring, Simon Goda Hi Miguel, On 28/11/2019 13:43, Miguel Ojeda wrote: > Hi Kieran, > > On Thu, Nov 28, 2019 at 11:55 AM <kbingham@kernel.org> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver 2019-11-28 14:08 ` Kieran Bingham @ 2019-11-28 14:34 ` Kieran Bingham 2019-11-28 14:37 ` Miguel Ojeda 1 sibling, 0 replies; 10+ messages in thread From: Kieran Bingham @ 2019-11-28 14:34 UTC (permalink / raw) To: Miguel Ojeda; +Cc: linux-kernel, devicetree, Rob Herring, Simon Goda Hello again, On 28/11/2019 14:08, Kieran Bingham wrote: > Hi Miguel, > > On 28/11/2019 13:43, Miguel Ojeda wrote: >> Hi Kieran, >> >> On Thu, Nov 28, 2019 at 11:55 AM <kbingham@kernel.org> 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 Following the rabbit-hole even deeper, I have now discovered that Seed have now put up a /real/ JHD1313 datasheet: https://github.com/SeeedDocument/Grove_LCD_RGB_Backlight/blob/master/res/JHD1313%20FP-RGB-1%201.4.pdf However, that leads down a further path - as it references in the block diagram on page 12 that the I2C component (which is really what this driver is about) is an AIP31068L (or equivalence) which handles the bridging between the I2C interface, and the segment driver. (And this is yet again, another newhaven-display part) So that's pushing me towards the idea that this is in fact a newhaven,aip31068l device driver. Perhaps we would support compatibles such as "jhd,jhd1313", "jhd,jhd1214" as convenience aliases though ? Any thoughts from anyone? > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver 2019-11-28 14:08 ` Kieran Bingham 2019-11-28 14:34 ` Kieran Bingham @ 2019-11-28 14:37 ` Miguel Ojeda 1 sibling, 0 replies; 10+ messages in thread From: Miguel Ojeda @ 2019-11-28 14:37 UTC (permalink / raw) To: kbingham; +Cc: linux-kernel, devicetree, Rob Herring, Simon Goda On Thu, Nov 28, 2019 at 3:08 PM Kieran Bingham <kbingham@kernel.org> wrote: > > Do you have any experience in these various part numbers, to suggest > perhaps a better naming? Not at all, sorry :( > Anyway, certainly adding in I2C would be beneficial though. I'd go with whatever you think will be the best for whoever reads the option in the config. It is not a big deal in any case -- whoever wants to use this kind of drivers have to know what they are looking for :) Cheers, Miguel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-13 21:03 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-28 10:55 [PATCH 0/3] drivers/auxdisplay: Provide support for JHD1313 kbingham 2019-11-28 10:55 ` [PATCH 1/3] dt-bindings: vendor: Add JHD LCD vendor kbingham 2019-12-13 21:01 ` Rob Herring 2019-11-28 10:55 ` [PATCH 2/3] dt-bindings: auxdisplay: Add JHD1313 bindings kbingham 2019-12-13 21:03 ` Rob Herring 2019-11-28 10:55 ` [PATCH 3/3] drivers: auxdisplay: Add JHD1313 I2C interface driver kbingham 2019-11-28 13:43 ` Miguel Ojeda 2019-11-28 14:08 ` Kieran Bingham 2019-11-28 14:34 ` Kieran Bingham 2019-11-28 14:37 ` Miguel Ojeda
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).