[RFC PATCH v1 0/5] Add pardata bus + tinydrm driver

[RFC PATCH v1 0/5] Add pardata bus + tinydrm driver

[Sam Ravnborg]

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(+)

[PATCH v1 1/5] dt-bindings: add parallel data bus (pardata)

[Sam Ravnborg]

The parallel data bus is a simple parallel bus
that may be used to drive displays.
It is mostly used in simple embedded systems
but is also used in some desings utilising Linux.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 .../bindings/pardata/parallel-data-bus.txt         | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pardata/parallel-data-bus.txt

diff --git a/Documentation/devicetree/bindings/pardata/parallel-data-bus.txt b/Documentation/devicetree/bindings/pardata/parallel-data-bus.txt
new file mode 100644
index 000000000000..fa33b5bb8302
--- /dev/null
+++ b/Documentation/devicetree/bindings/pardata/parallel-data-bus.txt
@@ -0,0 +1,60 @@
+DT binding for parallel data bus
+
+Required properties:
+- compatible: "parallel-data-bus"
+- data-gpios: array of 8 GPIO specifiers, referring to the GPIO pins
+  connected to the data signal lines DB0-DB7
+  (8-bit mode) of the LCD Controller's bus interface,
+- enable-gpios: GPIO specifier, referring to the GPIO pin connected to
+  the "E" (Enable) signal line of the controller's bus interface,
+- rs-gpios: GPIO specifier, referring to the GPIO pin
+  connected to the "RS" (Register Select) controller's bus interface,
+
+Required properties for 8080 interface:
+- readwrite-gpios: For chips with 8080 interface list the GPIO
+  pin connected to the controllers read/write pin
+
+Required properties for 6800 interface:
+- read-gpios: For chips with 6800 interface list the GPIO
+  pin connected to the controllers read pin
+- write-gpios: For chips with 6800 interface list the GPIO
+  pin connected to the controllers write pin
+
+Required property for child node(s):
+- reg: numeric identifier for the display
+
+Example:
+
+	backlight: backlight {
+		compatible = "gpio-backlight";
+	}
+
+	power: regulator@0 {
+	}
+
+	pardatabus {
+		compatible = "parallel-data-bus";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pardata>;
+		data-gpios = <&pioe 0 GPIO_ACTIVE_HIGH
+			      &pioe 1 GPIO_ACTIVE_HIGH
+			      &pioe 2 GPIO_ACTIVE_HIGH
+			      &pioe 3 GPIO_ACTIVE_HIGH
+			      &pioe 4 GPIO_ACTIVE_HIGH
+			      &pioe 5 GPIO_ACTIVE_HIGH
+			      &pioe 6 GPIO_ACTIVE_HIGH
+			      &pioe 7 GPIO_ACTIVE_HIGH>;
+		enable-gpios = <&pioe 8 GPIO_ACTIVE_HIGH>;
+		rs-gpios = <&pioe 12 GPIO_ACTIVE_HIGH>;
+		readdwrite-gpios = <&pioe 11 GPIO_ACTIVE_HIGH>;
+
+		wg160160@0 {
+			compatible = "winstar,wg160160";
+			reg = <1>;
+			reset-gpios = <&pioe 13 GPIO_ACTIVE_LOW>;
+			chipselect-gpios = <&pioe 9 GPIO_ACTIVE_LOW>;
+			backlight = &backlight;
+			power = &power;
+		}
+	}
+
-- 
2.12.0

[PATCH v1 2/5] pardata: new bus for parallel data access

[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 <sam@ravnborg.org>
---
 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

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index 6d9f2f9fe20e..1808fca406ae 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -41,6 +41,7 @@ available subsections can be seen below.
    miscellaneous
    w1
    rapidio
+   pardata
    s390-drivers
    vme
    80211/index
diff --git a/Documentation/driver-api/pardata.rst b/Documentation/driver-api/pardata.rst
new file mode 100644
index 000000000000..a811f024a0fe
--- /dev/null
+++ b/Documentation/driver-api/pardata.rst
@@ -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:
diff --git a/MAINTAINERS b/MAINTAINERS
index 96e98e206b0d..4ba7ff7c3e46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10727,6 +10727,15 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/panasonic-laptop.c
 
+PARALLEL DATA SUBSYSTEM
+M:	Sam Ravnborg <sam@ravnborg.org>
+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 <willy@haproxy.com>
 M:	Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9ccc08165..b51b25aae9a5 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -217,4 +217,6 @@ source "drivers/siox/Kconfig"
 
 source "drivers/slimbus/Kconfig"
 
+source "drivers/pardata/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 24cd47014657..cfe7945f2f6b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -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/
diff --git a/drivers/pardata/Kconfig b/drivers/pardata/Kconfig
new file mode 100644
index 000000000000..5a4280a3f85a
--- /dev/null
+++ b/drivers/pardata/Kconfig
@@ -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.
diff --git a/drivers/pardata/Makefile b/drivers/pardata/Makefile
new file mode 100644
index 000000000000..0e48bd648879
--- /dev/null
+++ b/drivers/pardata/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for kernel Parallel Data subsystem
+#
+obj-$(CONFIG_PARDATA) += pardata.o
diff --git a/drivers/pardata/pardata.c b/drivers/pardata/pardata.c
new file mode 100644
index 000000000000..985d692c116a
--- /dev/null
+++ b/drivers/pardata/pardata.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Sam Ravnborg
+ *
+ * Author: Sam Ravnborg <sam@ravnborg.org>
+ */
+
+/**
+ * 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");
diff --git a/include/linux/pardata.h b/include/linux/pardata.h
new file mode 100644
index 000000000000..888a1c88176e
--- /dev/null
+++ b/include/linux/pardata.h
@@ -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
+ *
+ * Helper macro for parallel data drivers which do not do anything special in
+ * module init/exit. This eliminates a lot of boilerplate. Each module may
+ * only use this macro once, and calling it replaces module_init() and
+ * module_exit()
+ */
+#define module_pardata_driver(__pardata_driver) \
+	module_driver(__pardata_driver, pardata_driver_register, \
+			pardata_driver_unregister)
+
+#endif /* _LINUX_PARDATA_H */
-- 
2.12.0

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Noralf Trønnes]

Hi Sam,

Den 02.08.2018 21.45, skrev 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 <sam@ravnborg.org>
> ---

 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

Noralf.

