All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh
  Cc: Jaghathiswari Rankappagounder Natarajan, devicetree,
	linux-kernel, linux-arm-kernel, joel

This patchset includes:

Documentation for the binding which provides an interface for adding clock,
data and clear signal GPIO lines to control seven segment display.

The platform device driver provides an API for displaying on two 7-segment
displays, and implements the required bit-banging. The hardware assumed is
74HC164 wired to two 7-segment displays.

The character device driver implements the user-space API for letting a user
write to two 7-segment displays including any conversion methods necessary
to map the user input to two 7-segment displays.

Adding clock, data and clear signal GPIO lines in the devicetree to control
seven segment display on zaius platform.

The platform driver matches on the device tree node; the platform driver also
initializes the character device.

Tested that the seven segment display works properly by writing to the
character device file on a EVB AST2500 board which also has 74HC164 wired
to two 7-segment displays.

Jaghathiswari Rankappagounder Natarajan (4):
  Documentation: dt-bindings: Document bindings for seven segment
    display support
  drivers: misc: Character device driver for seven segment display
  drivers: misc: Platform driver for seven segment display support
  arm: dts: Add dt-binding to support seven segment display on zaius

 .../devicetree/bindings/misc/seven-seg-gpio.txt    |  27 +++
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts         |   8 +
 drivers/misc/Kconfig                               |  16 ++
 drivers/misc/Makefile                              |   2 +
 drivers/misc/seven_seg_disp.c                      | 197 ++++++++++++++++++++
 drivers/misc/seven_seg_disp.h                      |  34 ++++
 drivers/misc/seven_seg_gpio.c                      | 206 +++++++++++++++++++++
 7 files changed, 490 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h
 create mode 100644 drivers/misc/seven_seg_gpio.c

--
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset includes:

Documentation for the binding which provides an interface for adding clock,
data and clear signal GPIO lines to control seven segment display.

The platform device driver provides an API for displaying on two 7-segment
displays, and implements the required bit-banging. The hardware assumed is
74HC164 wired to two 7-segment displays.

The character device driver implements the user-space API for letting a user
write to two 7-segment displays including any conversion methods necessary
to map the user input to two 7-segment displays.

Adding clock, data and clear signal GPIO lines in the devicetree to control
seven segment display on zaius platform.

The platform driver matches on the device tree node; the platform driver also
initializes the character device.

Tested that the seven segment display works properly by writing to the
character device file on a EVB AST2500 board which also has 74HC164 wired
to two 7-segment displays.

Jaghathiswari Rankappagounder Natarajan (4):
  Documentation: dt-bindings: Document bindings for seven segment
    display support
  drivers: misc: Character device driver for seven segment display
  drivers: misc: Platform driver for seven segment display support
  arm: dts: Add dt-binding to support seven segment display on zaius

 .../devicetree/bindings/misc/seven-seg-gpio.txt    |  27 +++
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts         |   8 +
 drivers/misc/Kconfig                               |  16 ++
 drivers/misc/Makefile                              |   2 +
 drivers/misc/seven_seg_disp.c                      | 197 ++++++++++++++++++++
 drivers/misc/seven_seg_disp.h                      |  34 ++++
 drivers/misc/seven_seg_gpio.c                      | 206 +++++++++++++++++++++
 7 files changed, 490 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h
 create mode 100644 drivers/misc/seven_seg_gpio.c

--
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 1/4] Documentation: dt-bindings: Document bindings for seven segment display support
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh
  Cc: Jaghathiswari Rankappagounder Natarajan, devicetree,
	linux-kernel, linux-arm-kernel, joel

This binding provides interface for adding clock, data and clear signal GPIO
lines to control seven segment display.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 .../devicetree/bindings/misc/seven-seg-gpio.txt    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt

diff --git a/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
new file mode 100644
index 0000000..9a4e255
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
@@ -0,0 +1,27 @@
+This binding defines interface to add clock, data and clear GPIO lines required
+for seven segment display support.
+
+Required properties:
+- compatible : should be "seven-seg-gpio-dev".
+- clock-gpios :  Should specify the GPIO pin connected to the Clock line on the
+  hardware.
+- data-gpios : Should specify the GPIO pin connected to Data line on the
+  hardware.
+- clear-gpios : Should specify the GPIO pin connected to Clear line on the
+  hardware.
+
+Optional properties:
+- refresh-interval-ms : The interval at which to refresh the display.
+  If this property is not present, the default value is 1000.
+
+Examples:
+
+#include <dt-bindings/gpio/gpio.h>
+
+seven-seg-disp {
+	compatible = "seven-seg-gpio-dev";
+	refresh-interval-ms = "1000";
+	clock-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
+	data-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+	clear-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+};
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 1/4] Documentation: dt-bindings: Document bindings for seven segment display support
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	arnd-r2nGTMty4D4, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: Jaghathiswari Rankappagounder Natarajan,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg

This binding provides interface for adding clock, data and clear signal GPIO
lines to control seven segment display.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/misc/seven-seg-gpio.txt    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt

diff --git a/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
new file mode 100644
index 0000000..9a4e255
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
@@ -0,0 +1,27 @@
+This binding defines interface to add clock, data and clear GPIO lines required
+for seven segment display support.
+
+Required properties:
+- compatible : should be "seven-seg-gpio-dev".
+- clock-gpios :  Should specify the GPIO pin connected to the Clock line on the
+  hardware.
+- data-gpios : Should specify the GPIO pin connected to Data line on the
+  hardware.
+- clear-gpios : Should specify the GPIO pin connected to Clear line on the
+  hardware.
+
+Optional properties:
+- refresh-interval-ms : The interval at which to refresh the display.
+  If this property is not present, the default value is 1000.
+
+Examples:
+
+#include <dt-bindings/gpio/gpio.h>
+
+seven-seg-disp {
+	compatible = "seven-seg-gpio-dev";
+	refresh-interval-ms = "1000";
+	clock-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
+	data-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+	clear-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+};
--
2.8.0.rc3.226.g39d4020

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 1/4] Documentation: dt-bindings: Document bindings for seven segment display support
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

This binding provides interface for adding clock, data and clear signal GPIO
lines to control seven segment display.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 .../devicetree/bindings/misc/seven-seg-gpio.txt    | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt

diff --git a/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
new file mode 100644
index 0000000..9a4e255
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
@@ -0,0 +1,27 @@
+This binding defines interface to add clock, data and clear GPIO lines required
+for seven segment display support.
+
+Required properties:
+- compatible : should be "seven-seg-gpio-dev".
+- clock-gpios :  Should specify the GPIO pin connected to the Clock line on the
+  hardware.
+- data-gpios : Should specify the GPIO pin connected to Data line on the
+  hardware.
+- clear-gpios : Should specify the GPIO pin connected to Clear line on the
+  hardware.
+
+Optional properties:
+- refresh-interval-ms : The interval at which to refresh the display.
+  If this property is not present, the default value is 1000.
+
+Examples:
+
+#include <dt-bindings/gpio/gpio.h>
+
+seven-seg-disp {
+	compatible = "seven-seg-gpio-dev";
+	refresh-interval-ms = "1000";
+	clock-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
+	data-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+	clear-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+};
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh
  Cc: Jaghathiswari Rankappagounder Natarajan, devicetree,
	linux-kernel, linux-arm-kernel, joel

Character device driver which implements the user-space
API for letting a user write to two 7-segment displays including
any conversion methods necessary to map the user input
to two 7-segment displays.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 drivers/misc/Kconfig          |   8 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_disp.c | 197 ++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/seven_seg_disp.h |  34 ++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a216b46..a21aec1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -791,6 +791,14 @@ config PANEL_CHANGE_MESSAGE
 	  If you say 'Y' here, you'll be able to choose a message yourself. Otherwise,
 	  say 'N' and keep the default message with the version.

+config SEVEN_SEGMENT_DISPLAY
+	tristate "Character driver for seven segment display support"
+	help
+	  Character device driver which implements the user-space
+	  API for letting a user write to two 7-segment displays including
+	  any conversion methods necessary to map the user input
+	  to two 7-segment displays.
+
 config PANEL_BOOT_MESSAGE
 	depends on PANEL && PANEL_CHANGE_MESSAGE="y"
 	string "New initialization message"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b2fb6dbf..c2defbd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,4 +56,5 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)	+= seven_seg_disp.o
 obj-$(CONFIG_PANEL)             += panel.o
diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.c
new file mode 100644
index 0000000..4daeac5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/ctype.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+
+#include "seven_seg_disp.h"
+
+#define LED_DOT 0x01
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *  _       _   _       _   _   _   _   _   _       _       _   _
+ * | |   |  _|  _| |_| |_  |_    | |_| |_| |_| |_  |    _| |_  |_
+ * |_|   | |_   _|   |  _| |_|   | |_|   | | | |_| |_  |_| |_  |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 seven_seg_bits[] = {
+	0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
+	0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
+	};
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *      _       _   _                              _            _
+ *     |   |_  |_| |_  _   _   _   _   _   _   _  |_    _|  _| | |
+ *     |_  |_  |   |                               _|  |_| |_| | |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 special_seven_seg_bits[] = {
+	0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
+	0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
+	};
+
+static dev_t seven_seg_devno;
+static struct class *seven_seg_disp_class;
+
+static int seven_seg_disp_open(struct inode *inode, struct file *filp)
+{
+	struct seven_seg_disp_dev *disp_dev;
+
+	disp_dev = container_of(inode->i_cdev,
+				 struct seven_seg_disp_dev, cdev);
+	filp->private_data = disp_dev;
+	return 0;
+}
+
+static int seven_seg_disp_close(struct inode *inode, struct file *filp)
+{
+	filp->private_data = NULL;
+	return 0;
+}
+
+static ssize_t seven_seg_disp_read(struct file *filp, char __user *buf, size_t
+				len, loff_t *off)
+{
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (disp_dev->disp_data_valid)
+		return -EINVAL;
+
+	if (copy_to_user(buf, disp_dev->seven_seg_disp_data_array,
+				MAX_DISP_CHAR_SIZE) != 0) {
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+	u8 low_display;
+	u8 high_display;
+	u16 led_value;
+
+	low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+	high_display = (buf[0] == '1') ?
+	special_seven_seg_bits[hex_to_bin(buf[1])] :
+	seven_seg_bits[hex_to_bin(buf[1])];
+
+	led_value = low_display | (high_display << 8);
+	if (buf[0] == '1')
+		led_value |= LED_DOT | (LED_DOT << 8);
+
+	return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user *buf,
+				size_t len, loff_t *off)
+{
+	int length = len - 1;
+	int i;
+
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (length != MAX_DISP_CHAR_SIZE)
+		return -EINVAL;
+
+	if (copy_from_user(disp_dev->seven_seg_disp_data_array,
+				buf, length) != 0) {
+		return -EFAULT;
+	}
+
+	for (i = 0; i < MAX_DISP_CHAR_SIZE; i++) {
+		if (!isxdigit(disp_dev->seven_seg_disp_data_array[i]))
+			return -EINVAL;
+	}
+
+	disp_dev->current_seven_seg_disp_data = convert_to_disp_data(
+			disp_dev->seven_seg_disp_data_array);
+	disp_dev->disp_data_valid = true;
+	disp_dev->update_seven_seg_data(&disp_dev->parent,
+			disp_dev->current_seven_seg_disp_data);
+
+	return len;
+}
+
+static const struct file_operations seven_seg_disp_fops = {
+
+	.owner = THIS_MODULE,
+	.open = seven_seg_disp_open,
+	.release = seven_seg_disp_close,
+	.read = seven_seg_disp_read,
+	.write = seven_seg_disp_write
+};
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev)
+{
+	cdev_del(&disp_dev->cdev);
+	device_destroy(seven_seg_disp_class, seven_seg_devno);
+}
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data))
+{
+	struct device *dev;
+	int err;
+
+	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
+			seven_seg_devno,
+			NULL, "seven_seg_disp_val");
+	if (dev == NULL)
+		return -1;
+	disp_dev->dev = dev;
+	disp_dev->update_seven_seg_data = update_disp_data;
+	disp_dev->disp_data_valid = false;
+
+	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
+	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
+	if (err)
+		device_destroy(seven_seg_disp_class, seven_seg_devno);
+	return err;
+}
+
+static int __init seven_seg_disp_init(void)
+{
+	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
+		return -1;
+
+	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
+	if (seven_seg_disp_class == NULL)
+		goto unreg_chrdev;
+
+unreg_chrdev:
+	unregister_chrdev_region(seven_seg_devno, 1);
+	return -1;
+}
+
+static void __exit seven_seg_disp_exit(void)
+{
+	class_destroy(seven_seg_disp_class);
+	unregister_chrdev_region(seven_seg_devno, 1);
+}
+
+module_init(seven_seg_disp_init);
+module_exit(seven_seg_disp_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display character driver");
diff --git a/drivers/misc/seven_seg_disp.h b/drivers/misc/seven_seg_disp.h
new file mode 100644
index 0000000..b0f93c5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef SEVEN_SEG_DISP_H
+#define SEVEN_SEG_DISP_H
+
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+#define MAX_DISP_CHAR_SIZE 3
+
+#define DEFAULT_REFRESH_INTERVAL_MS 1000
+
+struct seven_seg_disp_dev {
+	bool disp_data_valid;
+	u16 current_seven_seg_disp_data;
+	char seven_seg_disp_data_array[MAX_DISP_CHAR_SIZE];
+	struct device parent;
+	struct device *dev;
+	struct cdev cdev;
+	void (*update_seven_seg_data)(struct device *, u16 data);
+};
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data));
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	arnd-r2nGTMty4D4, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: Jaghathiswari Rankappagounder Natarajan,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg

Character device driver which implements the user-space
API for letting a user write to two 7-segment displays including
any conversion methods necessary to map the user input
to two 7-segment displays.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 drivers/misc/Kconfig          |   8 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_disp.c | 197 ++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/seven_seg_disp.h |  34 ++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a216b46..a21aec1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -791,6 +791,14 @@ config PANEL_CHANGE_MESSAGE
 	  If you say 'Y' here, you'll be able to choose a message yourself. Otherwise,
 	  say 'N' and keep the default message with the version.

+config SEVEN_SEGMENT_DISPLAY
+	tristate "Character driver for seven segment display support"
+	help
+	  Character device driver which implements the user-space
+	  API for letting a user write to two 7-segment displays including
+	  any conversion methods necessary to map the user input
+	  to two 7-segment displays.
+
 config PANEL_BOOT_MESSAGE
 	depends on PANEL && PANEL_CHANGE_MESSAGE="y"
 	string "New initialization message"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b2fb6dbf..c2defbd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,4 +56,5 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)	+= seven_seg_disp.o
 obj-$(CONFIG_PANEL)             += panel.o
diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.c
new file mode 100644
index 0000000..4daeac5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/ctype.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+
+#include "seven_seg_disp.h"
+
+#define LED_DOT 0x01
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *  _       _   _       _   _   _   _   _   _       _       _   _
+ * | |   |  _|  _| |_| |_  |_    | |_| |_| |_| |_  |    _| |_  |_
+ * |_|   | |_   _|   |  _| |_|   | |_|   | | | |_| |_  |_| |_  |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 seven_seg_bits[] = {
+	0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
+	0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
+	};
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *      _       _   _                              _            _
+ *     |   |_  |_| |_  _   _   _   _   _   _   _  |_    _|  _| | |
+ *     |_  |_  |   |                               _|  |_| |_| | |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 special_seven_seg_bits[] = {
+	0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
+	0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
+	};
+
+static dev_t seven_seg_devno;
+static struct class *seven_seg_disp_class;
+
+static int seven_seg_disp_open(struct inode *inode, struct file *filp)
+{
+	struct seven_seg_disp_dev *disp_dev;
+
+	disp_dev = container_of(inode->i_cdev,
+				 struct seven_seg_disp_dev, cdev);
+	filp->private_data = disp_dev;
+	return 0;
+}
+
+static int seven_seg_disp_close(struct inode *inode, struct file *filp)
+{
+	filp->private_data = NULL;
+	return 0;
+}
+
+static ssize_t seven_seg_disp_read(struct file *filp, char __user *buf, size_t
+				len, loff_t *off)
+{
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (disp_dev->disp_data_valid)
+		return -EINVAL;
+
+	if (copy_to_user(buf, disp_dev->seven_seg_disp_data_array,
+				MAX_DISP_CHAR_SIZE) != 0) {
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+	u8 low_display;
+	u8 high_display;
+	u16 led_value;
+
+	low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+	high_display = (buf[0] == '1') ?
+	special_seven_seg_bits[hex_to_bin(buf[1])] :
+	seven_seg_bits[hex_to_bin(buf[1])];
+
+	led_value = low_display | (high_display << 8);
+	if (buf[0] == '1')
+		led_value |= LED_DOT | (LED_DOT << 8);
+
+	return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user *buf,
+				size_t len, loff_t *off)
+{
+	int length = len - 1;
+	int i;
+
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (length != MAX_DISP_CHAR_SIZE)
+		return -EINVAL;
+
+	if (copy_from_user(disp_dev->seven_seg_disp_data_array,
+				buf, length) != 0) {
+		return -EFAULT;
+	}
+
+	for (i = 0; i < MAX_DISP_CHAR_SIZE; i++) {
+		if (!isxdigit(disp_dev->seven_seg_disp_data_array[i]))
+			return -EINVAL;
+	}
+
+	disp_dev->current_seven_seg_disp_data = convert_to_disp_data(
+			disp_dev->seven_seg_disp_data_array);
+	disp_dev->disp_data_valid = true;
+	disp_dev->update_seven_seg_data(&disp_dev->parent,
+			disp_dev->current_seven_seg_disp_data);
+
+	return len;
+}
+
+static const struct file_operations seven_seg_disp_fops = {
+
+	.owner = THIS_MODULE,
+	.open = seven_seg_disp_open,
+	.release = seven_seg_disp_close,
+	.read = seven_seg_disp_read,
+	.write = seven_seg_disp_write
+};
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev)
+{
+	cdev_del(&disp_dev->cdev);
+	device_destroy(seven_seg_disp_class, seven_seg_devno);
+}
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data))
+{
+	struct device *dev;
+	int err;
+
+	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
+			seven_seg_devno,
+			NULL, "seven_seg_disp_val");
+	if (dev == NULL)
+		return -1;
+	disp_dev->dev = dev;
+	disp_dev->update_seven_seg_data = update_disp_data;
+	disp_dev->disp_data_valid = false;
+
+	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
+	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
+	if (err)
+		device_destroy(seven_seg_disp_class, seven_seg_devno);
+	return err;
+}
+
+static int __init seven_seg_disp_init(void)
+{
+	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
+		return -1;
+
+	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
+	if (seven_seg_disp_class == NULL)
+		goto unreg_chrdev;
+
+unreg_chrdev:
+	unregister_chrdev_region(seven_seg_devno, 1);
+	return -1;
+}
+
+static void __exit seven_seg_disp_exit(void)
+{
+	class_destroy(seven_seg_disp_class);
+	unregister_chrdev_region(seven_seg_devno, 1);
+}
+
+module_init(seven_seg_disp_init);
+module_exit(seven_seg_disp_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Seven segment display character driver");
diff --git a/drivers/misc/seven_seg_disp.h b/drivers/misc/seven_seg_disp.h
new file mode 100644
index 0000000..b0f93c5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef SEVEN_SEG_DISP_H
+#define SEVEN_SEG_DISP_H
+
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+#define MAX_DISP_CHAR_SIZE 3
+
+#define DEFAULT_REFRESH_INTERVAL_MS 1000
+
+struct seven_seg_disp_dev {
+	bool disp_data_valid;
+	u16 current_seven_seg_disp_data;
+	char seven_seg_disp_data_array[MAX_DISP_CHAR_SIZE];
+	struct device parent;
+	struct device *dev;
+	struct cdev cdev;
+	void (*update_seven_seg_data)(struct device *, u16 data);
+};
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data));
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--
2.8.0.rc3.226.g39d4020

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

Character device driver which implements the user-space
API for letting a user write to two 7-segment displays including
any conversion methods necessary to map the user input
to two 7-segment displays.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 drivers/misc/Kconfig          |   8 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_disp.c | 197 ++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/seven_seg_disp.h |  34 ++++++++
 4 files changed, 240 insertions(+)
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a216b46..a21aec1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -791,6 +791,14 @@ config PANEL_CHANGE_MESSAGE
 	  If you say 'Y' here, you'll be able to choose a message yourself. Otherwise,
 	  say 'N' and keep the default message with the version.

+config SEVEN_SEGMENT_DISPLAY
+	tristate "Character driver for seven segment display support"
+	help
+	  Character device driver which implements the user-space
+	  API for letting a user write to two 7-segment displays including
+	  any conversion methods necessary to map the user input
+	  to two 7-segment displays.
+
 config PANEL_BOOT_MESSAGE
 	depends on PANEL && PANEL_CHANGE_MESSAGE="y"
 	string "New initialization message"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index b2fb6dbf..c2defbd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -56,4 +56,5 @@ obj-$(CONFIG_GENWQE)		+= genwqe/
 obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
+obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)	+= seven_seg_disp.o
 obj-$(CONFIG_PANEL)             += panel.o
diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.c
new file mode 100644
index 0000000..4daeac5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/ctype.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+
+#include "seven_seg_disp.h"
+
+#define LED_DOT 0x01
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *  _       _   _       _   _   _   _   _   _       _       _   _
+ * | |   |  _|  _| |_| |_  |_    | |_| |_| |_| |_  |    _| |_  |_
+ * |_|   | |_   _|   |  _| |_|   | |_|   | | | |_| |_  |_| |_  |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 seven_seg_bits[] = {
+	0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
+	0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
+	};
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *      _       _   _                              _            _
+ *     |   |_  |_| |_  _   _   _   _   _   _   _  |_    _|  _| | |
+ *     |_  |_  |   |                               _|  |_| |_| | |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 special_seven_seg_bits[] = {
+	0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
+	0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
+	};
+
+static dev_t seven_seg_devno;
+static struct class *seven_seg_disp_class;
+
+static int seven_seg_disp_open(struct inode *inode, struct file *filp)
+{
+	struct seven_seg_disp_dev *disp_dev;
+
+	disp_dev = container_of(inode->i_cdev,
+				 struct seven_seg_disp_dev, cdev);
+	filp->private_data = disp_dev;
+	return 0;
+}
+
+static int seven_seg_disp_close(struct inode *inode, struct file *filp)
+{
+	filp->private_data = NULL;
+	return 0;
+}
+
+static ssize_t seven_seg_disp_read(struct file *filp, char __user *buf, size_t
+				len, loff_t *off)
+{
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (disp_dev->disp_data_valid)
+		return -EINVAL;
+
+	if (copy_to_user(buf, disp_dev->seven_seg_disp_data_array,
+				MAX_DISP_CHAR_SIZE) != 0) {
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+	u8 low_display;
+	u8 high_display;
+	u16 led_value;
+
+	low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+	high_display = (buf[0] == '1') ?
+	special_seven_seg_bits[hex_to_bin(buf[1])] :
+	seven_seg_bits[hex_to_bin(buf[1])];
+
+	led_value = low_display | (high_display << 8);
+	if (buf[0] == '1')
+		led_value |= LED_DOT | (LED_DOT << 8);
+
+	return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user *buf,
+				size_t len, loff_t *off)
+{
+	int length = len - 1;
+	int i;
+
+	struct seven_seg_disp_dev *disp_dev = filp->private_data;
+
+	if (length != MAX_DISP_CHAR_SIZE)
+		return -EINVAL;
+
+	if (copy_from_user(disp_dev->seven_seg_disp_data_array,
+				buf, length) != 0) {
+		return -EFAULT;
+	}
+
+	for (i = 0; i < MAX_DISP_CHAR_SIZE; i++) {
+		if (!isxdigit(disp_dev->seven_seg_disp_data_array[i]))
+			return -EINVAL;
+	}
+
+	disp_dev->current_seven_seg_disp_data = convert_to_disp_data(
+			disp_dev->seven_seg_disp_data_array);
+	disp_dev->disp_data_valid = true;
+	disp_dev->update_seven_seg_data(&disp_dev->parent,
+			disp_dev->current_seven_seg_disp_data);
+
+	return len;
+}
+
+static const struct file_operations seven_seg_disp_fops = {
+
+	.owner = THIS_MODULE,
+	.open = seven_seg_disp_open,
+	.release = seven_seg_disp_close,
+	.read = seven_seg_disp_read,
+	.write = seven_seg_disp_write
+};
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev)
+{
+	cdev_del(&disp_dev->cdev);
+	device_destroy(seven_seg_disp_class, seven_seg_devno);
+}
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data))
+{
+	struct device *dev;
+	int err;
+
+	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
+			seven_seg_devno,
+			NULL, "seven_seg_disp_val");
+	if (dev == NULL)
+		return -1;
+	disp_dev->dev = dev;
+	disp_dev->update_seven_seg_data = update_disp_data;
+	disp_dev->disp_data_valid = false;
+
+	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
+	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
+	if (err)
+		device_destroy(seven_seg_disp_class, seven_seg_devno);
+	return err;
+}
+
+static int __init seven_seg_disp_init(void)
+{
+	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
+		return -1;
+
+	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
+	if (seven_seg_disp_class == NULL)
+		goto unreg_chrdev;
+
+unreg_chrdev:
+	unregister_chrdev_region(seven_seg_devno, 1);
+	return -1;
+}
+
+static void __exit seven_seg_disp_exit(void)
+{
+	class_destroy(seven_seg_disp_class);
+	unregister_chrdev_region(seven_seg_devno, 1);
+}
+
+module_init(seven_seg_disp_init);
+module_exit(seven_seg_disp_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display character driver");
diff --git a/drivers/misc/seven_seg_disp.h b/drivers/misc/seven_seg_disp.h
new file mode 100644
index 0000000..b0f93c5
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef SEVEN_SEG_DISP_H
+#define SEVEN_SEG_DISP_H
+
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+#define MAX_DISP_CHAR_SIZE 3
+
+#define DEFAULT_REFRESH_INTERVAL_MS 1000
+
+struct seven_seg_disp_dev {
+	bool disp_data_valid;
+	u16 current_seven_seg_disp_data;
+	char seven_seg_disp_data_array[MAX_DISP_CHAR_SIZE];
+	struct device parent;
+	struct device *dev;
+	struct cdev cdev;
+	void (*update_seven_seg_data)(struct device *, u16 data);
+};
+
+int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
+	void (*update_disp_data)(struct device *, u16 data));
+
+void seven_seg_rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 3/4] drivers: misc: Platform driver for seven segment display support
  2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  -1 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh
  Cc: Jaghathiswari Rankappagounder Natarajan, devicetree,
	linux-kernel, linux-arm-kernel, joel

Platform device driver which provides an API for displaying on two
7-segment displays, and implements the required bit-banging.
The hardware assumed is 74HC164 wired to two 7-segment displays.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 drivers/misc/Kconfig          |   8 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_gpio.c | 206 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 drivers/misc/seven_seg_gpio.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a21aec1..6508108 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -812,6 +812,14 @@ config PANEL_BOOT_MESSAGE
 	  An empty message will only clear the display at driver init time. Any other
 	  printf()-formatted message is valid with newline and escape codes.

+config SEVEN_SEGMENT_GPIO
+	tristate "Platform driver to update seven segment display"
+	depends on SEVEN_SEGMENT_DISPLAY
+	help
+	  Platform device driver which provides an API for displaying on two
+	  7-segment displays, and implements the required bit-banging.
+	  The hardware assumed is 74HC164 wired to two 7-segment displays.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c2defbd..d9c0d20 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)	+= seven_seg_disp.o
 obj-$(CONFIG_PANEL)             += panel.o
