devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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

* [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

* [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

* 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

* 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

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).