>   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
>
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index 6d9f2f9fe20e..1808fca406ae 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -41,6 +41,7 @@ available subsections can be seen below.
>      miscellaneous
>      w1
>      rapidio
> +   pardata
>      s390-drivers
>      vme
>      80211/index
> diff --git a/Documentation/driver-api/pardata.rst b/Documentation/driver-api/pardata.rst
> new file mode 100644
> index 000000000000..a811f024a0fe
> --- /dev/null
> +++ b/Documentation/driver-api/pardata.rst
> @@ -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:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96e98e206b0d..4ba7ff7c3e46 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10727,6 +10727,15 @@ L:	platform-driver-x86@vger.kernel.org
>   S:	Maintained
>   F:	drivers/platform/x86/panasonic-laptop.c
>   
> +PARALLEL DATA SUBSYSTEM
> +M:	Sam Ravnborg <sam@ravnborg.org>
> +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 <willy@haproxy.com>
>   M:	Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 95b9ccc08165..b51b25aae9a5 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -217,4 +217,6 @@ source "drivers/siox/Kconfig"
>   
>   source "drivers/slimbus/Kconfig"
>   
> +source "drivers/pardata/Kconfig"
> +
>   endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..cfe7945f2f6b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -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/
> diff --git a/drivers/pardata/Kconfig b/drivers/pardata/Kconfig
> new file mode 100644
> index 000000000000..5a4280a3f85a
> --- /dev/null
> +++ b/drivers/pardata/Kconfig
> @@ -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.
> diff --git a/drivers/pardata/Makefile b/drivers/pardata/Makefile
> new file mode 100644
> index 000000000000..0e48bd648879
> --- /dev/null
> +++ b/drivers/pardata/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for kernel Parallel Data subsystem
> +#
> +obj-$(CONFIG_PARDATA) += pardata.o
> diff --git a/drivers/pardata/pardata.c b/drivers/pardata/pardata.c
> new file mode 100644
> index 000000000000..985d692c116a
> --- /dev/null
> +++ b/drivers/pardata/pardata.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, Sam Ravnborg
> + *
> + * Author: Sam Ravnborg <sam@ravnborg.org>
> + */
> +
> +/**
> + * 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");
> diff --git a/include/linux/pardata.h b/include/linux/pardata.h
> new file mode 100644
> index 000000000000..888a1c88176e
> --- /dev/null
> +++ b/include/linux/pardata.h
> @@ -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
> + *
> + * Helper macro for parallel data drivers which do not do anything special in
> + * module init/exit. This eliminates a lot of boilerplate. Each module may
> + * only use this macro once, and calling it replaces module_init() and
> + * module_exit()
> + */
> +#define module_pardata_driver(__pardata_driver) \
> +	module_driver(__pardata_driver, pardata_driver_register, \
> +			pardata_driver_unregister)
> +
> +#endif /* _LINUX_PARDATA_H */

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Noralf Trønnes]

Hi Sam,

Den 02.08.2018 21.45, skrev 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 <sam@ravnborg.org>
> ---

 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

Noralf.

>   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
>
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index 6d9f2f9fe20e..1808fca406ae 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -41,6 +41,7 @@ available subsections can be seen below.
>      miscellaneous
>      w1
>      rapidio
> +   pardata
>      s390-drivers
>      vme
>      80211/index
> diff --git a/Documentation/driver-api/pardata.rst b/Documentation/driver-api/pardata.rst
> new file mode 100644
> index 000000000000..a811f024a0fe
> --- /dev/null
> +++ b/Documentation/driver-api/pardata.rst
> @@ -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:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 96e98e206b0d..4ba7ff7c3e46 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10727,6 +10727,15 @@ L:	platform-driver-x86@vger.kernel.org
>   S:	Maintained
>   F:	drivers/platform/x86/panasonic-laptop.c
>   
> +PARALLEL DATA SUBSYSTEM
> +M:	Sam Ravnborg <sam@ravnborg.org>
> +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 <willy@haproxy.com>
>   M:	Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 95b9ccc08165..b51b25aae9a5 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -217,4 +217,6 @@ source "drivers/siox/Kconfig"
>   
>   source "drivers/slimbus/Kconfig"
>   
> +source "drivers/pardata/Kconfig"
> +
>   endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 24cd47014657..cfe7945f2f6b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -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/
> diff --git a/drivers/pardata/Kconfig b/drivers/pardata/Kconfig
> new file mode 100644
> index 000000000000..5a4280a3f85a
> --- /dev/null
> +++ b/drivers/pardata/Kconfig
> @@ -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.
> diff --git a/drivers/pardata/Makefile b/drivers/pardata/Makefile
> new file mode 100644
> index 000000000000..0e48bd648879
> --- /dev/null
> +++ b/drivers/pardata/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for kernel Parallel Data subsystem
> +#
> +obj-$(CONFIG_PARDATA) += pardata.o
> diff --git a/drivers/pardata/pardata.c b/drivers/pardata/pardata.c
> new file mode 100644
> index 000000000000..985d692c116a
> --- /dev/null
> +++ b/drivers/pardata/pardata.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, Sam Ravnborg
> + *
> + * Author: Sam Ravnborg <sam@ravnborg.org>
> + */
> +
> +/**
> + * 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");
> diff --git a/include/linux/pardata.h b/include/linux/pardata.h
> new file mode 100644
> index 000000000000..888a1c88176e
> --- /dev/null
> +++ b/include/linux/pardata.h
> @@ -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
> + *
> + * Helper macro for parallel data drivers which do not do anything special in
> + * module init/exit. This eliminates a lot of boilerplate. Each module may
> + * only use this macro once, and calling it replaces module_init() and
> + * module_exit()
> + */
> +#define module_pardata_driver(__pardata_driver) \
> +	module_driver(__pardata_driver, pardata_driver_register, \
> +			pardata_driver_unregister)
> +
> +#endif /* _LINUX_PARDATA_H */

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Sam Ravnborg]

Hi Noralf.

On Tue, Aug 07, 2018 at 06:40:29PM +0200, Noralf Trønnes wrote:
> Hi Sam,
> 
> Den 02.08.2018 21.45, skrev 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 <sam@ravnborg.org>
> >---
> 
> 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.

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

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

	Sam

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Noralf Trønnes]

Den 08.08.2018 10.24, skrev Sam Ravnborg:
> Hi Noralf.
>
> On Tue, Aug 07, 2018 at 06:40:29PM +0200, Noralf Trønnes wrote:
>> Hi Sam,
>>
>> Den 02.08.2018 21.45, skrev 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 <sam@ravnborg.org>
>>> ---
>>  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.
>
>> 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.
>
>> 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.

I couldn't find a parport gpio driver, so you would have to write one by
the looks of it.

I think it would be best to add a new bus type, but that's just my opinion.
I suggest you study the other bus types (spi, i2c, spmi) to get an
understanding of how it's done.

Noralf.

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Geert Uytterhoeven]

Hi Sam,

(stumbled on this accidentally)

On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> 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 <sam@ravnborg.org>

> --- /dev/null
> +++ b/Documentation/driver-api/pardata.rst
> @@ -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
> +           ----------------------------
> +           |                          +----
> +       ----+

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

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Geert Uytterhoeven]

Hi Sam,

(stumbled on this accidentally)

On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> 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 <sam@ravnborg.org>

> --- /dev/null
> +++ b/Documentation/driver-api/pardata.rst
> @@ -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
> +           ----------------------------
> +           |                          +----
> +       ----+

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

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Sam Ravnborg]

Hi Geert.

On Mon, Jan 20, 2020 at 11:10:37AM +0100, Geert Uytterhoeven wrote:
> Hi Sam,
> 
> (stumbled on this accidentally)
> 
> On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > 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 <sam@ravnborg.org>
> 
> > --- /dev/null
> > +++ b/Documentation/driver-api/pardata.rst
> > @@ -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
> > +           ----------------------------
> > +           |                          +----
> > +       ----+
> 
> Oh, cool!  Looks like this can be used by the hd44780 driver.
> 
>     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.
But that TODO list have a growing tendency.
It seems that pretending to be co-maintainer on panel/ stuff
alone can take up most of my DRM time.

I hope Paul will contribute the i8080 support to drm_mipi_dbi,
at least he mentioned he planned to work on this.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Sam Ravnborg]

Hi Geert.

On Mon, Jan 20, 2020 at 11:10:37AM +0100, Geert Uytterhoeven wrote:
> Hi Sam,
> 
> (stumbled on this accidentally)
> 
> On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > 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 <sam@ravnborg.org>
> 
> > --- /dev/null
> > +++ b/Documentation/driver-api/pardata.rst
> > @@ -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
> > +           ----------------------------
> > +           |                          +----
> > +       ----+
> 
> Oh, cool!  Looks like this can be used by the hd44780 driver.
> 
>     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.
But that TODO list have a growing tendency.
It seems that pretending to be co-maintainer on panel/ stuff
alone can take up most of my DRM time.