+obj-$(CONFIG_SEVEN_SEGMENT_GPIO)	+= seven_seg_gpio.o
diff --git a/drivers/misc/seven_seg_gpio.c b/drivers/misc/seven_seg_gpio.c
new file mode 100644
index 0000000..3dcb903
--- /dev/null
+++ b/drivers/misc/seven_seg_gpio.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/sizes.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/gpio/consumer.h>
+
+#include "seven_seg_disp.h"
+
+#define DELAY_INTVL_US 1
+
+#define CLOCK_GPIO_NAME "clock"
+#define DATA_GPIO_NAME "data"
+#define CLEAR_GPIO_NAME "clear"
+
+struct seven_seg_gpio_info {
+	u16 curr_disp_value;
+	u16 refresh_interval;
+	struct timer_list update_timer;
+	struct gpio_desc *clock_gpio;
+	struct gpio_desc *data_gpio;
+	struct gpio_desc *clear_gpio;
+};
+
+static void update_seven_seg_gpio_data(struct device *dev, u16 data)
+{
+	struct platform_device *pdev;
+	struct seven_seg_gpio_info *gpio_info;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	if (pdev == NULL) {
+		pr_err("invalid NULL platform_device\n");
+		return;
+	}
+
+	gpio_info = platform_get_drvdata(pdev);
+	if (gpio_info == NULL) {
+		pr_err("invalid NULL gpio_info\n");
+		return;
+	}
+
+	gpio_info->curr_disp_value = data;
+}
+
+static void clear_seven_seg_gpio_data(struct device *dev, u16 data)
+{
+	struct platform_device *pdev;
+	struct seven_seg_gpio_info *gpio_info;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	if (pdev == NULL) {
+		pr_err("invalid NULL platform_device\n");
+		return;
+	}
+
+	gpio_info = platform_get_drvdata(pdev);
+	if (gpio_info == NULL) {
+		pr_err("invalid NULL gpio_info\n");
+		return;
+	}
+
+	gpio_info->curr_disp_value = 0;
+}
+
+static void send_seven_seg_gpio_data(u16 disp_data,
+		struct seven_seg_gpio_info *gpio_info)
+{
+	int i;
+
+	gpiod_set_value(gpio_info->clear_gpio, 0);
+	udelay(DELAY_INTVL_US);
+	gpiod_set_value(gpio_info->clear_gpio, 1);
+	udelay(DELAY_INTVL_US);
+
+	for (i = 0; i < 16; i++) {
+		if (disp_data & 0x01)
+			gpiod_set_value(gpio_info->data_gpio, 1);
+		else
+			gpiod_set_value(gpio_info->data_gpio, 0);
+
+		udelay(DELAY_INTVL_US);
+
+		gpiod_set_value(gpio_info->clock_gpio, 0);
+		udelay(DELAY_INTVL_US);
+		gpiod_set_value(gpio_info->clock_gpio, 1);
+		udelay(DELAY_INTVL_US);
+
+		disp_data >>= 1;
+	}
+}
+
+static void disp_refresh_timer_handler(unsigned long data)
+{
+	u16 disp_data;
+	struct seven_seg_gpio_info *gpio_info =
+		(struct seven_seg_gpio_info *)data;
+	disp_data = gpio_info->curr_disp_value;
+
+	send_seven_seg_gpio_data(disp_data, gpio_info);
+	mod_timer(&gpio_info->update_timer,
+		jiffies + msecs_to_jiffies(gpio_info->refresh_interval));
+}
+
+static const struct of_device_id of_seven_seg_gpio_match[] = {
+		{ .compatible = "seven-seg-gpio-dev" },
+		{},
+};
+
+MODULE_DEVICE_TABLE(of, of_seven_seg_gpio_match);
+
+static int seven_seg_gpio_probe(struct platform_device *pdev)
+{
+	u16 interval;
+	int result;
+	struct seven_seg_gpio_info *gpio_info;
+	struct device *dev = &pdev->dev;
+	struct seven_seg_disp_dev *disp_dev;
+
+	gpio_info = devm_kzalloc(dev,
+			sizeof(struct seven_seg_gpio_info),
+			GFP_KERNEL);
+	if (gpio_info == NULL)
+		return -ENOMEM;
+
+	/* Requesting the clock gpio */
+	gpio_info->clock_gpio = devm_gpiod_get(dev, CLOCK_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->clock_gpio))
+		return PTR_ERR(gpio_info->clock_gpio);
+
+	/* Requesting the data gpio */
+	gpio_info->data_gpio = devm_gpiod_get(dev, DATA_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->data_gpio))
+		return PTR_ERR(gpio_info->data_gpio);
+
+	/* Requesting the clear gpio */
+	gpio_info->clear_gpio = devm_gpiod_get(dev, CLEAR_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->clear_gpio))
+		return PTR_ERR(gpio_info->clear_gpio);
+
+	result = of_property_read_u16(pdev->dev.of_node,
+		"refresh-interval-ms", &interval);
+	gpio_info->refresh_interval = result ? DEFAULT_REFRESH_INTERVAL_MS :
+		interval;
+
+	/* Start timer to update seven segment display every second */
+	setup_timer(&gpio_info->update_timer, disp_refresh_timer_handler,
+			(unsigned long)gpio_info);
+	result = mod_timer(&gpio_info->update_timer,
+			jiffies +
+			msecs_to_jiffies(gpio_info->refresh_interval));
+	if (result)
+		return result;
+
+	gpio_info->curr_disp_value = 0;
+
+	platform_set_drvdata(pdev, gpio_info);
+
+	disp_dev = devm_kzalloc(dev, sizeof(struct seven_seg_disp_dev),
+				GFP_KERNEL);
+	disp_dev->parent = *dev;
+	seven_seg_setup_cdev(disp_dev, &update_seven_seg_gpio_data);
+	return 0;
+}
+
+static int seven_seg_gpio_remove(struct platform_device *pdev)
+{
+	struct seven_seg_gpio_info *gpio_info = platform_get_drvdata(pdev);
+	struct seven_seg_disp_dev *disp_dev =
+				container_of(&pdev->dev,
+				struct seven_seg_disp_dev, parent);
+	seven_seg_rem_cdev(disp_dev);
+	del_timer_sync(&gpio_info->update_timer);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+static struct platform_driver seven_seg_gpio_driver = {
+	.probe		= seven_seg_gpio_probe,
+	.remove		= seven_seg_gpio_remove,
+	.driver		= {
+		.name	= "seven-seg-gpio",
+		.of_match_table = of_seven_seg_gpio_match,
+	},
+};
+
+module_platform_driver(seven_seg_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display driver using GPIO config");
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 3/4] drivers: misc: Platform driver for seven segment display support
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

Platform device driver which provides an API for displaying on two
7-segment displays, and implements the required bit-banging.
The hardware assumed is 74HC164 wired to two 7-segment displays.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 drivers/misc/Kconfig          |   8 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_gpio.c | 206 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+)
 create mode 100644 drivers/misc/seven_seg_gpio.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index a21aec1..6508108 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -812,6 +812,14 @@ config PANEL_BOOT_MESSAGE
 	  An empty message will only clear the display at driver init time. Any other
 	  printf()-formatted message is valid with newline and escape codes.

+config SEVEN_SEGMENT_GPIO
+	tristate "Platform driver to update seven segment display"
+	depends on SEVEN_SEGMENT_DISPLAY
+	help
+	  Platform device driver which provides an API for displaying on two
+	  7-segment displays, and implements the required bit-banging.
+	  The hardware assumed is 74HC164 wired to two 7-segment displays.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c2defbd..d9c0d20 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)	+= seven_seg_disp.o
 obj-$(CONFIG_PANEL)             += panel.o
+obj-$(CONFIG_SEVEN_SEGMENT_GPIO)	+= seven_seg_gpio.o
diff --git a/drivers/misc/seven_seg_gpio.c b/drivers/misc/seven_seg_gpio.c
new file mode 100644
index 0000000..3dcb903
--- /dev/null
+++ b/drivers/misc/seven_seg_gpio.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2016 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/sizes.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/gpio/consumer.h>
+
+#include "seven_seg_disp.h"
+
+#define DELAY_INTVL_US 1
+
+#define CLOCK_GPIO_NAME "clock"
+#define DATA_GPIO_NAME "data"
+#define CLEAR_GPIO_NAME "clear"
+
+struct seven_seg_gpio_info {
+	u16 curr_disp_value;
+	u16 refresh_interval;
+	struct timer_list update_timer;
+	struct gpio_desc *clock_gpio;
+	struct gpio_desc *data_gpio;
+	struct gpio_desc *clear_gpio;
+};
+
+static void update_seven_seg_gpio_data(struct device *dev, u16 data)
+{
+	struct platform_device *pdev;
+	struct seven_seg_gpio_info *gpio_info;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	if (pdev == NULL) {
+		pr_err("invalid NULL platform_device\n");
+		return;
+	}
+
+	gpio_info = platform_get_drvdata(pdev);
+	if (gpio_info == NULL) {
+		pr_err("invalid NULL gpio_info\n");
+		return;
+	}
+
+	gpio_info->curr_disp_value = data;
+}
+
+static void clear_seven_seg_gpio_data(struct device *dev, u16 data)
+{
+	struct platform_device *pdev;
+	struct seven_seg_gpio_info *gpio_info;
+
+	pdev = container_of(dev, struct platform_device, dev);
+	if (pdev == NULL) {
+		pr_err("invalid NULL platform_device\n");
+		return;
+	}
+
+	gpio_info = platform_get_drvdata(pdev);
+	if (gpio_info == NULL) {
+		pr_err("invalid NULL gpio_info\n");
+		return;
+	}
+
+	gpio_info->curr_disp_value = 0;
+}
+
+static void send_seven_seg_gpio_data(u16 disp_data,
+		struct seven_seg_gpio_info *gpio_info)
+{
+	int i;
+
+	gpiod_set_value(gpio_info->clear_gpio, 0);
+	udelay(DELAY_INTVL_US);
+	gpiod_set_value(gpio_info->clear_gpio, 1);
+	udelay(DELAY_INTVL_US);
+
+	for (i = 0; i < 16; i++) {
+		if (disp_data & 0x01)
+			gpiod_set_value(gpio_info->data_gpio, 1);
+		else
+			gpiod_set_value(gpio_info->data_gpio, 0);
+
+		udelay(DELAY_INTVL_US);
+
+		gpiod_set_value(gpio_info->clock_gpio, 0);
+		udelay(DELAY_INTVL_US);
+		gpiod_set_value(gpio_info->clock_gpio, 1);
+		udelay(DELAY_INTVL_US);
+
+		disp_data >>= 1;
+	}
+}
+
+static void disp_refresh_timer_handler(unsigned long data)
+{
+	u16 disp_data;
+	struct seven_seg_gpio_info *gpio_info =
+		(struct seven_seg_gpio_info *)data;
+	disp_data = gpio_info->curr_disp_value;
+
+	send_seven_seg_gpio_data(disp_data, gpio_info);
+	mod_timer(&gpio_info->update_timer,
+		jiffies + msecs_to_jiffies(gpio_info->refresh_interval));
+}
+
+static const struct of_device_id of_seven_seg_gpio_match[] = {
+		{ .compatible = "seven-seg-gpio-dev" },
+		{},
+};
+
+MODULE_DEVICE_TABLE(of, of_seven_seg_gpio_match);
+
+static int seven_seg_gpio_probe(struct platform_device *pdev)
+{
+	u16 interval;
+	int result;
+	struct seven_seg_gpio_info *gpio_info;
+	struct device *dev = &pdev->dev;
+	struct seven_seg_disp_dev *disp_dev;
+
+	gpio_info = devm_kzalloc(dev,
+			sizeof(struct seven_seg_gpio_info),
+			GFP_KERNEL);
+	if (gpio_info == NULL)
+		return -ENOMEM;
+
+	/* Requesting the clock gpio */
+	gpio_info->clock_gpio = devm_gpiod_get(dev, CLOCK_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->clock_gpio))
+		return PTR_ERR(gpio_info->clock_gpio);
+
+	/* Requesting the data gpio */
+	gpio_info->data_gpio = devm_gpiod_get(dev, DATA_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->data_gpio))
+		return PTR_ERR(gpio_info->data_gpio);
+
+	/* Requesting the clear gpio */
+	gpio_info->clear_gpio = devm_gpiod_get(dev, CLEAR_GPIO_NAME,
+		GPIOD_OUT_HIGH);
+	if (IS_ERR(gpio_info->clear_gpio))
+		return PTR_ERR(gpio_info->clear_gpio);
+
+	result = of_property_read_u16(pdev->dev.of_node,
+		"refresh-interval-ms", &interval);
+	gpio_info->refresh_interval = result ? DEFAULT_REFRESH_INTERVAL_MS :
+		interval;
+
+	/* Start timer to update seven segment display every second */
+	setup_timer(&gpio_info->update_timer, disp_refresh_timer_handler,
+			(unsigned long)gpio_info);
+	result = mod_timer(&gpio_info->update_timer,
+			jiffies +
+			msecs_to_jiffies(gpio_info->refresh_interval));
+	if (result)
+		return result;
+
+	gpio_info->curr_disp_value = 0;
+
+	platform_set_drvdata(pdev, gpio_info);
+
+	disp_dev = devm_kzalloc(dev, sizeof(struct seven_seg_disp_dev),
+				GFP_KERNEL);
+	disp_dev->parent = *dev;
+	seven_seg_setup_cdev(disp_dev, &update_seven_seg_gpio_data);
+	return 0;
+}
+
+static int seven_seg_gpio_remove(struct platform_device *pdev)
+{
+	struct seven_seg_gpio_info *gpio_info = platform_get_drvdata(pdev);
+	struct seven_seg_disp_dev *disp_dev =
+				container_of(&pdev->dev,
+				struct seven_seg_disp_dev, parent);
+	seven_seg_rem_cdev(disp_dev);
+	del_timer_sync(&gpio_info->update_timer);
+	platform_set_drvdata(pdev, NULL);
+	return 0;
+}
+
+static struct platform_driver seven_seg_gpio_driver = {
+	.probe		= seven_seg_gpio_probe,
+	.remove		= seven_seg_gpio_remove,
+	.driver		= {
+		.name	= "seven-seg-gpio",
+		.of_match_table = of_seven_seg_gpio_match,
+	},
+};
+
+module_platform_driver(seven_seg_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display driver using GPIO config");
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  -1 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh
  Cc: Jaghathiswari Rankappagounder Natarajan, devicetree,
	linux-kernel, linux-arm-kernel, joel

Add clock, data and clear signal GPIO lines to control seven segment display on
zaius platform.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 8ef4ece..ccb8147 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -43,6 +43,14 @@
 			gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	seven-seg-disp {
+		compatible = "seven-seg-gpio-dev";
+		refresh-interval-ms = "1000";
+		clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
+		data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
+		clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
+	};
 };

 &fmc {
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-14  7:55 UTC (permalink / raw)
  To: linux-arm-kernel

Add clock, data and clear signal GPIO lines to control seven segment display on
zaius platform.

Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 8ef4ece..ccb8147 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -43,6 +43,14 @@
 			gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
 		};
 	};
