Add pardata bus + tinydrm driver

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

---

  Documentation/driver-api/index.rst   |   1 +
  Documentation/driver-api/pardata.rst |  60 ++++++++
  MAINTAINERS                          |   9 ++
  drivers/Kconfig                      |   2 +
  drivers/Makefile                     |   1 +
  drivers/pardata/Kconfig              |  17 +++
  drivers/pardata/Makefile             |   5 +
  drivers/pardata/pardata.c            | 282 +++++++++++++++++++++++++++++++++++
  include/linux/pardata.h              | 138 +++++++++++++++++
  9 files changed, 515 insertions(+)

create mode 100644 Documentation/driver-api/pardata.rst create mode 100644 drivers/pardata/Kconfig create mode 100644 drivers/pardata/Makefile create mode 100644 drivers/pardata/pardata.c create mode 100644 include/linux/pardata.h

Documentation/driver-api/index.rst +1 -0

index 6d9f2f9fe20e..1808fca406ae 100644
@@ -41,6 +41,7 @@ available subsections can be seen below.
     miscellaneous
     w1
     rapidio
+   pardata
     s390-drivers
     vme
     80211/index

Documentation/driver-api/pardata.rst new file +60 -0

new file mode 100644
index 000000000000..a811f024a0fe
@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+Parallel Data Bus/Drivers
+=========================
+
+Displays may be connected using a simple parallel bus.
+This is often seen in embedded systems with a simple MCU, but is also
+used in Linux based systems to a small extent.
+
+The bus looks like this:
+
+.. code-block:: none
+
+       ----+
+           |  DB0-DB7 or DB4-DB7      +----
+           ===/========================
+           |  E - enable              |  D
+           ----------------------------  I
+        C  |  Reset                   |  S
+        P  ----------------------------  P
+        U  |  Read/Write (one or two) |  L
+           ----------------------------  A
+           |  RS - instruction/data   |  Y
+           ----------------------------
+           |                          +----
+       ----+
+
+There may be several devices connected to the bus with individual
+reset and read/write signals.
+Two types of interfaces are supported. 8800 with a single read/write signal,
+and 6800 with individual read/write signals.
+
+Display supported by the parallel data bus: `<https://www.sparkfun.com/products/710>`_
+
+The overall code structure is
+
+.. code-block:: none
+
+  platform_device [pardata bus]   <--- pardatabus [driver]
+       |
+       +-> device
+             |
+             +-> pardata_bus_data
+
+  pardata_device
+      |
+      +-> device
+            |
+            +->
+
+  pardatabus_bus
+
+
+API documentation
+=================
+.. kernel-doc:: include/linux/pardata.h
+   :internal:
+.. kernel-doc:: drivers/pardata/pardata.c
+   :internal:

MAINTAINERS +9 -0

index 96e98e206b0d..4ba7ff7c3e46 100644
@@ -10727,6 +10727,15 @@ L:	[email protected]
  S:	Maintained
  F:	drivers/platform/x86/panasonic-laptop.c
 
+PARALLEL DATA SUBSYSTEM
+M:	Sam Ravnborg <[email protected]>
+S:	Maintained
+F:	drivers/pardata/
+F:	include/linux/pardata.h
+F:	drivers/gpu/drm/tinydrm/pardata-dbi.c
+F:	include/drm/pardata-dbi.h
+F:	Documentation/driver-api/pardata.rst
+
  PARALLEL LCD/KEYPAD PANEL DRIVER
  M:	Willy Tarreau <[email protected]>
  M:	Ksenija Stanojevic <[email protected]>

drivers/Kconfig +2 -0

index 95b9ccc08165..b51b25aae9a5 100644
@@ -217,4 +217,6 @@ source "drivers/siox/Kconfig"
 
  source "drivers/slimbus/Kconfig"
 
+source "drivers/pardata/Kconfig"
+
  endmenu

drivers/Makefile +1 -0

index 24cd47014657..cfe7945f2f6b 100644
@@ -67,6 +67,7 @@ obj-$(CONFIG_CONNECTOR)		+= connector/
  obj-$(CONFIG_FB_I810)           += video/fbdev/i810/
  obj-$(CONFIG_FB_INTEL)          += video/fbdev/intelfb/
 
+obj-$(CONFIG_PARDATA)		+= pardata/
  obj-$(CONFIG_PARPORT)		+= parport/
  obj-$(CONFIG_NVM)		+= lightnvm/
  obj-y				+= base/ block/ misc/ mfd/ nfc/

drivers/pardata/Kconfig new file +17 -0