I hope Paul will contribute the i8080 support to drm_mipi_dbi,
at least he mentioned he planned to work on this.

	Sam

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Geert Uytterhoeven]

Hi Sam,

On Mon, Jan 20, 2020 at 7:48 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> On Mon, Jan 20, 2020 at 11:10:37AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > > 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 <sam@ravnborg.org>
> >
> > > --- /dev/null
> > > +++ b/Documentation/driver-api/pardata.rst
> > > @@ -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
> > > +           ----------------------------
> > > +           |                          +----
> > > +       ----+
> >
> > Oh, cool!  Looks like this can be used by the hd44780 driver.
> >
> >     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 :-)

The more you know, the more you realize what you don't know ;-)

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Geert Uytterhoeven]

Hi Sam,

On Mon, Jan 20, 2020 at 7:48 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> On Mon, Jan 20, 2020 at 11:10:37AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Aug 2, 2018 at 9:46 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> > > 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 <sam@ravnborg.org>
> >
> > > --- /dev/null
> > > +++ b/Documentation/driver-api/pardata.rst
> > > @@ -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
> > > +           ----------------------------
> > > +           |                          +----
> > > +       ----+
> >
> > Oh, cool!  Looks like this can be used by the hd44780 driver.
> >
> >     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 :-)

The more you know, the more you realize what you don't know ;-)

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Sam Ravnborg]

Hi Geert.

> > 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.
Directory was correct - but name was wrong.
It is cfag12864b I have on my TODO.

And yes, the code for hd44780 is similar. The name was familiar
because I also looked at this driver back then.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 2/5] pardata: new bus for parallel data access

[Sam Ravnborg]

Hi Geert.

> > 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.
Directory was correct - but name was wrong.
It is cfag12864b I have on my TODO.

And yes, the code for hd44780 is similar. The name was familiar
because I also looked at this driver back then.

	Sam

[PATCH v1 3/5] tinydrm: add support for parallel data displays

[Sam Ravnborg]

Utilising the pardata bus/driver add support for
displays connected to a parallel data bus.
There is no specific protocol implemented,
but the display is tied to the tinydrm pipe.

Only monochrome displays supported using the
XRGB8888 format.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/tinydrm/Kconfig       |   3 +
 drivers/gpu/drm/tinydrm/Makefile      |   1 +
 drivers/gpu/drm/tinydrm/pardata-dbi.c | 417 ++++++++++++++++++++++++++++++++++
 include/drm/tinydrm/pardata-dbi.h     | 257 +++++++++++++++++++++
 4 files changed, 678 insertions(+)
 create mode 100644 drivers/gpu/drm/tinydrm/pardata-dbi.c
 create mode 100644 include/drm/tinydrm/pardata-dbi.h

diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 4592a5e3f20b..435de2f8d8f5 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -10,6 +10,9 @@ menuconfig DRM_TINYDRM
 config TINYDRM_MIPI_DBI
 	tristate
 
+config TINYDRM_PARDATA_DBI
+	tristate
+
 config TINYDRM_ILI9225
 	tristate "DRM support for ILI9225 display panels"
 	depends on DRM_TINYDRM && SPI
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 49a111929724..0b52df08b0a4 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_DRM_TINYDRM)		+= core/
 
 # Controllers
 obj-$(CONFIG_TINYDRM_MIPI_DBI)		+= mipi-dbi.o
+obj-$(CONFIG_TINYDRM_PARDATA_DBI)	+= pardata-dbi.o
 
 # Displays
 obj-$(CONFIG_TINYDRM_ILI9225)		+= ili9225.o