+
+	seven-seg-disp {
+		compatible = "seven-seg-gpio-dev";
+		refresh-interval-ms = "1000";
+		clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
+		data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
+		clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
+	};
 };

 &fmc {
--
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
  (?)
@ 2016-12-14  8:55     ` Arnd Bergmann
  -1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14  8:55 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Jaghathiswari Rankappagounder Natarajan, openbmc, robh+dt,
	mark.rutland, linux, gregkh, joel, linux-kernel, devicetree

On Tuesday, December 13, 2016 11:55:04 PM CET Jaghathiswari Rankappagounder Natarajan wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
> 
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>  			gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>  		};
>  	};
> +
> +	seven-seg-disp {
> +		compatible = "seven-seg-gpio-dev";
> +		refresh-interval-ms = "1000";
> +		clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +		data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +		clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +	};
>  };


According to your introductory mail, the interface is assumed to be
a 74HC164. Should we use that ID in the compatible string?

We can always add other strings later if we want to support multiple
wire formats.

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  8:55     ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14  8:55 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: mark.rutland, Jaghathiswari Rankappagounder Natarajan,
	devicetree, gregkh, openbmc, linux, linux-kernel, robh+dt, joel

On Tuesday, December 13, 2016 11:55:04 PM CET Jaghathiswari Rankappagounder Natarajan wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
> 
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>  			gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>  		};
>  	};
> +
> +	seven-seg-disp {
> +		compatible = "seven-seg-gpio-dev";
> +		refresh-interval-ms = "1000";
> +		clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +		data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +		clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +	};
>  };


According to your introductory mail, the interface is assumed to be
a 74HC164. Should we use that ID in the compatible string?

We can always add other strings later if we want to support multiple
wire formats.

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  8:55     ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14  8:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday, December 13, 2016 11:55:04 PM CET Jaghathiswari Rankappagounder Natarajan wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
> 
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>  			gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>  		};
>  	};
> +
> +	seven-seg-disp {
> +		compatible = "seven-seg-gpio-dev";
> +		refresh-interval-ms = "1000";
> +		clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +		data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +		clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +	};
>  };


According to your introductory mail, the interface is assumed to be
a 74HC164. Should we use that ID in the compatible string?

We can always add other strings later if we want to support multiple
wire formats.

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14  8:55     ` Arnd Bergmann
@ 2016-12-14  9:00       ` Arnd Bergmann
  -1 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14  9:00 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Jaghathiswari Rankappagounder Natarajan, openbmc, robh+dt,
	mark.rutland, linux, gregkh, joel, linux-kernel, devicetree,
	linux-gpio, linus.walleij

On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> According to your introductory mail, the interface is assumed to be
> a 74HC164. Should we use that ID in the compatible string?
> 
> We can always add other strings later if we want to support multiple
> wire formats.

Actually, looking up 74hc164, that seems to be a gpio expander,
so maybe a more flexible way to do the same is to put a driver
for the expander into drivers/gpio/ and have the main driver
access the outputs of that using the gpiolib interface.

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  9:00       ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14  9:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> According to your introductory mail, the interface is assumed to be
> a 74HC164. Should we use that ID in the compatible string?
> 
> We can always add other strings later if we want to support multiple
> wire formats.

Actually, looking up 74hc164, that seems to be a gpio expander,
so maybe a more flexible way to do the same is to put a driver
for the expander into drivers/gpio/ and have the main driver
access the outputs of that using the gpiolib interface.

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  9:02     ` Joel Stanley
  0 siblings, 0 replies; 54+ messages in thread
From: Joel Stanley @ 2016-12-14  9:02 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: OpenBMC Maillist, Rob Herring, Mark Rutland, Russell King,
	Arnd Bergmann, gregkh, devicetree, linux-kernel,
	linux-arm-kernel

Hello Jagha,

On Wed, Dec 14, 2016 at 6:25 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>

The Zaius device tree is not upstream. I suggest you submit it through
the Aspeed maintainer's tree (me!) for inclusion in the next merge
window.

For the time being, drop this patch from your series as it will not
apply to the upstream kernel.

As a general rule make sure you're basing the patches you send
upstream on a tag from an upstream tree. Linus' v4.9 tag would be the
best one at this point in time.

Cheers,

Joel

> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>                         gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>                 };
>         };
> +
> +       seven-seg-disp {
> +               compatible = "seven-seg-gpio-dev";
> +               refresh-interval-ms = "1000";
> +               clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +               clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +       };
>  };
>
>  &fmc {
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  9:02     ` Joel Stanley
  0 siblings, 0 replies; 54+ messages in thread
From: Joel Stanley @ 2016-12-14  9:02 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: OpenBMC Maillist, Rob Herring, Mark Rutland, Russell King,
	Arnd Bergmann, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello Jagha,

On Wed, Dec 14, 2016 at 6:25 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

The Zaius device tree is not upstream. I suggest you submit it through
the Aspeed maintainer's tree (me!) for inclusion in the next merge
window.

For the time being, drop this patch from your series as it will not
apply to the upstream kernel.

As a general rule make sure you're basing the patches you send
upstream on a tag from an upstream tree. Linus' v4.9 tag would be the
best one at this point in time.

Cheers,

Joel

> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>                         gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>                 };
>         };
> +
> +       seven-seg-disp {
> +               compatible = "seven-seg-gpio-dev";
> +               refresh-interval-ms = "1000";
> +               clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +               clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +       };
>  };
>
>  &fmc {
> --
> 2.8.0.rc3.226.g39d4020
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  9:02     ` Joel Stanley
  0 siblings, 0 replies; 54+ messages in thread
From: Joel Stanley @ 2016-12-14  9:02 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: OpenBMC Maillist, Rob Herring, Mark Rutland, Russell King,
	Arnd Bergmann, gregkh, devicetree, linux-kernel,
	linux-arm-kernel

Hello Jagha,

On Wed, Dec 14, 2016 at 6:25 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>

The Zaius device tree is not upstream. I suggest you submit it through
the Aspeed maintainer's tree (me!) for inclusion in the next merge
window.

For the time being, drop this patch from your series as it will not
apply to the upstream kernel.

As a general rule make sure you're basing the patches you send
upstream on a tag from an upstream tree. Linus' v4.9 tag would be the
best one at this point in time.

Cheers,

Joel

> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>                         gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>                 };
>         };
> +
> +       seven-seg-disp {
> +               compatible = "seven-seg-gpio-dev";
> +               refresh-interval-ms = "1000";
> +               clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +               clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +       };
>  };
>
>  &fmc {
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14  9:02     ` Joel Stanley
  0 siblings, 0 replies; 54+ messages in thread
From: Joel Stanley @ 2016-12-14  9:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Jagha,

On Wed, Dec 14, 2016 at 6:25 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> Add clock, data and clear signal GPIO lines to control seven segment display on
> zaius platform.
>
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>

The Zaius device tree is not upstream. I suggest you submit it through
the Aspeed maintainer's tree (me!) for inclusion in the next merge
window.

For the time being, drop this patch from your series as it will not
apply to the upstream kernel.

As a general rule make sure you're basing the patches you send
upstream on a tag from an upstream tree. Linus' v4.9 tag would be the
best one at this point in time.

Cheers,

Joel

> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index 8ef4ece..ccb8147 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -43,6 +43,14 @@
>                         gpios = <&gpio ASPEED_GPIO(H, 7) GPIO_ACTIVE_LOW>;
>                 };
>         };
> +
> +       seven-seg-disp {
> +               compatible = "seven-seg-gpio-dev";
> +               refresh-interval-ms = "1000";
> +               clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
> +               data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
> +               clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
> +       };
>  };
>
>  &fmc {
> --
> 2.8.0.rc3.226.g39d4020
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14  9:00       ` Arnd Bergmann
  (?)
@ 2016-12-14 11:06         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 11:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Jaghathiswari Rankappagounder Natarajan,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A

On Wed, Dec 14, 2016 at 10:00:46AM +0100, Arnd Bergmann wrote:
> On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> > According to your introductory mail, the interface is assumed to be
> > a 74HC164. Should we use that ID in the compatible string?
> > 
> > We can always add other strings later if we want to support multiple
> > wire formats.
> 
> Actually, looking up 74hc164, that seems to be a gpio expander,
> so maybe a more flexible way to do the same is to put a driver
> for the expander into drivers/gpio/ and have the main driver
> access the outputs of that using the gpiolib interface.

There already is - drivers/gpio/gpio-74x164.c

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14 11:06         ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 11:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Jaghathiswari Rankappagounder Natarajan,
	openbmc, robh+dt, mark.rutland, gregkh, joel, linux-kernel,
	devicetree, linux-gpio, linus.walleij

On Wed, Dec 14, 2016 at 10:00:46AM +0100, Arnd Bergmann wrote:
> On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> > According to your introductory mail, the interface is assumed to be
> > a 74HC164. Should we use that ID in the compatible string?
> > 
> > We can always add other strings later if we want to support multiple
> > wire formats.
> 
> Actually, looking up 74hc164, that seems to be a gpio expander,
> so maybe a more flexible way to do the same is to put a driver
> for the expander into drivers/gpio/ and have the main driver
> access the outputs of that using the gpiolib interface.

There already is - drivers/gpio/gpio-74x164.c

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14 11:06         ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 11:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 14, 2016 at 10:00:46AM +0100, Arnd Bergmann wrote:
> On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> > According to your introductory mail, the interface is assumed to be
> > a 74HC164. Should we use that ID in the compatible string?
> > 
> > We can always add other strings later if we want to support multiple
> > wire formats.
> 
> Actually, looking up 74hc164, that seems to be a gpio expander,
> so maybe a more flexible way to do the same is to put a driver
> for the expander into drivers/gpio/ and have the main driver
> access the outputs of that using the gpiolib interface.

There already is - drivers/gpio/gpio-74x164.c

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14 11:06         ` Russell King - ARM Linux
@ 2016-12-14 11:40           ` Russell King - ARM Linux
  -1 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 11:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: mark.rutland, Jaghathiswari Rankappagounder Natarajan,
	devicetree, gregkh, openbmc, linux-kernel, linux-gpio, robh+dt,
	joel, linus.walleij, linux-arm-kernel

On Wed, Dec 14, 2016 at 11:06:35AM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 14, 2016 at 10:00:46AM +0100, Arnd Bergmann wrote:
> > On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> > > According to your introductory mail, the interface is assumed to be
> > > a 74HC164. Should we use that ID in the compatible string?
> > > 
> > > We can always add other strings later if we want to support multiple
> > > wire formats.
> > 
> > Actually, looking up 74hc164, that seems to be a gpio expander,
> > so maybe a more flexible way to do the same is to put a driver
> > for the expander into drivers/gpio/ and have the main driver
> > access the outputs of that using the gpiolib interface.
> 
> There already is - drivers/gpio/gpio-74x164.c

Looking at this more, it's a SPI driver, presumably because the first
case where it appeared was on a SPI bus.

However, it's not a SPI device as such, it's a piece of standard,
general purpose logic that's been around for many years, pre-dating
the SPI bus.

Now, as for DT, we have this "DT represents the hardware, not the
implementation" edict, which now brings up an interesting problem.

If we want to use this driver in its existing form, we need to:

- declare in DT a spi-gpio driver to provide a SPI bus on the GPIO
  pins connected to the 74HC164.
- attach the 74HC164 to the SPI bus.

The problem with that is it's not representative of the hardware -
what we're saying is that we want to reuse our existing implementation
and make DT conform to the implementation.  At that point, we might as
well scrap our "DT is implementation independent" edict above.

What if, tomorrow, we end up with 74HC164 connected to via a different
method?

I think a much more sensible approach would be to turn the GPIO side
of the 74x164 driver into a library, which can be re-used by multiple
bus-specific drivers - one for SPI which allows it to be used in its
current form, one for our platform bus which takes the GPIO lines for
the data, clock and clear signals.

I also don't see why they shouldn't use the same compatible - they're
the same _device_ at the end of the day, just wired up differently.
It makes the binding documentation a little fun wrt what are required
and optional properties, but nothing that shouldn't be too difficult.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-14 11:40           ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 11:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 14, 2016 at 11:06:35AM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 14, 2016 at 10:00:46AM +0100, Arnd Bergmann wrote:
> > On Wednesday, December 14, 2016 9:55:47 AM CET Arnd Bergmann wrote:
> > > According to your introductory mail, the interface is assumed to be
> > > a 74HC164. Should we use that ID in the compatible string?
> > > 
> > > We can always add other strings later if we want to support multiple
> > > wire formats.
> > 
> > Actually, looking up 74hc164, that seems to be a gpio expander,
> > so maybe a more flexible way to do the same is to put a driver
> > for the expander into drivers/gpio/ and have the main driver
> > access the outputs of that using the gpiolib interface.
> 
> There already is - drivers/gpio/gpio-74x164.c

Looking at this more, it's a SPI driver, presumably because the first
case where it appeared was on a SPI bus.

However, it's not a SPI device as such, it's a piece of standard,
general purpose logic that's been around for many years, pre-dating
the SPI bus.

Now, as for DT, we have this "DT represents the hardware, not the
implementation" edict, which now brings up an interesting problem.

If we want to use this driver in its existing form, we need to:

- declare in DT a spi-gpio driver to provide a SPI bus on the GPIO
  pins connected to the 74HC164.
- attach the 74HC164 to the SPI bus.

The problem with that is it's not representative of the hardware -
what we're saying is that we want to reuse our existing implementation
and make DT conform to the implementation.  At that point, we might as
well scrap our "DT is implementation independent" edict above.

What if, tomorrow, we end up with 74HC164 connected to via a different
method?

I think a much more sensible approach would be to turn the GPIO side
of the 74x164 driver into a library, which can be re-used by multiple
bus-specific drivers - one for SPI which allows it to be used in its
current form, one for our platform bus which takes the GPIO lines for
the data, clock and clear signals.

I also don't see why they shouldn't use the same compatible - they're
the same _device_ at the end of the day, just wired up differently.
It makes the binding documentation a little fun wrt what are required
and optional properties, but nothing that shouldn't be too difficult.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14 12:32     ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 12:32 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: openbmc, robh+dt, mark.rutland, arnd, gregkh, devicetree,
	linux-kernel, linux-arm-kernel, joel

On Tue, Dec 13, 2016 at 11:55:02PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
> +int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
> +	void (*update_disp_data)(struct device *, u16 data))
> +{
> +	struct device *dev;
> +	int err;
> +
> +	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
> +			seven_seg_devno,
> +			NULL, "seven_seg_disp_val");
> +	if (dev == NULL)
> +		return -1;

Do not use return -1 in kernel code.

> +	disp_dev->dev = dev;
> +	disp_dev->update_seven_seg_data = update_disp_data;
> +	disp_dev->disp_data_valid = false;
> +
> +	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
> +	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
> +	if (err)
> +		device_destroy(seven_seg_disp_class, seven_seg_devno);
> +	return err;
> +}
> +
> +static int __init seven_seg_disp_init(void)
> +{
> +	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
> +		return -1;

Do not use return -1 in kernel code.

> +
> +	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
> +	if (seven_seg_disp_class == NULL)
> +		goto unreg_chrdev;
> +
> +unreg_chrdev:
> +	unregister_chrdev_region(seven_seg_devno, 1);
> +	return -1;

Do not use return -1 in kernel code.

(Look up what an errno value of '1' means.  Negative values returned from
functions are interpreted as negated errno values.)

Always propagate error codes, or select an appropriate errno value to
return.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14 12:32     ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 12:32 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg

On Tue, Dec 13, 2016 at 11:55:02PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
> +int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
> +	void (*update_disp_data)(struct device *, u16 data))
> +{
> +	struct device *dev;
> +	int err;
> +
> +	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
> +			seven_seg_devno,
> +			NULL, "seven_seg_disp_val");
> +	if (dev == NULL)
> +		return -1;

Do not use return -1 in kernel code.