new file mode 100644
index 000000000000..5a4280a3f85a
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Parallel Data Bus framework
+#
+menuconfig PARDATA
+	tristate "Parallel Data support"
+	help
+	  The parallel data framework is used for devices communicating
+	  with a simple parallel interface.
+	  The interface include 8 data pins, one register select,
+	  one enable pin, a chip select and a reset pin.
+	  The read/write may be 8080 style (one r/w pin) or 6800 style
+	  with separate read and write pins.
+	  The parallel data bus is often used for displays in embedded
+	  systems.
+
+	  If unsure, choose N.

drivers/pardata/Makefile new file +5 -0

new file mode 100644
index 000000000000..0e48bd648879
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for kernel Parallel Data subsystem
+#
+obj-$(CONFIG_PARDATA) += pardata.o

drivers/pardata/pardata.c new file +282 -0

new file mode 100644
index 000000000000..985d692c116a
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Sam Ravnborg
+ *
+ * Author: Sam Ravnborg <[email protected]>
+ */
+
+/**
+ * DOC: Bus driver for parallel data bus
+ *
+ * The parallel data bus is often used for displays
+ * in embedded designs and is sometimes also used
+ * in designs supporting Linux.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of_device.h>
+#include <linux/pardata.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/of.h>
+
+static struct pardata_device *
+pardata_device_create(struct pardatabus_data *pdbus)
+{
+	struct pardata_device *pddev;
+
+	/* We allocate the device, and initialize the default values */
+	pddev = kzalloc(sizeof(*pddev), GFP_KERNEL);
+	if (!pddev)
+		return ERR_PTR(-ENOMEM);
+
+	pddev->dev.parent = pdbus->dev;
+	pddev->dev.bus = &pardata_bus;
+
+	dev_set_name(&pddev->dev, "pardata");
+
+	device_initialize(&pddev->dev);
+
+	return pddev;
+}
+
+static void pardata_device_release(struct pardata_device *ppdev)
+{
+	kfree(ppdev);
+}
+
+int pardata_device_register(struct pardata_device *pddev)
+{
+	return device_add(&pddev->dev);
+}
+
+static int of_pdbus_register_device(struct pardatabus_data *pdbus,
+				    struct device_node *np)
+{
+	struct pardata_device *pddev;
+	int ret;
+	u32 id;
+
+	pddev = pardata_device_create(pdbus);
+	if (IS_ERR(pddev))
+		return PTR_ERR(pddev);
+
+	/* The reg property is used as id */
+	ret = of_property_read_u32(np, "reg", &id);
+	if (ret) {
+		dev_err(pdbus->dev, "Failed to read the 'reg' property");
+		pardata_device_release(pddev);
+		return ret;
+	}
+	pddev->id = id;
+
+	/*
+	 *  Associate the OF node with the device structure so it
+	 * can be looked up later.
+	 */
+	of_node_get(np);
+	pddev->dev.of_node = np;
+
+	/* All data is now stored in the pardata_device struct; register it. */
+	ret = pardata_device_register(pddev);
+	if (ret) {
+		pardata_device_release(pddev);
+		of_node_put(np);
+		return ret;
+	}
+
+	dev_dbg(&pddev->dev, "registered pardata device %s with id %i\n",
+		np->name, id);
+	return 0;
+}
+
+static void pdbus_unregister(void)
+{
+	bus_unregister(&pardata_bus);
+}
+
+/**
+ * of_pardata_register - Register pardatabus and create devices from DT
+ *
+ * @pdbus: pointer to pardatabus_data structure
+ *
+ * This function registers the pardatabus_data structure and registers
+ * a pardata_device for each child node of @np.
+ */
+int of_pardata_register(struct pardatabus_data *pdbus)
+{
+	struct device_node *child;
+	struct device_node *np;
+	int ret;
+
+	np = pdbus->dev->of_node;
+
+	/* Do not continue if the node is not available or disabled */
+	if (!np || !of_device_is_available(np))
+		return -ENODEV;
+
+	/* Register the parallel data bus */
+	ret = bus_register(&pardata_bus);
+	if (ret)
+		return ret;
+
+	/*
+	 * Loop over the child nodes and register a
+	 * pardata_device for each node
+	 */
+	for_each_available_child_of_node(np, child) {
+		ret = of_pdbus_register_device(pdbus, child);
+
+		if (ret == -ENODEV)
+			dev_err(pdbus->dev, "pardata device missing.\n");
+		else if (ret)
+			goto unregister;
+	}
+	return 0;
+
+unregister:
+	pdbus_unregister();
+	return ret;
+}
+
+static int pardata_device_match(struct device *dev, struct device_driver *drv)
+{
+	/* OF style match */
+	if (of_match_device(drv->of_match_table, dev) != NULL)
+		return 1;
+
+	return 0;
+}
+
+static int pardata_device_probe(struct device *dev)
+{
+	struct pardata_device *pddev;
+	struct pardata_driver *pddrv;
+
+	pddev = to_pardata_device(dev);
+	pddrv = to_pardata_driver(dev->driver);
+
+	return pddrv->probe(pddev);
+}
+
+struct bus_type pardata_bus = {
+	.name		= "pardata",
+	.match		= pardata_device_match,
+	.probe		= pardata_device_probe,
+};
+EXPORT_SYMBOL_GPL(pardata_bus);
+
+/*
+ * __pardata_driver_register() - Client driver registration
+ *
+ * @drv:Client driver to be associated with client-device.
+ * @owner: owning module/driver
+ *
+ * Do not call this direct, use pardata_driver_register()
+ */
+int __pardata_driver_register(struct pardata_driver *drv, struct module *owner)
+{
+	/* probe is mandatory */
+	if (!drv->probe)
+		return -EINVAL;
+
+	drv->driver.bus = &pardata_bus;
+	drv->driver.owner = owner;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL_GPL(__pardata_driver_register);
+
+/*
+ * Use a simple pardatabus driver to create the pardata
+ * devices from child nodes in DT.
+ *
+ * The pardatabus driver parses the properties in the
+ * device tree that will be used by the pardata drivers.
+ */
+static int pardatabus_probe(struct platform_device *pdev)
+{
+	struct pardatabus_data *pdbus;
+	struct gpio_descs *pins;
+	struct gpio_desc *pin;
+	struct device *dev;
+
+	dev = &pdev->dev;
+
+	pdbus = devm_kzalloc(dev, sizeof(*pdbus), GFP_KERNEL);
+	if (!pdbus)
+		return -ENOMEM;
+
+	pdbus->dev = &pdev->dev;
+	dev_set_drvdata(dev, pdbus);
+
+	/* Get the mandatory bus details from DT */
+	pins = devm_gpiod_get_array(dev, "data-gpios", GPIOD_OUT_LOW);
+	if (IS_ERR(pins)) {
+		dev_err(dev, "Failed to get gpio 'data-gpios'");
+		return PTR_ERR(pins);
+	}
+	if (pins->ndescs != 4 && pins->ndescs != 8) {
+		dev_err(dev, "Invalid number of data-gpios %d - expected 4 or 8\n",
+			    pins->ndescs);
+		return -EINVAL;
+	}
+	pdbus->data_pins = pins;
+
+	pin = devm_gpiod_get(dev, "enable-gpios", GPIOD_OUT_LOW);
+	if (IS_ERR(pin)) {
+		dev_err(dev, "Failed to get gpio 'enable-gpios'\n");
+		return PTR_ERR(pin);
+	}
+	pdbus->pin_enable = pin;
+
+	pin = devm_gpiod_get(dev, "rs-gpios", GPIOD_OUT_HIGH);
+	if (IS_ERR(pin)) {
+		dev_err(dev, "Failed to get gpio 'rs-gpios'\n");
+		return PTR_ERR(pin);
+	}
+	pdbus->pin_rs = pin;
+
+	pin = devm_gpiod_get(dev, "rw-gpios", GPIOD_OUT_HIGH);
+	if (IS_ERR(pin)) {
+		dev_err(dev, "Failed to get gpio 'rw-gpios'\n");
+		return PTR_ERR(pin);
+	}
+	pdbus->pin_readwrite = pin;
+
+	pin = devm_gpiod_get(dev, "read-gpios", GPIOD_OUT_HIGH);
+	if (IS_ERR(pin)) {
+		dev_err(dev, "Failed to get gpio 'read-gpios'\n");
+		return PTR_ERR(pin);
+	}
+	pdbus->pin_read = pin;
+
+	pin = devm_gpiod_get(dev, "write-gpios", GPIOD_OUT_HIGH);
+	if (IS_ERR(pin)) {
+		dev_err(dev, "Failed to get gpio 'write-gpios'\n");
+		return PTR_ERR(pin);
+	}
+	pdbus->pin_write = pin;
+
+	return of_pardata_register(pdbus);
+}
+
+static const struct of_device_id pardata_bus_dt_ids[] = {
+	{ .compatible = "parallel-data-bus", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver pardatabus_driver = {
+	.driver	 = {
+		.name   = "pardatabus",
+		.of_match_table = pardata_bus_dt_ids,
+	},
+	.probe = pardatabus_probe,
+};
+module_platform_driver(pardatabus_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Parallel Data Bus");

include/linux/pardata.h new file +128 -0

new file mode 100644
index 000000000000..888a1c88176e
@@ -0,0 +1,138 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 Sam Ravnborg
+ */
+
+#ifndef _LINUX_PARDATA_H
+#define _LINUX_PARDATA_H
+
+#include <linux/device.h>
+#include <linux/module.h>
+
+extern struct bus_type pardata_bus;
+
+/**
+ * struct pardatabus_data - private data for the pardata bus
+ *
+ * Drivers uses these data when usung the pins that are part of
+ * the bus.
+ */
+struct pardatabus_data {
+	/**
+	 * @dev: device node that represent the DT node of the bus
+	 */
+	struct device *dev;
+
+	/**
+	 * @data_pins: The 8 or 4 GPIOs used for data
+	 */
+	struct gpio_descs *data_pins;
+
+	/**
+	 * @pin_enable: The enable pin (mandatory)
+	 */
+	struct gpio_desc *pin_enable;
+
+	/**
+	 * @pin_rs: register select (0: instruction 1: data)
+	 */
+	struct gpio_desc *pin_rs;
+
+	/**
+	 * @pin_readwrite: readwrite pin for 8080 style interface
+	 */
+	struct gpio_desc *pin_readwrite;
+
+	/**
+	 * @pin_read: read pin for 6800 style interface
+	 */
+	struct gpio_desc *pin_read;
+
+	/**
+	 * @pin_write: write pin for 6800 style interface
+	 */
+	struct gpio_desc *pin_write;
+};
+
+static inline struct pardatabus_data *to_pardatabus_data(struct device *d)
+{
+	return container_of(d, struct pardatabus_data, dev);
+}
+
+/**
+ * struct pardata_device - pardata device handle.
+ * @dev: Driver model representation of the device.
+ *
+ * This is the device handle returned when a pardata
+ * device is registered.
+ * The id comes from the DT
+ *
+ * @dev: The device
+ * @id: Id of the device (from DT)
+ */
+struct pardata_device {
+	struct device dev;
+	u32	   id;
+};
+
+static inline struct pardata_device *to_pardata_device(struct device *d)
+{
+	return container_of(d, struct pardata_device, dev);
+}
+
+static inline void *pardata_dev_get_drvdata(const struct pardata_device *pdd)
+{
+	return dev_get_drvdata(&pdd->dev);
+}
+
+static inline void pardata_dev_set_drvdata(struct pardata_device *pdd,
+					    void *data)
+{
+	dev_set_drvdata(&pdd->dev, data);
+}
+
+/**
+ * struct pardata_driver - pardata 'generic device'
+ *			(similar to 'spi_device' on SPI)
+ *
+ * @driver: generic device driver
+ *	  pardata drivers must initialize name, owner and of_match_table
+ *	  of this structure
+ * @probe: Binds this driver to a pardata bus.
+ * @shutdown: Standard shutdown callback used during powerdown/halt.
+ */
+struct pardata_driver {
+	struct device_driver driver;
+	int   (*probe)(struct pardata_device *pdd);
+	void  (*shutdown)(struct pardata_device *pdd);
+};
+
+static inline struct pardata_driver *to_pardata_driver(struct device_driver *d)
+{
+	return container_of(d, struct pardata_driver, driver);
+}
+
+/* Use macro for driver registering to get simple access to THIS_MODULE */
+#define pardata_driver_register(drv) \
+	__pardata_driver_register(drv, THIS_MODULE)
+int __pardata_driver_register(struct pardata_driver *drv, struct module *owner);
+
+static inline void pardata_driver_unregister(struct pardata_driver *drv)
+{
+	return driver_unregister(&drv->driver);
+}
+
+/**
+ * module_pardata_driver() - Helper macro for registering a parallel data driver
+ * @__pardata_driver: pardatabus_driver struct
+ *

Hi Sam,

Den 02.08.2018 21.45, skrev Sam Ravnborg:

Quoted 7 lines

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

Quoted 6 lines

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

---

From a quick look at this I have these comments:
1. There can only be one implementation of this bus, the gpio one. There are SOC's with parallel bus hardware so you need to allow for more implementations.
2. The client shouldn't do the bus signaling. This should be hidden behind read and write functions in pardata.
3. I would also suggest you add an address bus instead of the RS pin
4. I don't think reset belongs in the bus. It's a device thing.

5. You can use gpiod_set_array_value() in the gpio implementation. Some gpio drivers can set all gpios at once.

I made an attempt at implementing a bus like this a while back:
https://github.com/notro/fbdbi/tree/master/i80

Hi Noralf.

Quoted 16 lines · Noralf Trønnes

Hi Sam,

Den 02.08.2018 21.45, skrev Sam Ravnborg:

Quoted 7 lines

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

Quoted 6 lines

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

---

Quoted 8 lines

From a quick look at this I have these comments:
1. There can only be one implementation of this bus, the gpio one. There are SOC's with parallel bus hardware so you need to allow for more implementations.
2. The client shouldn't do the bus signaling. This should be hidden behind read and write functions in pardata.
3. I would also suggest you add an address bus instead of the RS pin
4. I don't think reset belongs in the bus. It's a device thing.

Excellent feedback - I will address this in v2.

Quoted 2 lines

5. You can use gpiod_set_array_value() in the gpio implementation. Some gpio drivers can set all gpios at once.

Yes, it is already used. But only for DB0 to DB7 as there are timing constraings for the others.

Quoted 2 lines

I made an attempt at implementing a bus like this a while back:
https://github.com/notro/fbdbi/tree/master/i80

Thanks, very helpfull.

v2 will take a while as I plan to have something that actually works before posting next version.

One open question. Miguel Ojeda mentioned that there is already a limited fbdev driver made on top of parport (part of auxdisplay). Is it the correct design to come up with a new bus or should this try to build on top of parport?

I did not check in details if using parport is possible, but from a quick look it is doable.
But then we use parport for something that it originally was not designed for and we drag with us a lot of extra functionality. So I like the slimmer pardata bus.

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

Documentation/driver-api/pardata.rst new file +17 -0

@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+Parallel Data Bus/Drivers
+=========================
+
+Displays may be connected using a simple parallel bus.
+This is often seen in embedded systems with a simple MCU, but is also
+used in Linux based systems to a small extent.
+
+The bus looks like this:
+
+.. code-block:: none
+
+       ----+
+           |  DB0-DB7 or DB4-DB7      +----
+           ===/========================

Hi Sam,

(stumbled on this accidentally)

Quoted 33 lines · Sam Ravnborg

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

Documentation/driver-api/pardata.rst new file +17 -0

@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+Parallel Data Bus/Drivers
+=========================
+
+Displays may be connected using a simple parallel bus.
+This is often seen in embedded systems with a simple MCU, but is also
+used in Linux based systems to a small extent.
+
+The bus looks like this:
+
+.. code-block:: none
+
+       ----+
+           |  DB0-DB7 or DB4-DB7      +----
+           ===/========================

Documentation/driver-api/pardata.rst line 27

Quoted 10 lines

Documentation/driver-api/pardata.rst new file +10 -0

+           |  E - enable              |  D
+           ----------------------------  I
+        C  |  Reset                   |  S
+        P  ----------------------------  P
+        U  |  Read/Write (one or two) |  L
+           ----------------------------  A
+           |  RS - instruction/data   |  Y
+           ----------------------------
+           |                          +----
+       ----+

Oh, cool! Looks like this can be used by the hd44780 driver.

    Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
    drivers/auxdisplay/hd44780.c

Quoted 47 lines · Geert Uytterhoeven

Quoted 33 lines · Sam Ravnborg

The pardata supports implement a simple bus for devices that are connected using a parallel bus driven by GPIOs. The is often used in combination with simple displays that is often seen in older embedded designs. There is a demand for this support also in the linux kernel for HW designs that uses these kind of displays.

The pardata bus uses a platfrom_driver that when probed creates devices for all child nodes in the DT, which are then supposed to be handled by pardata_drivers.

Signed-off-by Sam Ravnborg <[email protected]>

Documentation/driver-api/pardata.rst new file +17 -0

@@ -0,0 +1,60 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=========================
+Parallel Data Bus/Drivers
+=========================
+
+Displays may be connected using a simple parallel bus.
+This is often seen in embedded systems with a simple MCU, but is also
+used in Linux based systems to a small extent.
+
+The bus looks like this:
+
+.. code-block:: none
+
+       ----+
+           |  DB0-DB7 or DB4-DB7      +----
+           ===/========================

Documentation/driver-api/pardata.rst line 27

Quoted 10 lines

Documentation/driver-api/pardata.rst new file +10 -0

+           |  E - enable              |  D
+           ----------------------------  I
+        C  |  Reset                   |  S
+        P  ----------------------------  P
+        U  |  Read/Write (one or two) |  L
+           ----------------------------  A
+           |  RS - instruction/data   |  Y
+           ----------------------------
+           |                          +----
+       ----+

Oh, cool! Looks like this can be used by the hd44780 driver.

Quoted 2 lines

    Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
    drivers/auxdisplay/hd44780.c

This patchset was from a time when I knew next to nothing about DRM. Now I am just confused on a different level :-)

It is on my TODO list to make a mipi-dbi driver that in the future replaces the auxdisplay driver for hd44780.

Quoted 2 lines

It is on my TODO list to make a mipi-dbi driver that in the future replaces the auxdisplay driver for hd44780.

Please note that hd44780 is a character controller.

Hi Sam,

I love your patch! Perhaps something to improve:

url: https://github.com/0day-ci/linux/commits/Sam-Ravnborg/dt-bindings-add-parallel-data-bus-pardata/20180803-090135

smatch warnings:
drivers/gpu/drm/tinydrm/wg160160.c:145 write_buf() warn: right shifting more than type allows 8 vs 8 include/drm/tinydrm/pardata-dbi.h:165 pardata_write_buf() error: we previously assumed 'pdd' could be null (see line 162)

# https://github.com/0day-ci/linux/commit/9fcebca9e208029e06eea5e4858c1055132f06b6
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 9fcebca9e208029e06eea5e4858c1055132f06b6
vim +145 drivers/gpu/drm/tinydrm/wg160160.c

9fcebca9 Sam Ravnborg 2018-08-02  124  
9fcebca9 Sam Ravnborg 2018-08-02  125  /**
9fcebca9 Sam Ravnborg 2018-08-02  126   * write_buf - write buffer on parallel bus to controller
9fcebca9 Sam Ravnborg 2018-08-02  127   *
9fcebca9 Sam Ravnborg 2018-08-02  128   * @pdd: pardata data
9fcebca9 Sam Ravnborg 2018-08-02  129   * @offset: offset into display RAM
9fcebca9 Sam Ravnborg 2018-08-02  130   * @data: pointer to data to write
9fcebca9 Sam Ravnborg 2018-08-02  131   * @len: number of bytes to write
9fcebca9 Sam Ravnborg 2018-08-02  132   *
9fcebca9 Sam Ravnborg 2018-08-02  133   * Returns:
9fcebca9 Sam Ravnborg 2018-08-02  134   * Zero on success, negative error code on failure
9fcebca9 Sam Ravnborg 2018-08-02  135   */
9fcebca9 Sam Ravnborg 2018-08-02  136  int write_buf(struct pardata_data *pdd, u8 offset, u8 *data, size_t len)
9fcebca9 Sam Ravnborg 2018-08-02  137  {
9fcebca9 Sam Ravnborg 2018-08-02  138  	int ins[PIN_NUM];
9fcebca9 Sam Ravnborg 2018-08-02  139  	int val[PIN_NUM];
9fcebca9 Sam Ravnborg 2018-08-02  140  	int bit;
9fcebca9 Sam Ravnborg 2018-08-02  141  	int i;
9fcebca9 Sam Ravnborg 2018-08-02  142  
9fcebca9 Sam Ravnborg 2018-08-02  143  	/* Setup address */
9fcebca9 Sam Ravnborg 2018-08-02  144  	write_reg(pdd, WG160160_ADDRSL_REG, offset & 0xff);
9fcebca9 Sam Ravnborg 2018-08-02 @145  	write_reg(pdd, WG160160_ADDRSL_REG, (offset >> 8) & 0xff);
                                                                            ^^^^^^^^^^^^^^^^^^^^

Probably this is fine. I don't know. If so then feel free to ignore the warning.

Add driver for the winstar wg160160 display.
The driver utilises pardata-dbi that
again utilise the pardata subsystem.

Signed-off-by Sam Ravnborg <[email protected]>

---
  MAINTAINERS                        |   5 +
  drivers/gpu/drm/tinydrm/Kconfig    |  10 ++
  drivers/gpu/drm/tinydrm/Makefile   |   1 +
  drivers/gpu/drm/tinydrm/wg160160.c | 298 +++++++++++++++++++++++++++++++++++++
  4 files changed, 314 insertions(+)

create mode 100644 drivers/gpu/drm/tinydrm/wg160160.c

drivers/gpu/drm/tinydrm/wg160160.c new file +27 -0

+
+/**
+ * write_reg - Write instruction on parallel bus to controller
+ *
+ * Check BUSY flag and write instruction
+ *
+ * @pdd: pardata data
+ * @reg: The register to write
+ * @value: The value of the register
+ *
+ * Returns:
+ * Zero on success, negative error code on failure
+ */
+int write_reg(struct pardata_data *pdd, unsigned int reg, unsigned int value)
+{
+	int ins[PIN_NUM];
+	int val[PIN_NUM];
+	int i;
+
+	for (i = 0; i < PIN_NUM; i++)
+		ins[PIN_DB0 + i] = !!BIT(reg);
+
+	for (i = 0; i < PIN_NUM; i++)
+		val[PIN_DB0 + i] = !!(value & BIT(i));
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);

drivers/gpu/drm/tinydrm/wg160160.c new file +48 -0

+
+/**
+ * write_buf - write buffer on parallel bus to controller
+ *
+ * @pdd: pardata data
+ * @offset: offset into display RAM
+ * @data: pointer to data to write
+ * @len: number of bytes to write
+ *
+ * Returns:
+ * Zero on success, negative error code on failure
+ */
+int write_buf(struct pardata_data *pdd, u8 offset, u8 *data, size_t len)
+{
+	int ins[PIN_NUM];
+	int val[PIN_NUM];
+	int bit;
+	int i;
+
+	/* Setup address */
+	write_reg(pdd, WG160160_ADDRSL_REG, offset & 0xff);
+	write_reg(pdd, WG160160_ADDRSL_REG, (offset >> 8) & 0xff);
+
+	/* prepare to write data */
+	for (i = 0; i < PIN_NUM; i++)
+		ins[PIN_DB0 + i] = !!(WG160160_WRITE_REG & BIT(i));
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	/* Write data byte - by byte */
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
+
+	for (i = offset; i < (offset + len); i++) {
+		for (bit = 0; bit < PIN_NUM; bit++)
+			val[PIN_DB0 + bit] = !!(data[i] & BIT(bit));
+
+		gpiod_set_array_value_cansleep(PIN_NUM,
+					       pdd->bus->data_pins->desc,
+					       val);
+		wait_busy(pdd);
+		pardata_strobe_write(pdd);
+	}
+
+	return 0;
+}

Den 02.08.2018 21.45, skrev Sam Ravnborg:

Quoted 13 lines

Add driver for the winstar wg160160 display.
The driver utilises pardata-dbi that
again utilise the pardata subsystem.

Signed-off-by Sam Ravnborg <[email protected]>

---
 MAINTAINERS                        |   5 +
 drivers/gpu/drm/tinydrm/Kconfig    |  10 ++
 drivers/gpu/drm/tinydrm/Makefile   |   1 +
 drivers/gpu/drm/tinydrm/wg160160.c | 298 +++++++++++++++++++++++++++++++++++++
 4 files changed, 314 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/wg160160.c

[...]

Quoted 32 lines

drivers/gpu/drm/tinydrm/wg160160.c new file +32 -0

+
+/**
+ * write_reg - Write instruction on parallel bus to controller
+ *
+ * Check BUSY flag and write instruction
+ *
+ * @pdd: pardata data
+ * @reg: The register to write
+ * @value: The value of the register
+ *
+ * Returns:
+ * Zero on success, negative error code on failure
+ */
+int write_reg(struct pardata_data *pdd, unsigned int reg, unsigned int value)
+{
+	int ins[PIN_NUM];
+	int val[PIN_NUM];
+	int i;
+
+	for (i = 0; i < PIN_NUM; i++)
+		ins[PIN_DB0 + i] = !!BIT(reg);
+
+	for (i = 0; i < PIN_NUM; i++)
+		val[PIN_DB0 + i] = !!(value & BIT(i));
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, val);

drivers/gpu/drm/tinydrm/wg160160.c line 123

Quoted 5 lines

drivers/gpu/drm/tinydrm/wg160160.c new file +5 -0

+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	return 0;
+}

If this controller has normal registers, you could do a regmap implementation for pardata: drivers/base/regmap.

Hi Noralf.

Quoted 47 lines · Noralf Trønnes

Den 02.08.2018 21.45, skrev Sam Ravnborg:

Quoted 13 lines

Add driver for the winstar wg160160 display.
The driver utilises pardata-dbi that
again utilise the pardata subsystem.

Signed-off-by Sam Ravnborg <[email protected]>

---
  MAINTAINERS                        |   5 +
  drivers/gpu/drm/tinydrm/Kconfig    |  10 ++
  drivers/gpu/drm/tinydrm/Makefile   |   1 +
  drivers/gpu/drm/tinydrm/wg160160.c | 298 +++++++++++++++++++++++++++++++++++++
  4 files changed, 314 insertions(+)

create mode 100644 drivers/gpu/drm/tinydrm/wg160160.c

[...]

Quoted 31 lines

drivers/gpu/drm/tinydrm/wg160160.c new file +31 -0

+
+/**
+ * write_reg - Write instruction on parallel bus to controller
+ *
+ * Check BUSY flag and write instruction
+ *
+ * @pdd: pardata data
+ * @reg: The register to write
+ * @value: The value of the register
+ *
+ * Returns:
+ * Zero on success, negative error code on failure
+ */
+int write_reg(struct pardata_data *pdd, unsigned int reg, unsigned int value)
+{
+	int ins[PIN_NUM];
+	int val[PIN_NUM];
+	int i;
+
+	for (i = 0; i < PIN_NUM; i++)
+		ins[PIN_DB0 + i] = !!BIT(reg);
+
+	for (i = 0; i < PIN_NUM; i++)
+		val[PIN_DB0 + i] = !!(value & BIT(i));
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);

Quoted 8 lines

drivers/gpu/drm/tinydrm/wg160160.c line 123

Quoted 6 lines

drivers/gpu/drm/tinydrm/wg160160.c new file +6 -0

+	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, val);
+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	return 0;
+}

If this controller has normal registers, you could do a regmap implementation for pardata: drivers/base/regmap.

If I understand you correct then you suggest to add a regmap implementation on top of the registers replacing the current gpio support?

So we in regmap can optimize access to the registers better. Or did you have something else in mind?

As speed is not the main challenge here I will likely wait with this and continue using gpio.

Sam

This is an RFC - to get some responses on the overall design. The code builds but has not yet been tested on any HW. Before investing more time into this I would like some feedback if this is the right way forward or a different design should be pursued.

The problem to solve is that I have a propriatary ARM based board with a display that is connected using a parallel bus. The display is capable of showing graphics
so it is more advanced than what is found in auxdisplay/

I know there are others using a display connected usign a parallel data bus, so this is not a unique problem for my board alone. But I do not expect many users as any modern design likely uses SPI or similar.

The old (proprietary) approach was to implement a char driver and then let some 3rd party lib use the char driver to write to the display.
The goal is to move to a more modern world where I expect to have a simple Qt based program running that can
be used for a few simple things.
(I do not expect any high performance and do not need it).

Implementation:

A pardata bus is implemented.
It uses a platform_driver to hook into the DT. When probed the pardatabus driver creates pardata devices for all child nodes in the tree.

Within tinydrm the pardata support is used to implement a driver for the display I have (Winstar wg160160). A library module is used to implement the more basic things allowing us to have a more simple driver, and thus making it simpler to add new drivers.

The implmentation uses array support in gpiolib to try to have some performance on screen updates.

TODO:
- Test on HW
- Add locking so there can be more than one user on the bus
- Improve pardata.rst documentation
- Add support for a sparkfun parallel data display (To verify that the library is generic)
- I may add a class if I see the need
- Likewise I may add sysfs attributes if there is a need for it

Any comments appreciated!

Sam

Sam Ravnborg (5):
dt-bindings: add parallel data bus (pardata)
pardata: new bus for parallel data access
tinydrm: add support for parallel data displays
dt-bindings: add winstar,wg160160 display bindings
tinydrm: add winstar wg160160 driver

 .../bindings/display/winstar,wg160160.txt          |  53 +++
 .../bindings/pardata/parallel-data-bus.txt         |  60 +++
 Documentation/driver-api/index.rst                 |   1 +
 Documentation/driver-api/pardata.rst               |  60 +++
 MAINTAINERS                                        |  14 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/gpu/drm/tinydrm/Kconfig                    |  13 +
 drivers/gpu/drm/tinydrm/Makefile                   |   2 +
 drivers/gpu/drm/tinydrm/pardata-dbi.c              | 417 +++++++++++++++++++++
 drivers/gpu/drm/tinydrm/wg160160.c                 | 298 +++++++++++++++
 drivers/pardata/Kconfig                            |  17 +
 drivers/pardata/Makefile                           |   5 +
 drivers/pardata/pardata.c                          | 282 ++++++++++++++
 include/drm/tinydrm/pardata-dbi.h                  | 257 +++++++++++++
 include/linux/pardata.h                            | 138 +++++++
 16 files changed, 1620 insertions(+)