diff --git a/drivers/gpu/drm/tinydrm/pardata-dbi.c b/drivers/gpu/drm/tinydrm/pardata-dbi.c
new file mode 100644
index 000000000000..09bdfdba6291
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/pardata-dbi.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Parallel Data Display Bus Interface LCD controller support
+ */
+
+#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
+#include <linux/dma-buf.h>
+#include <linux/pardata.h>
+#include <linux/module.h>
+
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <drm/tinydrm/pardata-dbi.h>
+
+/**
+ * pardata_strobe_8080_write - using the 8080 interface create
+ *			     an enable strobe
+ *
+ * The interface requires that readwrite and chipselect are
+ * setup before the strobe of "e".
+ * There are timings constraints that is handled using udelay().
+ * Drivers can use this for the strobe_write callback.
+ *
+ * @pdd: pardata data
+ */
+void pardata_strobe_8080_write(struct pardata_data *pdd)
+{
+	gpiod_set_value_cansleep(pdd->bus->pin_readwrite, 0);
+
+	if (pdd->pin_cs)
+		gpiod_set_value_cansleep(pdd->pin_cs, 0);
+	/* min 90 nsec from cs/rs/rw to e */
+	udelay(1);
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 1);
+	/* data setup time 220 ns*/
+	udelay(2);
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 0);
+	/* data hold time 20 ns */
+	udelay(1);
+	if (pdd->pin_cs)
+		gpiod_set_value_cansleep(pdd->pin_cs, 1);
+}
+EXPORT_SYMBOL_GPL(pardata_strobe_8080_write);
+
+/**
+ * pardata_strobe_6800_write - using the 6800 interface create
+ *			     an enable strobe
+ *
+ * The interface requires that read + write and chipselect are
+ * setup before the strobe of "e".
+ * There are timings constraints that is handled using udelay()
+ * Drivers can use this for the strobe_write callback.
+ *
+ * @pdd: pardata data
+ */
+
+void pardata_strobe_6800_write(struct pardata_data *pdd)
+{
+	gpiod_set_value_cansleep(pdd->bus->pin_read, 0);
+	gpiod_set_value_cansleep(pdd->bus->pin_write, 1);
+
+	if (pdd->pin_cs)
+		gpiod_set_value_cansleep(pdd->pin_cs, 0);
+	/* min 90 nsec from cs/rs/rw to e */
+	udelay(1);
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 1);
+	/* data setup time 220 ns*/
+	udelay(2);
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 0);
+	/* data hold time 20 ns */
+	udelay(1);
+	if (pdd->pin_cs)
+		gpiod_set_value_cansleep(pdd->pin_cs, 1);
+}
+EXPORT_SYMBOL_GPL(pardata_strobe_6800_write);
+
+
+static int pardata_write_clip(struct pardata_data *pdd,
+			      struct drm_framebuffer *fb,
+			      u8 *data,
+			      struct drm_clip_rect *clip)
+{
+	bool line_by_line;
+	size_t linelen;
+	size_t len;
+	int x, y;
+	u8 *buf;
+
+	linelen = clip->x2 - clip->x1;
+	len = linelen * (clip->y2 - clip->y1) / 8;
+	buf = kzalloc(len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	/*
+	 * If the clip is the full width then we need only one
+	 * write operation.
+	 */
+	line_by_line = (clip->x2 - clip->x1) != fb->width;
+
+	for (y = clip->y1; y < clip->y2; y++) {
+		for (x = clip->x1; x < clip->x2; x++) {
+			unsigned int index;
+			u8 cc;
+
+			index = x + y * linelen;
+			cc = data[index];
+
+			/* Most significant bit determine bit on/off */
+			if (cc & 0x80)
+				buf[index / 8] |= BIT(index % 8);
+		}
+		if (line_by_line) {
+			int offset;
+
+			offset = y * fb->width + clip->x1;
+			pardata_write_buf(pdd, offset, &buf[offset], linelen);
+		}
+	}
+	if (!line_by_line) {
+		int offset;
+
+		offset = clip->y1 * fb->width;
+		pardata_write_buf(pdd, offset, &buf[offset], len);
+	}
+
+	kfree(buf);
+
+	return 0;
+}
+
+/**
+ * pardata_buf_copy - Copy a framebuffer, transforming it if necessary
+ *
+ * @dst: The destination buffer
+ * @fb: The source framebuffer
+ * @clip: Clipping rectangle of the area to be copied
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+static int pardata_buf_copy(void *dst, struct drm_framebuffer *fb,
+			    struct drm_clip_rect *clip)
+{
+	struct dma_buf_attachment *import_attach;
+	struct drm_gem_cma_object *cma_obj;
+	void *src;
+	int ret;
+
+	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	import_attach = cma_obj->base.import_attach;
+	src = cma_obj->vaddr;
+	ret = 0;
+
+	if (import_attach) {
+		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
+					       DMA_FROM_DEVICE);
+		if (ret)
+			return ret;
+	}
+
+	tinydrm_xrgb8888_to_gray8(dst, src, fb, clip);
+
+	if (import_attach)
+		ret = dma_buf_end_cpu_access(import_attach->dmabuf,
+					     DMA_FROM_DEVICE);
+	return ret;
+}
+
+static int pardata_fb_dirty(struct drm_framebuffer *fb,
+			    struct drm_file *file_priv,
+			    unsigned int flags,
+			    unsigned int color,
+			    struct drm_clip_rect *clips,
+			    unsigned int num_clips)
+{
+	struct drm_format_name_buf format_name;
+	struct drm_gem_cma_object *cma_obj;
+	struct tinydrm_device *tdev;
+	struct drm_clip_rect clip;
+	struct pardata_data *pdd;
+	bool full;
+	int ret;
+
+	cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
+	tdev = fb->dev->dev_private;
+	pdd = pardata_from_tinydrm(tdev);
+	ret = 0;
+
+	if (!pdd->enabled)
+		return 0;
+
+	if (fb->format->format != DRM_FORMAT_XRGB8888) {
+		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
+			     drm_get_format_name(fb->format->format,
+						 &format_name));
+		return -EINVAL;
+	}
+
+	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
+				   fb->width, fb->height);
+
+	DRM_DEV_DEBUG(&pdd->pddev->dev,
+		      "Flushing [FB:%d] x1=%u, x2=%u, y1=%u, y2=%u\n",
+		      fb->base.id, clip.x1, clip.x2, clip.y1, clip.y2);
+
+	if (!full) {
+		ret = pardata_buf_copy(pdd->tx_buf, fb, &clip);
+		if (!ret)
+			ret = pardata_write_clip(pdd, fb, pdd->tx_buf, &clip);
+	} else {
+		size_t len;
+
+		len = fb->width * fb->height;
+		ret = pardata_write_buf(pdd, 0, cma_obj->vaddr, len);
+	}
+
+	return ret;
+}
+
+static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+	.dirty		= tinydrm_fb_dirty,
+};
+
+/**
+ * pardata_enable_flush -  enable helper
+ *
+ * @pdd: pardata data
+ * @crtc_state: crtc state
+ * @plane_state: plane state
+ *
+ * This function sets &pdd->enabled, flushes the whole framebuffer and
+ * enables the backlight. Drivers can use this in their
+ * &drm_simple_display_pipe_funcs->enable callback.
+ */
+void pardata_enable_flush(struct pardata_data *pdd,
+			  struct drm_crtc_state *crtc_state,
+			  struct drm_plane_state *plane_state)
+{
+	struct tinydrm_device *tdev;
+	struct drm_framebuffer *fb;
+
+	tdev = &pdd->tinydrm;
+	fb = plane_state->fb;
+	pdd->enabled = true;
+
+	if (fb)
+		tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
+
+	backlight_enable(pdd->backlight);
+}
+EXPORT_SYMBOL_GPL(pardata_enable_flush);
+
+static void pardata_blank(struct pardata_data *pdd)
+{
+	struct drm_device *drm;
+	u16 height, width;
+	size_t len;
+
+	drm = pdd->tinydrm.drm;
+	height = drm->mode_config.min_height;
+	width = drm->mode_config.min_width;
+
+	len = width * height;
+
+	memset(pdd->tx_buf, 0, len);
+	pardata_write_buf(pdd, 0, pdd->tx_buf, len);
+}
+
+/**
+ * pardata_pipe_disable - pipe disable helper
+ *
+ * @pipe: Display pipe
+ *
+ * This function disables backlight if present, if not the display memory is
+ * blanked. The regulator is disabled if in use. Drivers can use this as their
+ * &drm_simple_display_pipe_funcs->disable callback.
+ */
+void pardata_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct tinydrm_device *tdev;
+	struct pardata_data *pdd;
+
+	tdev = pipe_to_tinydrm(pipe);
+	pdd = pardata_from_tinydrm(tdev);
+
+	pdd->enabled = false;
+
+	if (pdd->backlight)
+		backlight_disable(pdd->backlight);
+	else
+		pardata_blank(pdd);
+
+	if (pdd->regulator)
+		regulator_disable(pdd->regulator);
+}
+EXPORT_SYMBOL_GPL(pardata_pipe_disable);
+
+static const uint32_t pardata_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+/**
+ * pardata_init - initialization
+ * @dev: Parent device
+ * @pdd: pardata data to initialize
+ * @pipe_funcs: Display pipe functions
+ * @driver: DRM driver
+ * @mode: Display mode
+ *
+ * This function initializes a &pardata data structure and it's underlying
+ * @tinydrm_device. It also sets up the display pipeline.
+ *
+ * Supported formats: Emulated XRGB8888.
+ *
+ * Objects created by this function will be automatically freed on driver
+ * detach (devres).
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int pardata_init(struct device *dev,
+		 struct pardata_data *pdd,
+		 const struct drm_simple_display_pipe_funcs *pipe_funcs,
+		 struct drm_driver *driver,
+		 const struct drm_display_mode *mode)
+{
+	struct tinydrm_device *tdev;
+	size_t bufsize;
+	int ret;
+
+	/* shortcut to pardatabus_data */
+	pdd->bus = dev_get_drvdata(dev->parent);
+
+	tdev = &pdd->tinydrm;
+	bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
+
+	pdd->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
+	if (!pdd->tx_buf)
+		return -ENOMEM;
+
+	ret = devm_tinydrm_init(dev, tdev, &mipi_dbi_fb_funcs, driver);
+	if (ret)
+		return ret;
+
+	tdev->fb_dirty = pardata_fb_dirty;
+
+	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
+					DRM_MODE_CONNECTOR_VIRTUAL,
+					pardata_formats,
+					ARRAY_SIZE(pardata_formats),
+					mode,
+					0);
+	if (ret)
+		return ret;
+
+	tdev->drm->mode_config.preferred_depth = 16;
+
+	drm_mode_config_reset(tdev->drm);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pardata_init);
+
+/**
+ * pardata_hw_reset - Hardware reset of controller
+ *
+ * @pdd: pardata data
+ *
+ * Reset controller if the &pardata->pin_reset gpio is set.
+ */
+void pardata_hw_reset(struct pardata_data *pdd)
+{
+	if (!pdd->pin_reset)
+		return;
+
+	gpiod_set_value_cansleep(pdd->pin_reset, 0);
+	usleep_range(20, 1000);
+	gpiod_set_value_cansleep(pdd->pin_reset, 1);
+	msleep(120);
+}
+EXPORT_SYMBOL_GPL(pardata_hw_reset);
+
+/**
+ * pardata_poweron_reset - poweron and reset
+ * @pdd: pardata data
+ *
+ * This function enables the regulator if used and does a hardware reset.
+ *
+ * Returns:
+ * Zero on success, or a negative error code.
+ */
+int pardata_poweron_reset(struct pardata_data *pdd)
+{
+	int ret;
+
+
+	if (pdd->regulator) {
+		ret = regulator_enable(pdd->regulator);
+		if (ret) {
+			DRM_DEV_ERROR(&pdd->pddev->dev,
+				      "Failed to enable regulator (%d)\n",
+				      ret);
+			return ret;
+		}
+	}
+
+	pardata_hw_reset(pdd);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pardata_poweron_reset);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/drm/tinydrm/pardata-dbi.h b/include/drm/tinydrm/pardata-dbi.h
new file mode 100644
index 000000000000..d72d9a1dff27
--- /dev/null
+++ b/include/drm/tinydrm/pardata-dbi.h
@@ -0,0 +1,257 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Parallel Data Display Bus Interface LCD controller support
+ */
+
+#include <linux/pardata.h>
+
+#include <drm/tinydrm/tinydrm.h>
+
+/**
+ * pardata_pins - the data pins
+ *
+ * The order is important due to use of writing to gpio arrays.
+ * The first four pins (PIN_DB0 to PIN_DB3) are not used for
+ * interfaces using only four data bits.
+ */
+enum pardata_pins {
+	PIN_DB0,
+	PIN_DB1,
+	PIN_DB2,
+	PIN_DB3,
+	PIN_DB4,
+	PIN_DB5,
+	PIN_DB6,
+	PIN_DB7,
+	PIN_NUM,	/* Number of pins */
+};
+
+struct pardata_data {
+	/**
+	 * @tinydrm - tinydrm base
+	 */
+	struct tinydrm_device tinydrm;
+
+	/**
+	 * dev - pardata device
+	 */
+	struct pardata_device *pddev;
+
+	/**
+	 * @busdata - data for the bus
+	 */
+	struct pardatabus_data *bus;
+
+	/**
+	 * @pin_reset: Optional GPIO reset pin
+	 */
+	struct gpio_desc *pin_reset;
+
+	/**
+	 * @pin_cs: Optional chip select pin
+	 */
+	struct gpio_desc *pin_cs;
+
+	/**
+	 * @tx_buf: Poitner to buffer to transer to display RAM
+	 */
+	u8 *tx_buf;
+
+	/**
+	 * @backlight - backlight device (optional)
+	 */
+	struct backlight_device *backlight;
+
+	/**
+	 * @regulator - power regulator (optional)
+	 */
+	struct regulator *regulator;
+
+	/**
+	 * @strobe_write - activate chip select to move data to controller
+	 *
+	 * This callback is used to set read/write and activate
+	 * pin_e and pin_cs - with respect to the timing required by
+	 * the controller.
+	 *
+	 * Use pardata_strobe_8080_write() for 8080 style interface or
+	 * pardata_strobe_6800_write() for 6800 style interface.
+	 * has individual pins for read and write.
+	 *
+	 * Parameters:
+	 *
+	 * pdd: pardata data
+	 */
+	void (*strobe_write)(struct pardata_data *pdd);
+
+	/**
+	 * write_reg - write value to register
+	 *
+	 * This callback is used to write the value to the register.
+	 *
+	 * Parameters:
+	 *
+	 * pdd: pardata data
+	 * reg: The register to write
+	 * value: The value to write to the register
+	 *
+	 * Returns:
+	 * 0 on success, negative value on error
+	 */
+	int (*write_reg)(struct pardata_data *pdd,
+			unsigned int reg, unsigned int value);
+
+	/**
+	 * write_buf - write content of buffer to controller
+	 *
+	 * This callback writes the data to the controller at offset
+	 * and forward. The write operation shall be as efficient as
+	 * possible as this will be called every time the content on
+	 * the display changes
+	 *
+	 * Parameters:
+	 *
+	 * pdd: pardata data
+	 * offset: The offset into the display memory of the controller where
+	 *	 the data shall be written.
+	 * data: Pointer to the data to write to display memory
+	 * len: Number of bytes to write to the display memory
+	 *
+	 * Returns:
+	 * 0 on success, negative value on error
+	 */
+	int (*write_buf)(struct pardata_data *pdd,
+			 u8 offset,
+			 u8 *data,
+			 size_t len);
+
+	/**
+	 * @enabled: Pipeline is enabled (internal)
+	 */
+	bool enabled;
+};
+
+static inline struct pardata_data *
+pardata_from_tinydrm(struct tinydrm_device *tdev)
+{
+	return container_of(tdev, struct pardata_data, tinydrm);
+}
+
+static inline void pardata_strobe_write(struct pardata_data *pdd)
+{
+	if (pdd && pdd->strobe_write)
+		pdd->strobe_write(pdd);
+	else
+		DRM_DEV_ERROR(&pdd->pddev->dev, "No strobe_write callback");
+}
+
+static inline int pardata_write_reg(struct pardata_data *pdd,
+				    unsigned int reg, unsigned int value)
+{
+	if (pdd && pdd->write_reg)
+		pdd->write_reg(pdd, reg, value);
+	else
+		DRM_DEV_ERROR(&pdd->pddev->dev, "No write_reg callback");
+
+	return 0;
+}
+
+static inline int pardata_write_buf(struct pardata_data *pdd,
+				    u8 offset, u8 *data, size_t len)
+{
+	if (pdd && pdd->write_buf)
+		pdd->write_buf(pdd, offset, data, len);
+	else
+		DRM_DEV_ERROR(&pdd->pddev->dev, "No write_buf callback");
+
+	return 0;
+}
+
+/**
+ * pardata_strobe_8080_write - using the 8080 interface create
+ *                             an enable strobe
+ *
+ * The interface requires that readwrite and chipselect are
+ * setup before the strobe of "e".
+ * There are timings constraints that is handled using udelay().
+ * Drivers can use this for the strobe_write callback.
+ *
+ * @pdd: pardata data
+ */
+void pardata_strobe_8080_write(struct pardata_data *pdd);
+
+/**
+ * pardata_strobe_6800_write - using the 6800 interface create
+ *                             an enable strobe
+ *
+ * The interface requires that read + write and chipselect are
+ * setup before the strobe of "e".
+ * There are timings constraints that is handled using udelay()
+ * Drivers can use this for the strobe_write callback.
+ *
+ * @pdd: pardata data
+ */
+void pardata_strobe_6800_write(struct pardata_data *pdd);
+
+/**
+ * pardata_poweron_reset - poweron and reset display
+ *
+ * @pdd: pardata data
+ *
+ * This function enables the regulator if used and does a hardware reset.
+ * Returns:
+ * Zero on success, or a negative error code.
+ */
+int pardata_poweron_reset(struct pardata_data *pdd);
+
+/**
+ * pardata_enable_flush - enable helper
+ *
+ * @pdd: parDATA DATA
+ * @crtc_state: crtc state
+ * @plane_state: plane state
+ *
+ * This function sets &pardata->enabled, flushes the whole framebuffer and
+ * enables the backlight. Drivers can use this in their
+ * &drm_simple_display_pipe_funcs->enable callback.
+ */
+void pardata_enable_flush(struct pardata_data *pdd,
+			  struct drm_crtc_state *crtc_state,
+			  struct drm_plane_state *plane_state);
+
+/**
+ * pardata_pipe_disable - pipe disable helper
+ *
+ * @pipe: Display pipe
+ *
+ * This function disables backlight if present, if not the display memory is
+ * blanked. The regulator is disabled if in use. Drivers can use this as their
+ * &drm_simple_display_pipe_funcs->disable callback.
+ */
+void pardata_pipe_disable(struct drm_simple_display_pipe *pipe);
+
+/**
+ * pardata_init - initialization
+ *
+ * @dev: Parent device
+ * @pdd: pardata data to initialize
+ * @pipe_funcs: Display pipe functions
+ * @driver: DRM driver
+ * @mode: Display mode
+ *
+ * This function initializes a &pardata data structure and it's underlying
+ * @tinydrm_device. It also sets up the display pipeline.
+ *
+ * Supported formats: Emulated XRGB8888.
+ *
+ * Objects created by this function will be automatically freed on driver
+ * detach (devres).
+ *
+ * Returns:
+ * Zero on success, negative error code on failure.
+ */
+int pardata_init(struct device *dev,
+		 struct pardata_data *pdd,
+		 const struct drm_simple_display_pipe_funcs *pipe_funcs,
+		 struct drm_driver *driver,
+		 const struct drm_display_mode *mode);
-- 
2.12.0