> +	disp_dev->dev = dev;
> +	disp_dev->update_seven_seg_data = update_disp_data;
> +	disp_dev->disp_data_valid = false;
> +
> +	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
> +	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
> +	if (err)
> +		device_destroy(seven_seg_disp_class, seven_seg_devno);
> +	return err;
> +}
> +
> +static int __init seven_seg_disp_init(void)
> +{
> +	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
> +		return -1;

Do not use return -1 in kernel code.

> +
> +	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
> +	if (seven_seg_disp_class == NULL)
> +		goto unreg_chrdev;
> +
> +unreg_chrdev:
> +	unregister_chrdev_region(seven_seg_devno, 1);
> +	return -1;

Do not use return -1 in kernel code.

(Look up what an errno value of '1' means.  Negative values returned from
functions are interpreted as negated errno values.)

Always propagate error codes, or select an appropriate errno value to
return.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display
@ 2016-12-14 12:32     ` Russell King - ARM Linux
  0 siblings, 0 replies; 54+ messages in thread
From: Russell King - ARM Linux @ 2016-12-14 12:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Dec 13, 2016 at 11:55:02PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
> +int seven_seg_setup_cdev(struct seven_seg_disp_dev *disp_dev,
> +	void (*update_disp_data)(struct device *, u16 data))
> +{
> +	struct device *dev;
> +	int err;
> +
> +	dev = device_create(seven_seg_disp_class, &disp_dev->parent,
> +			seven_seg_devno,
> +			NULL, "seven_seg_disp_val");
> +	if (dev == NULL)
> +		return -1;

Do not use return -1 in kernel code.

> +	disp_dev->dev = dev;
> +	disp_dev->update_seven_seg_data = update_disp_data;
> +	disp_dev->disp_data_valid = false;
> +
> +	cdev_init(&disp_dev->cdev, &seven_seg_disp_fops);
> +	err = cdev_add(&disp_dev->cdev, seven_seg_devno, 1);
> +	if (err)
> +		device_destroy(seven_seg_disp_class, seven_seg_devno);
> +	return err;
> +}
> +
> +static int __init seven_seg_disp_init(void)
> +{
> +	if (alloc_chrdev_region(&seven_seg_devno, 0, 1, "disp_state") < 0)
> +		return -1;

Do not use return -1 in kernel code.

> +
> +	seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
> +	if (seven_seg_disp_class == NULL)
> +		goto unreg_chrdev;
> +
> +unreg_chrdev:
> +	unregister_chrdev_region(seven_seg_devno, 1);
> +	return -1;

Do not use return -1 in kernel code.

(Look up what an errno value of '1' means.  Negative values returned from
functions are interpreted as negated errno values.)

Always propagate error codes, or select an appropriate errno value to
return.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 12:45   ` Thomas Petazzoni
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Petazzoni @ 2016-12-14 12:45 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: openbmc, robh+dt, mark.rutland, linux, arnd, gregkh, joel,
	linux-kernel, linux-arm-kernel, devicetree

Hello,

On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
Natarajan wrote:

> Documentation for the binding which provides an interface for adding clock,
> data and clear signal GPIO lines to control seven segment display.
> 
> The platform device driver provides an API for displaying on two 7-segment
> displays, and implements the required bit-banging. The hardware assumed is
> 74HC164 wired to two 7-segment displays.
> 
> The character device driver implements the user-space API for letting a user
> write to two 7-segment displays including any conversion methods necessary
> to map the user input to two 7-segment displays.
> 
> Adding clock, data and clear signal GPIO lines in the devicetree to control
> seven segment display on zaius platform.
> 
> The platform driver matches on the device tree node; the platform driver also
> initializes the character device.
> 
> Tested that the seven segment display works properly by writing to the
> character device file on a EVB AST2500 board which also has 74HC164 wired
> to two 7-segment displays.

FWIW, I proposed a driver for seven segment displays back in 2013:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html

And the feedback from Greg KH was: we don't need a driver for that, do
it from userspace. See:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html

So: good luck :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 12:45   ` Thomas Petazzoni
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Petazzoni @ 2016-12-14 12:45 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	arnd-r2nGTMty4D4, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	joel-U3u1mxZcP9KHXe+LvDLADg, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hello,

On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
Natarajan wrote:

> Documentation for the binding which provides an interface for adding clock,
> data and clear signal GPIO lines to control seven segment display.
> 
> The platform device driver provides an API for displaying on two 7-segment
> displays, and implements the required bit-banging. The hardware assumed is
> 74HC164 wired to two 7-segment displays.
> 
> The character device driver implements the user-space API for letting a user
> write to two 7-segment displays including any conversion methods necessary
> to map the user input to two 7-segment displays.
> 
> Adding clock, data and clear signal GPIO lines in the devicetree to control
> seven segment display on zaius platform.
> 
> The platform driver matches on the device tree node; the platform driver also
> initializes the character device.
> 
> Tested that the seven segment display works properly by writing to the
> character device file on a EVB AST2500 board which also has 74HC164 wired
> to two 7-segment displays.

FWIW, I proposed a driver for seven segment displays back in 2013:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html

And the feedback from Greg KH was: we don't need a driver for that, do
it from userspace. See:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html

So: good luck :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 12:45   ` Thomas Petazzoni
  0 siblings, 0 replies; 54+ messages in thread
From: Thomas Petazzoni @ 2016-12-14 12:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
Natarajan wrote:

> Documentation for the binding which provides an interface for adding clock,
> data and clear signal GPIO lines to control seven segment display.
> 
> The platform device driver provides an API for displaying on two 7-segment
> displays, and implements the required bit-banging. The hardware assumed is
> 74HC164 wired to two 7-segment displays.
> 
> The character device driver implements the user-space API for letting a user
> write to two 7-segment displays including any conversion methods necessary
> to map the user input to two 7-segment displays.
> 
> Adding clock, data and clear signal GPIO lines in the devicetree to control
> seven segment display on zaius platform.
> 
> The platform driver matches on the device tree node; the platform driver also
> initializes the character device.
> 
> Tested that the seven segment display works properly by writing to the
> character device file on a EVB AST2500 board which also has 74HC164 wired
> to two 7-segment displays.

FWIW, I proposed a driver for seven segment displays back in 2013:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html

And the feedback from Greg KH was: we don't need a driver for that, do
it from userspace. See:

   http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html

So: good luck :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 12:45   ` Thomas Petazzoni
  (?)
@ 2016-12-14 12:56     ` Greg KH
  -1 siblings, 0 replies; 54+ messages in thread
From: Greg KH @ 2016-12-14 12:56 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Jaghathiswari Rankappagounder Natarajan, openbmc, robh+dt,
	mark.rutland, linux, arnd, joel, linux-kernel, linux-arm-kernel,
	devicetree

On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> Natarajan wrote:
> 
> > Documentation for the binding which provides an interface for adding clock,
> > data and clear signal GPIO lines to control seven segment display.
> > 
> > The platform device driver provides an API for displaying on two 7-segment
> > displays, and implements the required bit-banging. The hardware assumed is
> > 74HC164 wired to two 7-segment displays.
> > 
> > The character device driver implements the user-space API for letting a user
> > write to two 7-segment displays including any conversion methods necessary
> > to map the user input to two 7-segment displays.
> > 
> > Adding clock, data and clear signal GPIO lines in the devicetree to control
> > seven segment display on zaius platform.
> > 
> > The platform driver matches on the device tree node; the platform driver also
> > initializes the character device.
> > 
> > Tested that the seven segment display works properly by writing to the
> > character device file on a EVB AST2500 board which also has 74HC164 wired
> > to two 7-segment displays.
> 
> FWIW, I proposed a driver for seven segment displays back in 2013:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> 
> And the feedback from Greg KH was: we don't need a driver for that, do
> it from userspace. See:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> 
> So: good luck :-)

Did anyone ever write a library for this type of thing?

Again, I don't want to see one-off drivers for random devices like this
that should be able to all be controlled from userspace in a common
manner.  Much like we did for fingerprint readers a long long time
ago...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 12:56     ` Greg KH
  0 siblings, 0 replies; 54+ messages in thread
From: Greg KH @ 2016-12-14 12:56 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: mark.rutland, Jaghathiswari Rankappagounder Natarajan, arnd,
	devicetree, openbmc, linux, linux-kernel, robh+dt, joel,
	linux-arm-kernel

On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> Natarajan wrote:
> 
> > Documentation for the binding which provides an interface for adding clock,
> > data and clear signal GPIO lines to control seven segment display.
> > 
> > The platform device driver provides an API for displaying on two 7-segment
> > displays, and implements the required bit-banging. The hardware assumed is
> > 74HC164 wired to two 7-segment displays.
> > 
> > The character device driver implements the user-space API for letting a user
> > write to two 7-segment displays including any conversion methods necessary
> > to map the user input to two 7-segment displays.
> > 
> > Adding clock, data and clear signal GPIO lines in the devicetree to control
> > seven segment display on zaius platform.
> > 
> > The platform driver matches on the device tree node; the platform driver also
> > initializes the character device.
> > 
> > Tested that the seven segment display works properly by writing to the
> > character device file on a EVB AST2500 board which also has 74HC164 wired
> > to two 7-segment displays.
> 
> FWIW, I proposed a driver for seven segment displays back in 2013:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> 
> And the feedback from Greg KH was: we don't need a driver for that, do
> it from userspace. See:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> 
> So: good luck :-)

Did anyone ever write a library for this type of thing?

Again, I don't want to see one-off drivers for random devices like this
that should be able to all be controlled from userspace in a common
manner.  Much like we did for fingerprint readers a long long time
ago...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 12:56     ` Greg KH
  0 siblings, 0 replies; 54+ messages in thread
From: Greg KH @ 2016-12-14 12:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> Natarajan wrote:
> 
> > Documentation for the binding which provides an interface for adding clock,
> > data and clear signal GPIO lines to control seven segment display.
> > 
> > The platform device driver provides an API for displaying on two 7-segment
> > displays, and implements the required bit-banging. The hardware assumed is
> > 74HC164 wired to two 7-segment displays.
> > 
> > The character device driver implements the user-space API for letting a user
> > write to two 7-segment displays including any conversion methods necessary
> > to map the user input to two 7-segment displays.
> > 
> > Adding clock, data and clear signal GPIO lines in the devicetree to control
> > seven segment display on zaius platform.
> > 
> > The platform driver matches on the device tree node; the platform driver also
> > initializes the character device.
> > 
> > Tested that the seven segment display works properly by writing to the
> > character device file on a EVB AST2500 board which also has 74HC164 wired
> > to two 7-segment displays.
> 
> FWIW, I proposed a driver for seven segment displays back in 2013:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> 
> And the feedback from Greg KH was: we don't need a driver for that, do
> it from userspace. See:
> 
>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> 
> So: good luck :-)

Did anyone ever write a library for this type of thing?

Again, I don't want to see one-off drivers for random devices like this
that should be able to all be controlled from userspace in a common
manner.  Much like we did for fingerprint readers a long long time
ago...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 12:56     ` Greg KH
@ 2016-12-14 13:12       ` Neil Armstrong
  -1 siblings, 0 replies; 54+ messages in thread
From: Neil Armstrong @ 2016-12-14 13:12 UTC (permalink / raw)
  To: Greg KH, Thomas Petazzoni
  Cc: mark.rutland, Jaghathiswari Rankappagounder Natarajan, arnd,
	devicetree, openbmc, linux, linux-kernel, robh+dt, joel,
	linux-arm-kernel

On 12/14/2016 01:56 PM, Greg KH wrote:
> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>> Natarajan wrote:
>>
>>> Documentation for the binding which provides an interface for adding clock,
>>> data and clear signal GPIO lines to control seven segment display.
>>>
>>> The platform device driver provides an API for displaying on two 7-segment
>>> displays, and implements the required bit-banging. The hardware assumed is
>>> 74HC164 wired to two 7-segment displays.
>>>
>>> The character device driver implements the user-space API for letting a user
>>> write to two 7-segment displays including any conversion methods necessary
>>> to map the user input to two 7-segment displays.
>>>
>>> Adding clock, data and clear signal GPIO lines in the devicetree to control
>>> seven segment display on zaius platform.
>>>
>>> The platform driver matches on the device tree node; the platform driver also
>>> initializes the character device.
>>>
>>> Tested that the seven segment display works properly by writing to the
>>> character device file on a EVB AST2500 board which also has 74HC164 wired
>>> to two 7-segment displays.
>>
>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>
>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
>>
>> And the feedback from Greg KH was: we don't need a driver for that, do
>> it from userspace. See:
>>
>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
>>
>> So: good luck :-)
> 
> Did anyone ever write a library for this type of thing?
> 
> Again, I don't want to see one-off drivers for random devices like this
> that should be able to all be controlled from userspace in a common
> manner.  Much like we did for fingerprint readers a long long time
> ago...
> 
> thanks,
> 
> greg k-h

Hi Greg,

Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
characters a simple and clean driver could achieve.
Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
a 74HC164 like component and avoid gpio bit banging.

My personal concern is that you could also need to drive more segments, thus 7-segments
is too restrictive.

But this driver is well structured, the gpio-bitbanging sub-driver is welcome.

Neil

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 13:12       ` Neil Armstrong
  0 siblings, 0 replies; 54+ messages in thread
From: Neil Armstrong @ 2016-12-14 13:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/14/2016 01:56 PM, Greg KH wrote:
> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>> Natarajan wrote:
>>
>>> Documentation for the binding which provides an interface for adding clock,
>>> data and clear signal GPIO lines to control seven segment display.
>>>
>>> The platform device driver provides an API for displaying on two 7-segment
>>> displays, and implements the required bit-banging. The hardware assumed is
>>> 74HC164 wired to two 7-segment displays.
>>>
>>> The character device driver implements the user-space API for letting a user
>>> write to two 7-segment displays including any conversion methods necessary
>>> to map the user input to two 7-segment displays.
>>>
>>> Adding clock, data and clear signal GPIO lines in the devicetree to control
>>> seven segment display on zaius platform.
>>>
>>> The platform driver matches on the device tree node; the platform driver also
>>> initializes the character device.
>>>
>>> Tested that the seven segment display works properly by writing to the
>>> character device file on a EVB AST2500 board which also has 74HC164 wired
>>> to two 7-segment displays.
>>
>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>
>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
>>
>> And the feedback from Greg KH was: we don't need a driver for that, do
>> it from userspace. See:
>>
>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
>>
>> So: good luck :-)
> 
> Did anyone ever write a library for this type of thing?
> 
> Again, I don't want to see one-off drivers for random devices like this
> that should be able to all be controlled from userspace in a common
> manner.  Much like we did for fingerprint readers a long long time
> ago...
> 
> thanks,
> 
> greg k-h

Hi Greg,

Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
characters a simple and clean driver could achieve.
Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
a 74HC164 like component and avoid gpio bit banging.

My personal concern is that you could also need to drive more segments, thus 7-segments
is too restrictive.

But this driver is well structured, the gpio-bitbanging sub-driver is welcome.

Neil

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 14:15         ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14 14:15 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Greg KH, Thomas Petazzoni, mark.rutland,
	Jaghathiswari Rankappagounder Natarajan, devicetree, openbmc,
	linux, linux-kernel, robh+dt, joel, linux-arm-kernel

On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
> On 12/14/2016 01:56 PM, Greg KH wrote:
> > On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> >> Hello,
> >>
> >> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> >> Natarajan wrote:
> >>
> >>> Documentation for the binding which provides an interface for adding clock,
> >>> data and clear signal GPIO lines to control seven segment display.
> >>>
> >>> The platform device driver provides an API for displaying on two 7-segment
> >>> displays, and implements the required bit-banging. The hardware assumed is
> >>> 74HC164 wired to two 7-segment displays.
> >>>
> >>> The character device driver implements the user-space API for letting a user
> >>> write to two 7-segment displays including any conversion methods necessary
> >>> to map the user input to two 7-segment displays.
> >>>
> >>> Adding clock, data and clear signal GPIO lines in the devicetree to control
> >>> seven segment display on zaius platform.
> >>>
> >>> The platform driver matches on the device tree node; the platform driver also
> >>> initializes the character device.
> >>>
> >>> Tested that the seven segment display works properly by writing to the
> >>> character device file on a EVB AST2500 board which also has 74HC164 wired
> >>> to two 7-segment displays.
> >>
> >> FWIW, I proposed a driver for seven segment displays back in 2013:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> >>
> >> And the feedback from Greg KH was: we don't need a driver for that, do
> >> it from userspace. See:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> >>
> >> So: good luck 
> > 
> > Did anyone ever write a library for this type of thing?
> > 
> > Again, I don't want to see one-off drivers for random devices like this
> > that should be able to all be controlled from userspace in a common
> > manner.  Much like we did for fingerprint readers a long long time
> > ago...
> > 

> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
> characters a simple and clean driver could achieve.
> Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
> a 74HC164 like component and avoid gpio bit banging.
> 
> My personal concern is that you could also need to drive more segments, thus 7-segments
> is too restrictive.
> 
> But this driver is well structured, the gpio-bitbanging sub-driver is welcome.

Maybe we can find a way to fit this into the existing drivers/leds/ subsystem?

That already supports blinking, brightness and colour attributes of LEDs,
so could this be extended to support (one of) digit, number, character
or string with a common sysfs attribute and a way to hook up a led driver
to that?

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 14:15         ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14 14:15 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Greg KH, Thomas Petazzoni, mark.rutland-5wv7dgnIgG8,
	Jaghathiswari Rankappagounder Natarajan,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, joel-U3u1mxZcP9KHXe+LvDLADg,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
> On 12/14/2016 01:56 PM, Greg KH wrote:
> > On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> >> Hello,
> >>
> >> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> >> Natarajan wrote:
> >>
> >>> Documentation for the binding which provides an interface for adding clock,
> >>> data and clear signal GPIO lines to control seven segment display.
> >>>
> >>> The platform device driver provides an API for displaying on two 7-segment
> >>> displays, and implements the required bit-banging. The hardware assumed is
> >>> 74HC164 wired to two 7-segment displays.
> >>>
> >>> The character device driver implements the user-space API for letting a user
> >>> write to two 7-segment displays including any conversion methods necessary
> >>> to map the user input to two 7-segment displays.
> >>>
> >>> Adding clock, data and clear signal GPIO lines in the devicetree to control
> >>> seven segment display on zaius platform.
> >>>
> >>> The platform driver matches on the device tree node; the platform driver also
> >>> initializes the character device.
> >>>
> >>> Tested that the seven segment display works properly by writing to the
> >>> character device file on a EVB AST2500 board which also has 74HC164 wired
> >>> to two 7-segment displays.
> >>
> >> FWIW, I proposed a driver for seven segment displays back in 2013:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> >>
> >> And the feedback from Greg KH was: we don't need a driver for that, do
> >> it from userspace. See:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> >>
> >> So: good luck 
> > 
> > Did anyone ever write a library for this type of thing?
> > 
> > Again, I don't want to see one-off drivers for random devices like this
> > that should be able to all be controlled from userspace in a common
> > manner.  Much like we did for fingerprint readers a long long time
> > ago...
> > 

> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
> characters a simple and clean driver could achieve.
> Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
> a 74HC164 like component and avoid gpio bit banging.
> 
> My personal concern is that you could also need to drive more segments, thus 7-segments
> is too restrictive.
> 
> But this driver is well structured, the gpio-bitbanging sub-driver is welcome.

Maybe we can find a way to fit this into the existing drivers/leds/ subsystem?

That already supports blinking, brightness and colour attributes of LEDs,
so could this be extended to support (one of) digit, number, character
or string with a common sysfs attribute and a way to hook up a led driver
to that?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 14:15         ` Arnd Bergmann
  0 siblings, 0 replies; 54+ messages in thread