[PATCH v1 4/5] dt-bindings: add winstar,wg160160 display bindings

[Sam Ravnborg]

The winstar display uses the pardata binding.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 .../bindings/display/winstar,wg160160.txt          | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/winstar,wg160160.txt

diff --git a/Documentation/devicetree/bindings/display/winstar,wg160160.txt b/Documentation/devicetree/bindings/display/winstar,wg160160.txt
new file mode 100644
index 000000000000..893065d558c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/winstar,wg160160.txt
@@ -0,0 +1,53 @@
+DT binding for Winstar WG160160
+
+This binding is for the Winstar WG160160 display using a RA8822 controller
+
+Required properties:
+- compatible: "winstar,wg160160"
+- reg: numeric identifier for the display
+
+The node for this driver must be a child node of a parallel-data-bus, hence
+all mandatory properties described in ../pardata/parallel-data-bus.txt
+must be specified.
+
+Optional properties:
+- chipselect-gpios: GPIO used for chipselect of the display
+- reset-gpios: GPIO that can be used to do a hardware reset of the display
+- backlight: phandle of the backlight device attached to the panel
+- regulator: phandle of the regulator that provides the supply voltage
+
+Example:
+
+	backlight: backlight {
+		compatible = "gpio-backlight";
+	}
+
+	power: regulator@0 {
+	}
+
+	pardatabus {
+		compatible = "parallel-data-bus";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pardata>;
+		data-gpios = <&pioe 0 GPIO_ACTIVE_HIGH
+			      &pioe 1 GPIO_ACTIVE_HIGH
+			      &pioe 2 GPIO_ACTIVE_HIGH
+			      &pioe 3 GPIO_ACTIVE_HIGH
+			      &pioe 4 GPIO_ACTIVE_HIGH
+			      &pioe 5 GPIO_ACTIVE_HIGH
+			      &pioe 6 GPIO_ACTIVE_HIGH
+			      &pioe 7 GPIO_ACTIVE_HIGH>;
+		enable-gpios = <&pioe 8 GPIO_ACTIVE_HIGH>;
+		rs-gpios = <&pioe 12 GPIO_ACTIVE_HIGH>;
+		readdwrite-gpios = <&pioe 11 GPIO_ACTIVE_HIGH>;
+
+		wg160160@0 {
+			compatible = "winstar,wg160160";
+			reg = <1>;
+			reset-gpios = <&pioe 13 GPIO_ACTIVE_LOW>;
+			chipselect-gpios = <&pioe 9 GPIO_ACTIVE_LOW>;
+			backlight = &backlight;
+			power = &power;
+		}
+	}
+
-- 
2.12.0

[PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Sam Ravnborg]

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

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
---
 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

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ba7ff7c3e46..d77e53041395 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15501,6 +15501,11 @@ L:	linux-watchdog@vger.kernel.org
 S:	Maintained
 F:	drivers/watchdog/ebc-c384_wdt.c
 
+WINSTAR WG160160 DRIVER
+M:	Sam Ravnborg <sam@ravnborg.org>
+S:	Maintained
+F:	drivers/gpu/drm/tinydrm/wg160160.c
+
 WINSYSTEMS WS16C48 GPIO DRIVER
 M:	William Breathitt Gray <vilhelm.gray@gmail.com>
 L:	linux-gpio@vger.kernel.org
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index 435de2f8d8f5..40315680c0bc 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -65,3 +65,13 @@ config TINYDRM_ST7735R
 	  * JD-T18003-T01 1.8" 128x160 TFT
 
 	  If M is selected the module will be called st7735r.
+
+config TINYDRM_WG160160
+	tristate "DRM support for Winstar WG160160"
+	depends on DRM_TINYDRM && PARDATA
+	select TINYDRM_PARDATA_DBI
+	help
+	  DRM driver for Winstar WG160106.
+	  See https://www.winstar.com.tw/products/graphic-lcd-display-module/lcd-graphics.html
+
+	  If M is selected the module will be named wg160160.
diff --git a/drivers/gpu/drm/tinydrm/Makefile b/drivers/gpu/drm/tinydrm/Makefile
index 0b52df08b0a4..849891fe40cb 100644
--- a/drivers/gpu/drm/tinydrm/Makefile
+++ b/drivers/gpu/drm/tinydrm/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_TINYDRM_MI0283QT)		+= mi0283qt.o
 obj-$(CONFIG_TINYDRM_REPAPER)		+= repaper.o
 obj-$(CONFIG_TINYDRM_ST7586)		+= st7586.o
 obj-$(CONFIG_TINYDRM_ST7735R)		+= st7735r.o