From: Arnd Bergmann @ 2016-12-14 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
> On 12/14/2016 01:56 PM, Greg KH wrote:
> > On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> >> Hello,
> >>
> >> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> >> Natarajan wrote:
> >>
> >>> Documentation for the binding which provides an interface for adding clock,
> >>> data and clear signal GPIO lines to control seven segment display.
> >>>
> >>> The platform device driver provides an API for displaying on two 7-segment
> >>> displays, and implements the required bit-banging. The hardware assumed is
> >>> 74HC164 wired to two 7-segment displays.
> >>>
> >>> The character device driver implements the user-space API for letting a user
> >>> write to two 7-segment displays including any conversion methods necessary
> >>> to map the user input to two 7-segment displays.
> >>>
> >>> Adding clock, data and clear signal GPIO lines in the devicetree to control
> >>> seven segment display on zaius platform.
> >>>
> >>> The platform driver matches on the device tree node; the platform driver also
> >>> initializes the character device.
> >>>
> >>> Tested that the seven segment display works properly by writing to the
> >>> character device file on a EVB AST2500 board which also has 74HC164 wired
> >>> to two 7-segment displays.
> >>
> >> FWIW, I proposed a driver for seven segment displays back in 2013:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> >>
> >> And the feedback from Greg KH was: we don't need a driver for that, do
> >> it from userspace. See:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> >>
> >> So: good luck 
> > 
> > Did anyone ever write a library for this type of thing?
> > 
> > Again, I don't want to see one-off drivers for random devices like this
> > that should be able to all be controlled from userspace in a common
> > manner.  Much like we did for fingerprint readers a long long time
> > ago...
> > 

> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
> characters a simple and clean driver could achieve.
> Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
> a 74HC164 like component and avoid gpio bit banging.
> 
> My personal concern is that you could also need to drive more segments, thus 7-segments
> is too restrictive.
> 
> But this driver is well structured, the gpio-bitbanging sub-driver is welcome.

Maybe we can find a way to fit this into the existing drivers/leds/ subsystem?

That already supports blinking, brightness and colour attributes of LEDs,
so could this be extended to support (one of) digit, number, character
or string with a common sysfs attribute and a way to hook up a led driver
to that?

	Arnd

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
                   ` (5 preceding siblings ...)
  (?)
@ 2016-12-14 14:35 ` Patrick Williams
  2016-12-14 23:23   ` Rob Lippert
  -1 siblings, 1 reply; 54+ messages in thread
From: Patrick Williams @ 2016-12-14 14:35 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan; +Cc: openbmc

[-- Attachment #1: Type: text/plain, Size: 3468 bytes --]

On Tue, Dec 13, 2016 at 11:55:00PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:

Jaghu,

Bringing the discussion back onto just the OpenBMC list.

Thank you for getting this out to the broader Linux community to solicit
feedback.  As you see, even if things look good to some of the kernel
devs here from a "is this generally how the kernel does things" and "is
this code generally of quality to be acceptable upstream" there is a lot
of history and design decisions on something as large as the Linux
kernel for any of us to all keep track of.

When we are adding support for a new device of a type already supported,
like an i2c-eeprom or a thermal sensor, it is fairly easy to assume
we're going to go through the upstream community.  When we are adding
something new, like this 7-seg display, there is more likely to be
resistance.  Maybe Joel can weigh in on, going forward, how we get early
feedback so we don't spend a lot of time working on code that will never
be accepted.

I read through the current responses and the referenced email chain from
2013 with a similar driver and there seems to be two major points in the
feedback.

1) The chip you are driving the 7-seg display with is already supported
as a SPI-attached GPIO controller.

    It does look like that driver plus SPI_BITBANG might allow us to
    control the GPIOs on that device as a normal GPIO controller.  Is
    that something you would like to try out and see if you can get
    working?  Hopefully that would reduce your code to just the
    translation from character to GPIO settings.

2) The community isn't interested in a one-off driver like this.

    The thread from 2013 raised points about how a 7-segment display
    driver isn't generic enough to support "X-segment displays".  One
    of the authors mentioned how some displays have a decimal place that
    can also be lit.  This lead Greg K-H to the decision that it would
    best be handled from userspace.

    I believe Google has a desire to get post codes from the BMC U-boot and
    kernel, hence why you tried to put it into the kernel?  Are there
    additional reasons for wanting to put it in the kernel?

    I see three possible solutions, and there may be more:

    a) We use SPI_BITBANG and the GPIO driver for this device (which we
    probably should do in either case) and use device tree "hogging"
    of the GPIOs to set the initial kernel post code.  This will cause
    the kernel to light that display correctly as it is initializing.
    We can then write a userspace application to manipulate the GPIOs
    after that to change the display as appropriate.

    b) We go back to the LKML with some more background on why we would
    like this in-kernel, as in something unique to the BMC requirements
    that needs the support in-kernel, and see if that makes them more
    receptive.  I'm willing to help you work on the message here if you
    would like.

    c) We see how willing Joel is to keep this as an out-of-tree driver.
    It seems to use pretty generic and not-likely-to-change kernel APIs
    so I don't see this as a large maintenance burden.

    I suspect we would tend to prefer (a) but I don't know if that will
    still allow it to meet Google's needs.

Feel free to catch me on IRC today if you'd like to have an active
discussion on this to get things moving faster.

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 13:12       ` Neil Armstrong
@ 2016-12-14 16:50         ` Greg KH
  -1 siblings, 0 replies; 54+ messages in thread
From: Greg KH @ 2016-12-14 16:50 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Thomas Petazzoni, mark.rutland,
	Jaghathiswari Rankappagounder Natarajan, arnd, devicetree,
	openbmc, linux, linux-kernel, robh+dt, joel, linux-arm-kernel

On Wed, Dec 14, 2016 at 02:12:41PM +0100, Neil Armstrong wrote:
> On 12/14/2016 01:56 PM, Greg KH wrote:
> > On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> >> Hello,
> >>
> >> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> >> Natarajan wrote:
> >>
> >>> Documentation for the binding which provides an interface for adding clock,
> >>> data and clear signal GPIO lines to control seven segment display.
> >>>
> >>> The platform device driver provides an API for displaying on two 7-segment
> >>> displays, and implements the required bit-banging. The hardware assumed is
> >>> 74HC164 wired to two 7-segment displays.
> >>>
> >>> The character device driver implements the user-space API for letting a user
> >>> write to two 7-segment displays including any conversion methods necessary
> >>> to map the user input to two 7-segment displays.
> >>>
> >>> Adding clock, data and clear signal GPIO lines in the devicetree to control
> >>> seven segment display on zaius platform.
> >>>
> >>> The platform driver matches on the device tree node; the platform driver also
> >>> initializes the character device.
> >>>
> >>> Tested that the seven segment display works properly by writing to the
> >>> character device file on a EVB AST2500 board which also has 74HC164 wired
> >>> to two 7-segment displays.
> >>
> >> FWIW, I proposed a driver for seven segment displays back in 2013:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> >>
> >> And the feedback from Greg KH was: we don't need a driver for that, do
> >> it from userspace. See:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> >>
> >> So: good luck :-)
> > 
> > Did anyone ever write a library for this type of thing?
> > 
> > Again, I don't want to see one-off drivers for random devices like this
> > that should be able to all be controlled from userspace in a common
> > manner.  Much like we did for fingerprint readers a long long time
> > ago...
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
> characters a simple and clean driver could achieve.

Great, then let's make an API that all devices of this type could use,
and not just take individual drivers that all have a custom char or
sysfs interface which requires custom userspace code to be able to drive
all of the different devices in a common way (i.e. a library would have
to be written anyways...)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 16:50         ` Greg KH
  0 siblings, 0 replies; 54+ messages in thread
From: Greg KH @ 2016-12-14 16:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 14, 2016 at 02:12:41PM +0100, Neil Armstrong wrote:
> On 12/14/2016 01:56 PM, Greg KH wrote:
> > On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
> >> Hello,
> >>
> >> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
> >> Natarajan wrote:
> >>
> >>> Documentation for the binding which provides an interface for adding clock,
> >>> data and clear signal GPIO lines to control seven segment display.
> >>>
> >>> The platform device driver provides an API for displaying on two 7-segment
> >>> displays, and implements the required bit-banging. The hardware assumed is
> >>> 74HC164 wired to two 7-segment displays.
> >>>
> >>> The character device driver implements the user-space API for letting a user
> >>> write to two 7-segment displays including any conversion methods necessary
> >>> to map the user input to two 7-segment displays.
> >>>
> >>> Adding clock, data and clear signal GPIO lines in the devicetree to control
> >>> seven segment display on zaius platform.
> >>>
> >>> The platform driver matches on the device tree node; the platform driver also
> >>> initializes the character device.
> >>>
> >>> Tested that the seven segment display works properly by writing to the
> >>> character device file on a EVB AST2500 board which also has 74HC164 wired
> >>> to two 7-segment displays.
> >>
> >> FWIW, I proposed a driver for seven segment displays back in 2013:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
> >>
> >> And the feedback from Greg KH was: we don't need a driver for that, do
> >> it from userspace. See:
> >>
> >>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
> >>
> >> So: good luck :-)
> > 
> > Did anyone ever write a library for this type of thing?
> > 
> > Again, I don't want to see one-off drivers for random devices like this
> > that should be able to all be controlled from userspace in a common
> > manner.  Much like we did for fingerprint readers a long long time
> > ago...
> > 
> > thanks,
> > 
> > greg k-h
> 
> Hi Greg,
> 
> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
> characters a simple and clean driver could achieve.

Great, then let's make an API that all devices of this type could use,
and not just take individual drivers that all have a custom char or
sysfs interface which requires custom userspace code to be able to drive
all of the different devices in a common way (i.e. a library would have
to be written anyways...)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 14:15         ` Arnd Bergmann
@ 2016-12-14 20:05           ` David Daney
  -1 siblings, 0 replies; 54+ messages in thread
From: David Daney @ 2016-12-14 20:05 UTC (permalink / raw)
  To: Arnd Bergmann, Neil Armstrong
  Cc: Thomas Petazzoni, mark.rutland,
	Jaghathiswari Rankappagounder Natarajan, devicetree, Greg KH,
	openbmc, linux, linux-kernel, robh+dt, joel, linux-arm-kernel

On 12/14/2016 06:15 AM, Arnd Bergmann wrote:
> On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
>> On 12/14/2016 01:56 PM, Greg KH wrote:
>>> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>>>> Hello,
>>>>
>>>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>>>> Natarajan wrote:
>>>>
>>>>> Documentation for the binding which provides an interface for adding clock,
>>>>> data and clear signal GPIO lines to control seven segment display.
>>>>>
>>>>> The platform device driver provides an API for displaying on two 7-segment
>>>>> displays, and implements the required bit-banging. The hardware assumed is
>>>>> 74HC164 wired to two 7-segment displays.
>>>>>
>>>>> The character device driver implements the user-space API for letting a user
>>>>> write to two 7-segment displays including any conversion methods necessary
>>>>> to map the user input to two 7-segment displays.
>>>>>
>>>>> Adding clock, data and clear signal GPIO lines in the devicetree to control
>>>>> seven segment display on zaius platform.
>>>>>
>>>>> The platform driver matches on the device tree node; the platform driver also
>>>>> initializes the character device.
>>>>>
>>>>> Tested that the seven segment display works properly by writing to the
>>>>> character device file on a EVB AST2500 board which also has 74HC164 wired
>>>>> to two 7-segment displays.
>>>>
>>>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>>>
>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
>>>>
>>>> And the feedback from Greg KH was: we don't need a driver for that, do
>>>> it from userspace. See:
>>>>
>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
>>>>
>>>> So: good luck
>>>
>>> Did anyone ever write a library for this type of thing?
>>>
>>> Again, I don't want to see one-off drivers for random devices like this
>>> that should be able to all be controlled from userspace in a common
>>> manner.  Much like we did for fingerprint readers a long long time
>>> ago...
>>>
>
>> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
>> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
>> characters a simple and clean driver could achieve.
>> Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
>> a 74HC164 like component and avoid gpio bit banging.
>>
>> My personal concern is that you could also need to drive more segments, thus 7-segments
>> is too restrictive.
>>
>> But this driver is well structured, the gpio-bitbanging sub-driver is welcome.
>
> Maybe we can find a way to fit this into the existing drivers/leds/ subsystem?
>
> That already supports blinking, brightness and colour attributes of LEDs,
> so could this be extended to support (one of) digit, number, character
> or string with a common sysfs attribute and a way to hook up a led driver
> to that?