+obj-$(CONFIG_TINYDRM_WG160160)		+= wg160160.o
diff --git a/drivers/gpu/drm/tinydrm/wg160160.c b/drivers/gpu/drm/tinydrm/wg160160.c
new file mode 100644
index 000000000000..5477c8ed5599
--- /dev/null
+++ b/drivers/gpu/drm/tinydrm/wg160160.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DRM driver for Winstar WG160160 panels
+ *
+ * Copyright 2018 Sam Ravnborg <sam@ravnborg.org>
+ *
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/backlight.h>
+#include <linux/property.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/tinydrm/tinydrm-helpers.h>
+#include <drm/tinydrm/pardata-dbi.h>
+#include <drm/drm_fb_helper.h>
+
+#define WG160160_MODE_REG		0x00
+#define MODE_GRAPHIC_DISP_OFF		0x12
+#define MODE_GRAPHIC_DISP_ON		0x32
+
+#define WG160160_PITCH_REG		0x01
+#define WG160160_WIDTH_REG		0x02
+#define WG160160_DUTY_REG		0x03
+#define WG160160_CURSORPOS_REG		0x04
+#define WG160160_ADDRSL_REG		0x08 /* Start lower address */
+#define WG160160_ADDRSU_REG		0x09 /* Start upper address */
+#define WG160160_WRITE_REG		0x0c /* Write byte, inc cursor */
+#define WG160160_READ_REG		0x0d /* Read byte, inc cursor */
+
+#define WG160160_BUSY_MASK		0x80
+#define BUSY_ACTIVE			1
+#define BUSY_INACTIVE			0
+
+/**
+ * busy_status -read status of busy flag
+ *
+ * @pdd: pardata data
+ *
+ * returns: true if busy, false if not
+ */
+static bool busy_status(struct pardata_data *pdd)
+{
+	int data;
+
+	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
+	gpiod_set_value_cansleep(pdd->bus->pin_readwrite, 1);
+	gpiod_set_value_cansleep(pdd->pin_cs, 0);
+	/* min 90 nsec from cs/rs/rw to e */
+	udelay(1);
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 1);
+	/* data setup time 220 ns*/
+	udelay(2);
+
+	data = gpiod_get_value(pdd->bus->data_pins->desc[PIN_DB7]);
+
+	gpiod_set_value_cansleep(pdd->bus->pin_enable, 0);
+	/* data hold time 20 ns */
+	udelay(1);
+	gpiod_set_value_cansleep(pdd->pin_cs, 1);
+
+	return data == 1;
+}
+
+/**
+ * wait_busy - wait until controller is no longer busy
+ *
+ * Logs an ERROR once if we fail to see a not BUSY condition
+ *
+ * @pdd: pardata_data
+ */
+static void wait_busy(struct pardata_data *pdd)
+{
+	int i;
+
+	i = 0;
+
+	while (busy_status(pdd) && i++ < 10)
+		udelay(1);
+
+	if (i >= 10)
+		DRM_DEV_ERROR_RATELIMITED(&pdd->pddev->dev,
+					  "Timeout waiting for BUSY=0\n");
+}
+
+/**
+ * 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);
+	wait_busy(pdd);
+	pardata_strobe_write(pdd);
+
+	return 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;
+}
+
+static void wg160160_pipe_enable(struct drm_simple_display_pipe *pipe,
+				 struct drm_crtc_state *crtc_state,
+				 struct drm_plane_state *plane_state)
+{
+	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
+	struct pardata_data *pdd = pardata_from_tinydrm(tdev);
+	int ret;
+
+	ret = pardata_poweron_reset(pdd);
+	if (ret)
+		return;
+
+	/* Init quence for WG160160 display */
+
+	/* Graphics mode, Master */
+	pardata_write_reg(pdd, WG160160_MODE_REG, MODE_GRAPHIC_DISP_ON);
+	/* Set PITCH to 8 bits/bytes */
+	pardata_write_reg(pdd, WG160160_PITCH_REG, 0x7);
+	/* Duty cycle is the vertical resolution */
+	pardata_write_reg(pdd, WG160160_DUTY_REG, 160);
+
+	/* Start address in display RAM */
+	pardata_write_reg(pdd, WG160160_ADDRSL_REG, 0x0);
+	pardata_write_reg(pdd, WG160160_ADDRSU_REG, 0x0);
+
+	pardata_enable_flush(pdd, crtc_state, plane_state);
+}
+
+static const struct drm_simple_display_pipe_funcs wg160160_pipe_funcs = {
+	.enable		= wg160160_pipe_enable,
+	.disable	= pardata_pipe_disable,
+	.update		= tinydrm_display_pipe_update,
+	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
+};
+
+static const struct drm_display_mode wg160160_mode = {
+	TINYDRM_MODE(160, 160, 62, 62),
+};
+
+DEFINE_DRM_GEM_CMA_FOPS(wg160160_fops);
+
+static struct drm_driver wg160160_drm_driver = {
+	.driver_features	= DRIVER_GEM |
+				  DRIVER_MODESET |
+				  DRIVER_PRIME |
+				  DRIVER_ATOMIC,
+	.fops			= &wg160160_fops,
+	TINYDRM_GEM_DRIVER_OPS,
+	.lastclose		= drm_fb_helper_lastclose,
+	.name			= "wg160160",
+	.desc			= "Winstar WG160160",
+	.date			= "20180808",
+	.major			= 1,
+	.minor			= 0,
+};
+
+static const struct of_device_id wg160160_of_match[] = {
+	{ .compatible = "winstar,wg160160" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, wg160160_of_match);
+
+static struct pardata_driver wg160160_pardata_driver;
+
+static int wg160160_probe(struct pardata_device *pddev)
+{
+	struct device *dev = &pddev->dev;
+	struct pardata_data *pdd;
+	int ret;
+
+	pdd = devm_kzalloc(dev, sizeof(*pdd), GFP_KERNEL);
+	if (!pdd)
+		return -ENOMEM;
+
+	/* Find all pins */
+	pdd->pin_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(pdd->pin_reset)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
+		return PTR_ERR(pdd->pin_reset);
+	}
+
+	pdd->pin_cs = devm_gpiod_get(dev, "cs", GPIOD_OUT_HIGH);
+	if (IS_ERR(pdd->pin_cs)) {
+		DRM_DEV_ERROR(dev, "Failed to get gpio 'rs'\n");
+		return PTR_ERR(pdd->pin_cs);
+	}
+
+	pdd->backlight = devm_of_find_backlight(dev);
+	if (IS_ERR(pdd->backlight))
+		return PTR_ERR(pdd->backlight);
+
+	pdd->strobe_write = pardata_strobe_8080_write;
+	pdd->write_reg = write_reg;
+	pdd->write_buf = write_buf;
+
+	ret = pardata_init(dev, pdd, &wg160160_pipe_funcs,
+			   &wg160160_drm_driver, &wg160160_mode);
+	if (ret)
+		return ret;
+
+	pardata_dev_set_drvdata(pddev, pdd);
+
+	return devm_tinydrm_register(&pdd->tinydrm);
+}
+
+static void wg160160_shutdown(struct pardata_device *pddev)
+{
+	struct pardata_data *dpp = pardata_dev_get_drvdata(pddev);
+
+	tinydrm_shutdown(&dpp->tinydrm);
+}
+
+static struct pardata_driver wg160160_pardata_driver = {
+	.driver = {
+		.name = "wg160160",
+		.owner = THIS_MODULE,
+		.of_match_table = wg160160_of_match,
+	},
+	.probe = wg160160_probe,
+	.shutdown = wg160160_shutdown,
+};
+module_pardata_driver(wg160160_pardata_driver);
+
+MODULE_DESCRIPTION("Winstar WG160160 DRM driver");
+MODULE_AUTHOR("Sam Ravnborg <sam@ravnborg.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.12.0

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Dan Carpenter]

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.