We have a lot of boards with an 8-cell dot matrix LED. Each cell is 
programmed with an 8-bit value.  The mapping of these values to the dots 
defaults to ASCII character rendering, but there is the facility to 
install other bitmaps as well.

Really I view these things not as part of the LED subsystem, but more as 
a very small frame buffer.

We like to display entire words, and the most useful interface from a 
user point of view is something that consumes entire strings rather than 
having to manage each cell independently.

You could imagine that if the text to be displayed were longer than the 
display, that the driver would make it continuously scroll.  I would 
like to see a framework where a simple character device were exposed, 
and from userspace you could do: "echo message > /dev/small-display" and 
get something sensible.

>
> 	Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-14 20:05           ` David Daney
  0 siblings, 0 replies; 54+ messages in thread
From: David Daney @ 2016-12-14 20:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/14/2016 06:15 AM, Arnd Bergmann wrote:
> On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
>> On 12/14/2016 01:56 PM, Greg KH wrote:
>>> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>>>> Hello,
>>>>
>>>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>>>> Natarajan wrote:
>>>>
>>>>> Documentation for the binding which provides an interface for adding clock,
>>>>> data and clear signal GPIO lines to control seven segment display.
>>>>>
>>>>> The platform device driver provides an API for displaying on two 7-segment
>>>>> displays, and implements the required bit-banging. The hardware assumed is
>>>>> 74HC164 wired to two 7-segment displays.
>>>>>
>>>>> The character device driver implements the user-space API for letting a user
>>>>> write to two 7-segment displays including any conversion methods necessary
>>>>> to map the user input to two 7-segment displays.
>>>>>
>>>>> Adding clock, data and clear signal GPIO lines in the devicetree to control
>>>>> seven segment display on zaius platform.
>>>>>
>>>>> The platform driver matches on the device tree node; the platform driver also
>>>>> initializes the character device.
>>>>>
>>>>> Tested that the seven segment display works properly by writing to the
>>>>> character device file on a EVB AST2500 board which also has 74HC164 wired
>>>>> to two 7-segment displays.
>>>>
>>>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>>>
>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139986.html
>>>>
>>>> And the feedback from Greg KH was: we don't need a driver for that, do
>>>> it from userspace. See:
>>>>
>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-January/139992.html
>>>>
>>>> So: good luck
>>>
>>> Did anyone ever write a library for this type of thing?
>>>
>>> Again, I don't want to see one-off drivers for random devices like this
>>> that should be able to all be controlled from userspace in a common
>>> manner.  Much like we did for fingerprint readers a long long time
>>> ago...
>>>
>
>> Actually, it's more than a random interface, a lot of SoCs and boards actually have such displays
>> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly stuff to only print a few
>> characters a simple and clean driver could achieve.
>> Some very well known SoCs even have integrated registers to lower the BOM and bypass the need for
>> a 74HC164 like component and avoid gpio bit banging.
>>
>> My personal concern is that you could also need to drive more segments, thus 7-segments
>> is too restrictive.
>>
>> But this driver is well structured, the gpio-bitbanging sub-driver is welcome.
>
> Maybe we can find a way to fit this into the existing drivers/leds/ subsystem?
>
> That already supports blinking, brightness and colour attributes of LEDs,
> so could this be extended to support (one of) digit, number, character
> or string with a common sysfs attribute and a way to hook up a led driver
> to that?

We have a lot of boards with an 8-cell dot matrix LED. Each cell is 
programmed with an 8-bit value.  The mapping of these values to the dots 
defaults to ASCII character rendering, but there is the facility to 
install other bitmaps as well.

Really I view these things not as part of the LED subsystem, but more as 
a very small frame buffer.

We like to display entire words, and the most useful interface from a 
user point of view is something that consumes entire strings rather than 
having to manage each cell independently.

You could imagine that if the text to be displayed were longer than the 
display, that the driver would make it continuously scroll.  I would 
like to see a framework where a simple character device were exposed, 
and from userspace you could do: "echo message > /dev/small-display" and 
get something sensible.

>
> 	Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 14:35 ` Patrick Williams
@ 2016-12-14 23:23   ` Rob Lippert
  0 siblings, 0 replies; 54+ messages in thread
From: Rob Lippert @ 2016-12-14 23:23 UTC (permalink / raw)
  To: Patrick Williams; +Cc: Jaghathiswari Rankappagounder Natarajan, openbmc

On Wed, Dec 14, 2016 at 6:35 AM, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Dec 13, 2016 at 11:55:00PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
>
> Jaghu,
>
> Bringing the discussion back onto just the OpenBMC list.
>
> Thank you for getting this out to the broader Linux community to solicit
> feedback.  As you see, even if things look good to some of the kernel
> devs here from a "is this generally how the kernel does things" and "is
> this code generally of quality to be acceptable upstream" there is a lot
> of history and design decisions on something as large as the Linux
> kernel for any of us to all keep track of.
>
> When we are adding support for a new device of a type already supported,
> like an i2c-eeprom or a thermal sensor, it is fairly easy to assume
> we're going to go through the upstream community.  When we are adding
> something new, like this 7-seg display, there is more likely to be
> resistance.  Maybe Joel can weigh in on, going forward, how we get early
> feedback so we don't spend a lot of time working on code that will never
> be accepted.
>
> I read through the current responses and the referenced email chain from
> 2013 with a similar driver and there seems to be two major points in the
> feedback.
>
> 1) The chip you are driving the 7-seg display with is already supported
> as a SPI-attached GPIO controller.
>
>     It does look like that driver plus SPI_BITBANG might allow us to
>     control the GPIOs on that device as a normal GPIO controller.  Is
>     that something you would like to try out and see if you can get
>     working?  Hopefully that would reduce your code to just the
>     translation from character to GPIO settings.

This was my original hope but Jagha tried it and it doesn't work :(

The serial GPIO interface is slightly different wrt the buffer chips
that we use vs the ones that are needed to work with the serial GPIO
driver (namely the lack of a "load" signal) and there is no way to
make it work as far as I can tell.

>
>
> 2) The community isn't interested in a one-off driver like this.
>
>     The thread from 2013 raised points about how a 7-segment display
>     driver isn't generic enough to support "X-segment displays".  One
>     of the authors mentioned how some displays have a decimal place that
>     can also be lit.  This lead Greg K-H to the decision that it would
>     best be handled from userspace.
>
>     I believe Google has a desire to get post codes from the BMC U-boot and
>     kernel, hence why you tried to put it into the kernel?  Are there
>     additional reasons for wanting to put it in the kernel?
>
>     I see three possible solutions, and there may be more:
>
>     a) We use SPI_BITBANG and the GPIO driver for this device (which we
>     probably should do in either case) and use device tree "hogging"
>     of the GPIOs to set the initial kernel post code.  This will cause
>     the kernel to light that display correctly as it is initializing.
>     We can then write a userspace application to manipulate the GPIOs
>     after that to change the display as appropriate.
>
>     b) We go back to the LKML with some more background on why we would
>     like this in-kernel, as in something unique to the BMC requirements
>     that needs the support in-kernel, and see if that makes them more
>     receptive.  I'm willing to help you work on the message here if you
>     would like.

The other part of this driver that hasn't been written yet is to take
bytes written to LPC port 80h (from the hostboot/etc. firmware) and
echo them to the 7seg display.

That would be easier to do purely in the kernel, although it should
also be possible to do it via a userspace app I guess with a lot more
work...

>
>
>     c) We see how willing Joel is to keep this as an out-of-tree driver.
>     It seems to use pretty generic and not-likely-to-change kernel APIs
>     so I don't see this as a large maintenance burden.
>
>     I suspect we would tend to prefer (a) but I don't know if that will
>     still allow it to meet Google's needs.
>
> Feel free to catch me on IRC today if you'd like to have an active
> discussion on this to get things moving faster.
>
> --
> Patrick Williams
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
  2016-12-14 11:40           ` Russell King - ARM Linux
  (?)
  (?)
@ 2016-12-15 23:07             ` Linus Walleij
  -1 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2016-12-15 23:07 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Mark Rutland,
	Jaghathiswari Rankappagounder Natarajan, devicetree, Greg KH,
	OpenBMC Maillist, linux-kernel, linux-gpio, Rob Herring,
	Joel Stanley, linux-arm-kernel

On Wed, Dec 14, 2016 at 12:40 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Looking at this more, it's a SPI driver, presumably because the first
> case where it appeared was on a SPI bus.
>
> However, it's not a SPI device as such, it's a piece of standard,
> general purpose logic that's been around for many years, pre-dating
> the SPI bus.

Indeed.

> I think a much more sensible approach would be to turn the GPIO side
> of the 74x164 driver into a library, which can be re-used by multiple
> bus-specific drivers - one for SPI which allows it to be used in its
> current form, one for our platform bus which takes the GPIO lines for
> the data, clock and clear signals.
>
> I also don't see why they shouldn't use the same compatible - they're
> the same _device_ at the end of the day, just wired up differently.
> It makes the binding documentation a little fun wrt what are required
> and optional properties, but nothing that shouldn't be too difficult.

I agree on both accounts.

Sorry for not seeing this in the first place, I was well aware that this
is a standard component and may be connected in a myriad of ways,
so I should have known better :(

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-15 23:07             ` Linus Walleij
  0 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2016-12-15 23:07 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Mark Rutland,
	Jaghathiswari Rankappagounder Natarajan, devicetree, Greg KH,
	OpenBMC Maillist, linux-kernel, linux-gpio, Rob Herring,
	Joel Stanley, linux-arm-kernel

On Wed, Dec 14, 2016 at 12:40 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Looking at this more, it's a SPI driver, presumably because the first
> case where it appeared was on a SPI bus.
>
> However, it's not a SPI device as such, it's a piece of standard,
> general purpose logic that's been around for many years, pre-dating
> the SPI bus.

Indeed.

> I think a much more sensible approach would be to turn the GPIO side
> of the 74x164 driver into a library, which can be re-used by multiple
> bus-specific drivers - one for SPI which allows it to be used in its
> current form, one for our platform bus which takes the GPIO lines for
> the data, clock and clear signals.
>
> I also don't see why they shouldn't use the same compatible - they're
> the same _device_ at the end of the day, just wired up differently.
> It makes the binding documentation a little fun wrt what are required
> and optional properties, but nothing that shouldn't be too difficult.

I agree on both accounts.