9fcebca9 Sam Ravnborg 2018-08-02  146  
9fcebca9 Sam Ravnborg 2018-08-02  147  	/* prepare to write data */
9fcebca9 Sam Ravnborg 2018-08-02  148  	for (i = 0; i < PIN_NUM; i++)
9fcebca9 Sam Ravnborg 2018-08-02  149  		ins[PIN_DB0 + i] = !!(WG160160_WRITE_REG & BIT(i));
9fcebca9 Sam Ravnborg 2018-08-02  150  
9fcebca9 Sam Ravnborg 2018-08-02  151  	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
9fcebca9 Sam Ravnborg 2018-08-02  152  	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
9fcebca9 Sam Ravnborg 2018-08-02  153  	wait_busy(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  154  	pardata_strobe_write(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  155  
9fcebca9 Sam Ravnborg 2018-08-02  156  	/* Write data byte - by byte */
9fcebca9 Sam Ravnborg 2018-08-02  157  	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
9fcebca9 Sam Ravnborg 2018-08-02  158  
9fcebca9 Sam Ravnborg 2018-08-02  159  	for (i = offset; i < (offset + len); i++) {
9fcebca9 Sam Ravnborg 2018-08-02  160  		for (bit = 0; bit < PIN_NUM; bit++)
9fcebca9 Sam Ravnborg 2018-08-02  161  			val[PIN_DB0 + bit] = !!(data[i] & BIT(bit));
9fcebca9 Sam Ravnborg 2018-08-02  162  
9fcebca9 Sam Ravnborg 2018-08-02  163  		gpiod_set_array_value_cansleep(PIN_NUM,
9fcebca9 Sam Ravnborg 2018-08-02  164  					       pdd->bus->data_pins->desc,
9fcebca9 Sam Ravnborg 2018-08-02  165  					       val);
9fcebca9 Sam Ravnborg 2018-08-02  166  		wait_busy(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  167  		pardata_strobe_write(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  168  	}
9fcebca9 Sam Ravnborg 2018-08-02  169  
9fcebca9 Sam Ravnborg 2018-08-02  170  	return 0;
9fcebca9 Sam Ravnborg 2018-08-02  171  }
9fcebca9 Sam Ravnborg 2018-08-02  172  

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Dan Carpenter]

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.

9fcebca9 Sam Ravnborg 2018-08-02  146  
9fcebca9 Sam Ravnborg 2018-08-02  147  	/* prepare to write data */
9fcebca9 Sam Ravnborg 2018-08-02  148  	for (i = 0; i < PIN_NUM; i++)
9fcebca9 Sam Ravnborg 2018-08-02  149  		ins[PIN_DB0 + i] = !!(WG160160_WRITE_REG & BIT(i));
9fcebca9 Sam Ravnborg 2018-08-02  150  
9fcebca9 Sam Ravnborg 2018-08-02  151  	gpiod_set_value_cansleep(pdd->bus->pin_rs, 1);
9fcebca9 Sam Ravnborg 2018-08-02  152  	gpiod_set_array_value_cansleep(PIN_NUM, pdd->bus->data_pins->desc, ins);
9fcebca9 Sam Ravnborg 2018-08-02  153  	wait_busy(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  154  	pardata_strobe_write(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  155  
9fcebca9 Sam Ravnborg 2018-08-02  156  	/* Write data byte - by byte */
9fcebca9 Sam Ravnborg 2018-08-02  157  	gpiod_set_value_cansleep(pdd->bus->pin_rs, 0);
9fcebca9 Sam Ravnborg 2018-08-02  158  
9fcebca9 Sam Ravnborg 2018-08-02  159  	for (i = offset; i < (offset + len); i++) {
9fcebca9 Sam Ravnborg 2018-08-02  160  		for (bit = 0; bit < PIN_NUM; bit++)
9fcebca9 Sam Ravnborg 2018-08-02  161  			val[PIN_DB0 + bit] = !!(data[i] & BIT(bit));
9fcebca9 Sam Ravnborg 2018-08-02  162  
9fcebca9 Sam Ravnborg 2018-08-02  163  		gpiod_set_array_value_cansleep(PIN_NUM,
9fcebca9 Sam Ravnborg 2018-08-02  164  					       pdd->bus->data_pins->desc,
9fcebca9 Sam Ravnborg 2018-08-02  165  					       val);
9fcebca9 Sam Ravnborg 2018-08-02  166  		wait_busy(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  167  		pardata_strobe_write(pdd);
9fcebca9 Sam Ravnborg 2018-08-02  168  	}
9fcebca9 Sam Ravnborg 2018-08-02  169  
9fcebca9 Sam Ravnborg 2018-08-02  170  	return 0;
9fcebca9 Sam Ravnborg 2018-08-02  171  }
9fcebca9 Sam Ravnborg 2018-08-02  172  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Sam Ravnborg]

On Mon, Aug 06, 2018 at 12:15:48PM +0300, Dan Carpenter wrote:
> 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.

Thanks!
I will fix both issues in v2.

	Sam

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Noralf Trønnes]

Den 02.08.2018 21.45, skrev Sam Ravnborg:
> Add driver for the winstar wg160160 display.
> The driver utilises pardata-dbi that
> again utilise the pardata subsystem.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> ---
>   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
>

[...]

> +
> +/**
> + * 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);
> +	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.

Noralf.

> +
> +/**
> + * 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;
> +}

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Sam Ravnborg]

Hi Noralf.

On Tue, Aug 07, 2018 at 07:35:30PM +0200, Noralf Trønnes wrote:
> 
> Den 02.08.2018 21.45, skrev Sam Ravnborg:
> >Add driver for the winstar wg160160 display.
> >The driver utilises pardata-dbi that
> >again utilise the pardata subsystem.
> >
> >Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> >---
> >  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
> >
> 
> [...]
> 
> >+
> >+/**
> >+ * 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);
> >+	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

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Noralf Trønnes]

Den 08.08.2018 10.32, skrev Sam Ravnborg:
> Hi Noralf.
>
> On Tue, Aug 07, 2018 at 07:35:30PM +0200, Noralf Trønnes wrote:
>> Den 02.08.2018 21.45, skrev Sam Ravnborg:
>>> Add driver for the winstar wg160160 display.
>>> The driver utilises pardata-dbi that
>>> again utilise the pardata subsystem.
>>>
>>> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
>>> ---
>>>   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
>>>
>> [...]
>>
>>> +
>>> +/**
>>> + * 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);
>>> +	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?

I was thinking of a regmap-pardata.c like drivers/base/regmap/regmap-i2c.c.
It's not much code an we would avoid having generic register access
functions in DRM. It's not about speed or caching, but just to try and
put code where it logically belongs.

Here is one experiment where I have created a regmap on a 8080 bus:
https://github.com/notro/tinydrm/blob/master/tinydrm-regmap.c
Used here: https://github.com/notro/tinydrm/blob/master/fb_ili9325.c

I don't advocate this particular approach, I mention it just as an
example of using regmap in a tinydrm driver.

Noralf.

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v1 5/5] tinydrm: add winstar wg160160 driver

[Noralf Trønnes]

Den 08.08.2018 10.32, skrev Sam Ravnborg:
> Hi Noralf.
>
> On Tue, Aug 07, 2018 at 07:35:30PM +0200, Noralf Trønnes wrote:
>> Den 02.08.2018 21.45, skrev Sam Ravnborg:
>>> Add driver for the winstar wg160160 display.
>>> The driver utilises pardata-dbi that
>>> again utilise the pardata subsystem.
>>>
>>> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
>>> ---
>>>   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
>>>
>> [...]
>>
>>> +
>>> +/**
>>> + * 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);
>>> +	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?

I was thinking of a regmap-pardata.c like drivers/base/regmap/regmap-i2c.c.
It's not much code an we would avoid having generic register access
functions in DRM. It's not about speed or caching, but just to try and
put code where it logically belongs.

Here is one experiment where I have created a regmap on a 8080 bus:
https://github.com/notro/tinydrm/blob/master/tinydrm-regmap.c
Used here: https://github.com/notro/tinydrm/blob/master/fb_ili9325.c

I don't advocate this particular approach, I mention it just as an
example of using regmap in a tinydrm driver.

Noralf.

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

Re: [RFC PATCH v1 0/5] Add pardata bus + tinydrm driver

[Miguel Ojeda]

Hi Sam,

On Thu, Aug 2, 2018 at 9:39 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> 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/

Just a quick note: the original intention of auxdisplay/ was not only
text displays (actually, the first drivers there --cfag12864b/ks0108--
were graphic displays). They have a very low resolution (e.g. 128x64
for cfag12864b), though.

Cheers,
Miguel

> 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(+)