Sorry for not seeing this in the first place, I was well aware that this
is a standard component and may be connected in a myriad of ways,
so I should have known better :(

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-15 23:07             ` Linus Walleij
  0 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2016-12-15 23:07 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Mark Rutland,
	Jaghathiswari Rankappagounder Natarajan, devicetree, Greg KH,
	OpenBMC Maillist, linux-kernel, linux-gpio, Rob Herring,
	Joel Stanley, linux-arm-kernel

On Wed, Dec 14, 2016 at 12:40 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Looking at this more, it's a SPI driver, presumably because the first
> case where it appeared was on a SPI bus.
>
> However, it's not a SPI device as such, it's a piece of standard,
> general purpose logic that's been around for many years, pre-dating
> the SPI bus.

Indeed.

> I think a much more sensible approach would be to turn the GPIO side
> of the 74x164 driver into a library, which can be re-used by multiple
> bus-specific drivers - one for SPI which allows it to be used in its
> current form, one for our platform bus which takes the GPIO lines for
> the data, clock and clear signals.
>
> I also don't see why they shouldn't use the same compatible - they're
> the same _device_ at the end of the day, just wired up differently.
> It makes the binding documentation a little fun wrt what are required
> and optional properties, but nothing that shouldn't be too difficult.

I agree on both accounts.

Sorry for not seeing this in the first place, I was well aware that this
is a standard component and may be connected in a myriad of ways,
so I should have known better :(

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius
@ 2016-12-15 23:07             ` Linus Walleij
  0 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2016-12-15 23:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 14, 2016 at 12:40 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Looking at this more, it's a SPI driver, presumably because the first
> case where it appeared was on a SPI bus.
>
> However, it's not a SPI device as such, it's a piece of standard,
> general purpose logic that's been around for many years, pre-dating
> the SPI bus.

Indeed.

> I think a much more sensible approach would be to turn the GPIO side
> of the 74x164 driver into a library, which can be re-used by multiple
> bus-specific drivers - one for SPI which allows it to be used in its
> current form, one for our platform bus which takes the GPIO lines for
> the data, clock and clear signals.
>
> I also don't see why they shouldn't use the same compatible - they're
> the same _device_ at the end of the day, just wired up differently.
> It makes the binding documentation a little fun wrt what are required
> and optional properties, but nothing that shouldn't be too difficult.

I agree on both accounts.

Sorry for not seeing this in the first place, I was well aware that this
is a standard component and may be connected in a myriad of ways,
so I should have known better :(

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2016-12-14 20:05           ` David Daney
@ 2016-12-20  4:06             ` Jaghathiswari Rankappagounder Natarajan
  -1 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-20  4:06 UTC (permalink / raw)
  To: David Daney
  Cc: Arnd Bergmann, Neil Armstrong, Thomas Petazzoni, mark.rutland,
	devicetree, Greg KH, OpenBMC Maillist, linux, linux-kernel,
	robh+dt, Joel Stanley, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 4959 bytes --]

Hi,
Thanks all for the feedback; Further reimplementation of this driver will
be handled by
another member of my team. Please contact Nancy Yuen(yuenn@google.com) for
any details.
Summary of main points mentioned in the feedback were :
  - Address Russell's errno comments
  - Rebase per Joel's comment
  - Research fingerprint reader support (mentioned by Greg KH)
  - Research drivers/leds (mentioned by Arnd)
  - Reimplement either by making an API that all devices of this type could
use or in drivers/leds             model or the fingerprint reader model.



On Wed, Dec 14, 2016 at 12:05 PM, David Daney <ddaney.cavm@gmail.com> wrote:

> On 12/14/2016 06:15 AM, Arnd Bergmann wrote:
>
>> On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
>>
>>> On 12/14/2016 01:56 PM, Greg KH wrote:
>>>
>>>> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>>>>> Natarajan wrote:
>>>>>
>>>>> Documentation for the binding which provides an interface for adding
>>>>>> clock,
>>>>>> data and clear signal GPIO lines to control seven segment display.
>>>>>>
>>>>>> The platform device driver provides an API for displaying on two
>>>>>> 7-segment
>>>>>> displays, and implements the required bit-banging. The hardware
>>>>>> assumed is
>>>>>> 74HC164 wired to two 7-segment displays.
>>>>>>
>>>>>> The character device driver implements the user-space API for letting
>>>>>> a user
>>>>>> write to two 7-segment displays including any conversion methods
>>>>>> necessary
>>>>>> to map the user input to two 7-segment displays.
>>>>>>
>>>>>> Adding clock, data and clear signal GPIO lines in the devicetree to
>>>>>> control
>>>>>> seven segment display on zaius platform.
>>>>>>
>>>>>> The platform driver matches on the device tree node; the platform
>>>>>> driver also
>>>>>> initializes the character device.
>>>>>>
>>>>>> Tested that the seven segment display works properly by writing to the
>>>>>> character device file on a EVB AST2500 board which also has 74HC164
>>>>>> wired
>>>>>> to two 7-segment displays.
>>>>>>
>>>>>
>>>>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139986.html
>>>>>
>>>>> And the feedback from Greg KH was: we don't need a driver for that, do
>>>>> it from userspace. See:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139992.html
>>>>>
>>>>> So: good luck
>>>>>
>>>>
>>>> Did anyone ever write a library for this type of thing?
>>>>
>>>> Again, I don't want to see one-off drivers for random devices like this
>>>> that should be able to all be controlled from userspace in a common
>>>> manner.  Much like we did for fingerprint readers a long long time
>>>> ago...
>>>>
>>>>
>> Actually, it's more than a random interface, a lot of SoCs and boards
>>> actually have such displays
>>> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly
>>> stuff to only print a few
>>> characters a simple and clean driver could achieve.
>>> Some very well known SoCs even have integrated registers to lower the
>>> BOM and bypass the need for
>>> a 74HC164 like component and avoid gpio bit banging.
>>>
>>> My personal concern is that you could also need to drive more segments,
>>> thus 7-segments
>>> is too restrictive.
>>>
>>> But this driver is well structured, the gpio-bitbanging sub-driver is
>>> welcome.
>>>
>>
>> Maybe we can find a way to fit this into the existing drivers/leds/
>> subsystem?
>>
>> That already supports blinking, brightness and colour attributes of LEDs,
>> so could this be extended to support (one of) digit, number, character
>> or string with a common sysfs attribute and a way to hook up a led driver
>> to that?
>>
>
> We have a lot of boards with an 8-cell dot matrix LED. Each cell is
> programmed with an 8-bit value.  The mapping of these values to the dots
> defaults to ASCII character rendering, but there is the facility to install
> other bitmaps as well.
>
> Really I view these things not as part of the LED subsystem, but more as a
> very small frame buffer.
>
> We like to display entire words, and the most useful interface from a user
> point of view is something that consumes entire strings rather than having
> to manage each cell independently.
>
> You could imagine that if the text to be displayed were longer than the
> display, that the driver would make it continuously scroll.  I would like
> to see a framework where a simple character device were exposed, and from
> userspace you could do: "echo message > /dev/small-display" and get
> something sensible.
>
>
>>         Arnd
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 7130 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
@ 2016-12-20  4:06             ` Jaghathiswari Rankappagounder Natarajan
  0 siblings, 0 replies; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-20  4:06 UTC (permalink / raw)
  To: David Daney
  Cc: Arnd Bergmann, Neil Armstrong, Thomas Petazzoni, mark.rutland,
	devicetree, Greg KH, OpenBMC Maillist, linux, linux-kernel,
	robh+dt, Joel Stanley, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 4959 bytes --]

Hi,
Thanks all for the feedback; Further reimplementation of this driver will
be handled by
another member of my team. Please contact Nancy Yuen(yuenn@google.com) for
any details.
Summary of main points mentioned in the feedback were :
  - Address Russell's errno comments
  - Rebase per Joel's comment
  - Research fingerprint reader support (mentioned by Greg KH)
  - Research drivers/leds (mentioned by Arnd)
  - Reimplement either by making an API that all devices of this type could
use or in drivers/leds             model or the fingerprint reader model.



On Wed, Dec 14, 2016 at 12:05 PM, David Daney <ddaney.cavm@gmail.com> wrote:

> On 12/14/2016 06:15 AM, Arnd Bergmann wrote:
>
>> On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
>>
>>> On 12/14/2016 01:56 PM, Greg KH wrote:
>>>
>>>> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>>>>> Natarajan wrote:
>>>>>
>>>>> Documentation for the binding which provides an interface for adding
>>>>>> clock,
>>>>>> data and clear signal GPIO lines to control seven segment display.
>>>>>>
>>>>>> The platform device driver provides an API for displaying on two
>>>>>> 7-segment
>>>>>> displays, and implements the required bit-banging. The hardware
>>>>>> assumed is
>>>>>> 74HC164 wired to two 7-segment displays.
>>>>>>
>>>>>> The character device driver implements the user-space API for letting
>>>>>> a user
>>>>>> write to two 7-segment displays including any conversion methods
>>>>>> necessary
>>>>>> to map the user input to two 7-segment displays.
>>>>>>
>>>>>> Adding clock, data and clear signal GPIO lines in the devicetree to
>>>>>> control
>>>>>> seven segment display on zaius platform.
>>>>>>
>>>>>> The platform driver matches on the device tree node; the platform
>>>>>> driver also
>>>>>> initializes the character device.
>>>>>>
>>>>>> Tested that the seven segment display works properly by writing to the
>>>>>> character device file on a EVB AST2500 board which also has 74HC164
>>>>>> wired
>>>>>> to two 7-segment displays.
>>>>>>
>>>>>
>>>>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139986.html
>>>>>
>>>>> And the feedback from Greg KH was: we don't need a driver for that, do
>>>>> it from userspace. See:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139992.html
>>>>>
>>>>> So: good luck
>>>>>
>>>>
>>>> Did anyone ever write a library for this type of thing?
>>>>
>>>> Again, I don't want to see one-off drivers for random devices like this
>>>> that should be able to all be controlled from userspace in a common
>>>> manner.  Much like we did for fingerprint readers a long long time
>>>> ago...
>>>>
>>>>
>> Actually, it's more than a random interface, a lot of SoCs and boards
>>> actually have such displays
>>> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly
>>> stuff to only print a few
>>> characters a simple and clean driver could achieve.
>>> Some very well known SoCs even have integrated registers to lower the
>>> BOM and bypass the need for
>>> a 74HC164 like component and avoid gpio bit banging.
>>>
>>> My personal concern is that you could also need to drive more segments,
>>> thus 7-segments
>>> is too restrictive.
>>>
>>> But this driver is well structured, the gpio-bitbanging sub-driver is
>>> welcome.
>>>
>>
>> Maybe we can find a way to fit this into the existing drivers/leds/
>> subsystem?
>>
>> That already supports blinking, brightness and colour attributes of LEDs,
>> so could this be extended to support (one of) digit, number, character
>> or string with a common sysfs attribute and a way to hook up a led driver
>> to that?
>>
>
> We have a lot of boards with an 8-cell dot matrix LED. Each cell is
> programmed with an 8-bit value.  The mapping of these values to the dots
> defaults to ASCII character rendering, but there is the facility to install
> other bitmaps as well.
>
> Really I view these things not as part of the LED subsystem, but more as a
> very small frame buffer.
>
> We like to display entire words, and the most useful interface from a user
> point of view is something that consumes entire strings rather than having
> to manage each cell independently.
>
> You could imagine that if the text to be displayed were longer than the
> display, that the driver would make it continuously scroll.  I would like
> to see a framework where a simple character device were exposed, and from
> userspace you could do: "echo message > /dev/small-display" and get
> something sensible.
>
>
>>         Arnd
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 7130 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH linux v1 0/4] Seven segment display support
  2017-02-07  4:03 Jaghathiswari Rankappagounder Natarajan
@ 2017-02-09  3:08 ` Joel Stanley
  0 siblings, 0 replies; 54+ messages in thread
From: Joel Stanley @ 2017-02-09  3:08 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan; +Cc: OpenBMC Maillist, Nancy Yuen

Hello Jagha,

On Tue, Feb 7, 2017 at 2:33 PM, Jaghathiswari Rankappagounder
Natarajan <jaghu@google.com> wrote:
> Hi Joel,
> Nancy mentioned this patch can be included into openbmc linux tree now.
> Some other member will take up the work of addressing the comments from
> upstream reviewers and send the patch upstream.

Just before you sent these patches we had a chat on the topic. I
thought we decided to give the rest of this week for people to review
the new process, and then on Monday I would publish a tree based on
4.10 that you could submit patches to. Sorry for not making this
clear.

I suspect this set will apply to 4.10 as-is, so I will consider this
your submission for the 4.10 tree next week.

Next time please have a read of the development process[1] and the
rules for submitting patches[2] before sending patches. In particular,
you need to set the target branch in the subject line so I know where
you want the patches applied. If you you have any questions about the
process please let me know so I can update the documentation.

[1] https://github.com/openbmc/linux/wiki/DevelopmentProcess
[2] https://github.com/openbmc/linux/wiki/SubmittingPatches

Cheers,

Joel

>
> This patchset includes:
>
> Documentation for the binding which provides an interface for adding clock,
> data and clear signal GPIO lines to control seven segment display.
>
> The platform device driver provides an API for displaying on two 7-segment
> displays, and implements the required bit-banging. The hardware assumed is
> 74HC164 wired to two 7-segment displays.
>
> The character device driver implements the user-space API for letting a user
> write to two 7-segment displays including any conversion methods necessary
> to map the user input to two 7-segment displays.
>
> Adding clock, data and clear signal GPIO lines in the devicetree to control
> seven segment display on zaius platform.
>
> The platform driver matches on the device tree node; the platform driver also
> initializes the character device.
>
> Tested that the seven segment display works properly by writing to the
> character device file on a EVB AST2500 board which also has 74HC164 wired
> to two 7-segment displays.
>
> Jaghathiswari Rankappagounder Natarajan (4):
>   Documentation: dt-bindings: Document bindings for seven segment
>     display support
>   drivers: misc: Character device driver for seven segment display
>   drivers: misc: Platform driver for seven segment display support
>   arm: dts: Add dt-binding to support seven segment display on zaius
>
>  .../devicetree/bindings/misc/seven-seg-gpio.txt    |  27 +++
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts         |   8 +
>  drivers/misc/Kconfig                               |  16 ++
>  drivers/misc/Makefile                              |   2 +
>  drivers/misc/seven_seg_disp.c                      | 201 ++++++++++++++++++++
>  drivers/misc/seven_seg_disp.h                      |  34 ++++
>  drivers/misc/seven_seg_gpio.c                      | 206 +++++++++++++++++++++
>  7 files changed, 494 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
>  create mode 100644 drivers/misc/seven_seg_disp.c
>  create mode 100644 drivers/misc/seven_seg_disp.h
>  create mode 100644 drivers/misc/seven_seg_gpio.c
>
> --
> 2.11.0.483.g087da7b7c-goog
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH linux v1 0/4] Seven segment display support
@ 2017-02-07  4:03 Jaghathiswari Rankappagounder Natarajan
  2017-02-09  3:08 ` Joel Stanley
  0 siblings, 1 reply; 54+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2017-02-07  4:03 UTC (permalink / raw)
  To: openbmc, joel; +Cc: Jaghathiswari Rankappagounder Natarajan

Hi Joel,
Nancy mentioned this patch can be included into openbmc linux tree now.
Some other member will take up the work of addressing the comments from
upstream reviewers and send the patch upstream.

This patchset includes:

Documentation for the binding which provides an interface for adding clock,
data and clear signal GPIO lines to control seven segment display.

The platform device driver provides an API for displaying on two 7-segment
displays, and implements the required bit-banging. The hardware assumed is
74HC164 wired to two 7-segment displays.

The character device driver implements the user-space API for letting a user
write to two 7-segment displays including any conversion methods necessary
to map the user input to two 7-segment displays.

Adding clock, data and clear signal GPIO lines in the devicetree to control
seven segment display on zaius platform.

The platform driver matches on the device tree node; the platform driver also
initializes the character device.

Tested that the seven segment display works properly by writing to the
character device file on a EVB AST2500 board which also has 74HC164 wired
to two 7-segment displays.

Jaghathiswari Rankappagounder Natarajan (4):
  Documentation: dt-bindings: Document bindings for seven segment
    display support
  drivers: misc: Character device driver for seven segment display
  drivers: misc: Platform driver for seven segment display support
  arm: dts: Add dt-binding to support seven segment display on zaius

 .../devicetree/bindings/misc/seven-seg-gpio.txt    |  27 +++
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts         |   8 +
 drivers/misc/Kconfig                               |  16 ++
 drivers/misc/Makefile                              |   2 +
 drivers/misc/seven_seg_disp.c                      | 201 ++++++++++++++++++++
 drivers/misc/seven_seg_disp.h                      |  34 ++++
 drivers/misc/seven_seg_gpio.c                      | 206 +++++++++++++++++++++
 7 files changed, 494 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/seven-seg-gpio.txt
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_disp.h
 create mode 100644 drivers/misc/seven_seg_gpio.c

--
2.11.0.483.g087da7b7c-goog

^ permalink raw reply	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2017-02-09  3:09 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-14  7:55 [PATCH linux v1 0/4] Seven segment display support Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55 ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55 ` [PATCH linux v1 1/4] Documentation: dt-bindings: Document bindings for seven " Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55 ` [PATCH linux v1 2/4] drivers: misc: Character device driver for seven segment display Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14 12:32   ` Russell King - ARM Linux
2016-12-14 12:32     ` Russell King - ARM Linux
2016-12-14 12:32     ` Russell King - ARM Linux
2016-12-14  7:55 ` [PATCH linux v1 3/4] drivers: misc: Platform driver for seven segment display support Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55 ` [PATCH linux v1 4/4] arm: dts: Add dt-binding to support seven segment display on zaius Jaghathiswari Rankappagounder Natarajan
2016-12-14  7:55   ` Jaghathiswari Rankappagounder Natarajan
2016-12-14  8:55   ` Arnd Bergmann
2016-12-14  8:55     ` Arnd Bergmann
2016-12-14  8:55     ` Arnd Bergmann
2016-12-14  9:00     ` Arnd Bergmann
2016-12-14  9:00       ` Arnd Bergmann
2016-12-14 11:06       ` Russell King - ARM Linux
2016-12-14 11:06         ` Russell King - ARM Linux
2016-12-14 11:06         ` Russell King - ARM Linux
2016-12-14 11:40         ` Russell King - ARM Linux
2016-12-14 11:40           ` Russell King - ARM Linux
2016-12-15 23:07           ` Linus Walleij
2016-12-15 23:07             ` Linus Walleij
2016-12-15 23:07             ` Linus Walleij
2016-12-15 23:07             ` Linus Walleij
2016-12-14  9:02   ` Joel Stanley
2016-12-14  9:02     ` Joel Stanley
2016-12-14  9:02     ` Joel Stanley
2016-12-14  9:02     ` Joel Stanley
2016-12-14 12:45 ` [PATCH linux v1 0/4] Seven segment display support Thomas Petazzoni
2016-12-14 12:45   ` Thomas Petazzoni
2016-12-14 12:45   ` Thomas Petazzoni
2016-12-14 12:56   ` Greg KH
2016-12-14 12:56     ` Greg KH
2016-12-14 12:56     ` Greg KH
2016-12-14 13:12     ` Neil Armstrong
2016-12-14 13:12       ` Neil Armstrong
2016-12-14 14:15       ` Arnd Bergmann
2016-12-14 14:15         ` Arnd Bergmann
2016-12-14 14:15         ` Arnd Bergmann
2016-12-14 20:05         ` David Daney
2016-12-14 20:05           ` David Daney
2016-12-20  4:06           ` Jaghathiswari Rankappagounder Natarajan
2016-12-20  4:06             ` Jaghathiswari Rankappagounder Natarajan
2016-12-14 16:50       ` Greg KH
2016-12-14 16:50         ` Greg KH
2016-12-14 14:35 ` Patrick Williams
2016-12-14 23:23   ` Rob Lippert
2017-02-07  4:03 Jaghathiswari Rankappagounder Natarajan
2017-02-09  3:08 ` Joel Stanley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.