[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear drivers/{char,misc}/ maintainers,

This patch series implements a simple driver for 7-segment displays
connected over GPIOs through a BCD encoder. The userspace interface of
the driver is very simple: just a "value" file in the sysfs directory
of the device, which can be written to (changes the value shown on the
7-segment display) and read (gives the current value).

This driver has been tested on Marvell Armada 370 and Marvell Armada
XP evaluation boards, but could potentially be used by anybody having
a 7-segment display controlled over GPIOs with a BCD encoder.

 * Patch 1 adds the driver itself, together with the Device Tree
   binding documentation.

 * Patch 2 and 3 add the necessary Device Tree information to use this
   driver on the Armada 370 and Armada XP evaluation boards.

Best regards,

Thomas

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear drivers/{char,misc}/ maintainers,

This patch series implements a simple driver for 7-segment displays
connected over GPIOs through a BCD encoder. The userspace interface of
the driver is very simple: just a "value" file in the sysfs directory
of the device, which can be written to (changes the value shown on the
7-segment display) and read (gives the current value).

This driver has been tested on Marvell Armada 370 and Marvell Armada
XP evaluation boards, but could potentially be used by anybody having
a 7-segment display controlled over GPIOs with a BCD encoder.

 * Patch 1 adds the driver itself, together with the Device Tree
   binding documentation.

 * Patch 2 and 3 add the necessary Device Tree information to use this
   driver on the Armada 370 and Armada XP evaluation boards.

Best regards,

Thomas

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Thomas Petazzoni]

This patch adds a driver to control 7-segment displays connected over
GPIOs through a BCD encoder. Typically, up to four GPIOs go into a BCD
encoder, that drives the 7-segment display to display a numeric value
between 0 and 2^ngpios-1.

Once this driver is enabled, it creates a 'value' field in the sysfs
directory of the device, which allows to set the current value
displayed by the 7-segment.

It has been successfully tested on the Marvell Armada 370 and Marvell
Armada XP evaluation boards. Both of them have a 7-segment display
that can be controlled using 3 GPIOs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
 drivers/misc/Kconfig                               |   13 ++
 drivers/misc/Makefile                              |    1 +
 drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
 4 files changed, 200 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/gpio-7seg.txt
 create mode 100644 drivers/misc/gpio-7seg.c

diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
new file mode 100644
index 0000000..107d178
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
@@ -0,0 +1,18 @@
+* 7-segment driver connected over GPIO through a BCD decoder
+
+Required properties:
+- compatible: "generic,gpio-7seg"
+- gpios: list of GPIOs to use to control the 7-segment display
+
+Optional properties:
+- default-value: default value shown by the 7-segment display at boot
+  time. If not defined, defaults to 0.
+
+Example:
+
+gpio-7seg@0 {
+	compatible = "generic,gpio-7seg";
+	status = "okay";
+	gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
+	default-value = <5>;
+};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b151b7c..19f9d22 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -93,6 +93,19 @@ config ATMEL_TCB_CLKSRC_BLOCK
 	  TC can be used for other purposes, such as PWM generation and
 	  interval timing.
 
+config GPIO_7SEG
+	tristate "GPIO-connected 7-segment display driver"
+	help
+	  This option enables a driver to control 7-segment displays
+	  connected over GPIOs through a BCD encoder. Typically, up to
+	  four GPIOs go into a BCD encoder, that drives the 7-segment
+	  display to display a numeric value between 0 and
+	  2^ngpios-1.
+
+	  Once this driver is enabled, it creates a 'value' field in
+	  the sysfs directory of the device, which allows to set the
+	  current value displayed by the 7-segment.
+
 config IBM_ASM
 	tristate "Device driver for IBM RSA service processor"
 	depends on X86 && PCI && INPUT
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 2129377..b789358 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_INTEL_MID_PTI)	+= pti.o
 obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
+obj-$(CONFIG_GPIO_7SEG)         += gpio-7seg.o
 obj-$(CONFIG_BMP085)		+= bmp085.o
 obj-$(CONFIG_BMP085_I2C)	+= bmp085-i2c.o
 obj-$(CONFIG_BMP085_SPI)	+= bmp085-spi.o
diff --git a/drivers/misc/gpio-7seg.c b/drivers/misc/gpio-7seg.c
new file mode 100644
index 0000000..8cc5113
--- /dev/null
+++ b/drivers/misc/gpio-7seg.c
@@ -0,0 +1,168 @@
+/*
+ * Simple driver to control a 7 segment display connected through a
+ * BCD decoder using GPIOs.
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+
+struct gpio_7seg_dev {
+	struct device_attribute dev_attr;
+	int *gpios;
+	int ngpios;
+	int value;
+	int maxvalue;
+};
+
+static void gpio_7seg_display_val(struct gpio_7seg_dev *sdev,
+				  int value)
+{
+	int i;
+
+	for (i = 0; i < sdev->ngpios; i++) {
+		int bitval = (value & BIT(i)) ? 1 : 0;
+		gpio_set_value_cansleep(sdev->gpios[i], bitval);
+	}
+}
+
+static ssize_t gpio_7seg_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
+	return snprintf(buf, PAGE_SIZE, "%d\n", sdev->value);
+}
+
+static ssize_t gpio_7seg_store(struct device *dev, struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
+	unsigned long value;
+	char *end;
+
+	value = simple_strtoul(buf, &end, 0);
+	if (end == buf)
+		return -EINVAL;
+
+	if (value >= sdev->maxvalue)
+		return -EINVAL;
+
+	gpio_7seg_display_val(sdev, value);
+	sdev->value = value;
+
+	return count;
+}
+
+static int gpio_7seg_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct gpio_7seg_dev *sdev;
+	int ret, i;
+
+	/*
+	 * We only support being probed through the device tree, for
+	 * now
+	 */
+	if (!np)
+		return -EINVAL;
+
+	sdev = devm_kzalloc(&pdev->dev, sizeof(struct gpio_7seg_dev),
+			    GFP_KERNEL);
+	if (!sdev)
+		return -ENOMEM;
+
+	sdev->ngpios = of_gpio_count(np);
+	if (!sdev->ngpios)
+		return -EINVAL;
+
+	sdev->gpios = devm_kzalloc(&pdev->dev, sizeof(int) * sdev->ngpios,
+				   GFP_KERNEL);
+	if (!sdev->gpios)
+		return -ENOMEM;
+
+	for (i = 0; i < sdev->ngpios; i++) {
+		sdev->gpios[i] = of_get_gpio(np, i);
+		ret = gpio_request_one(sdev->gpios[i], GPIOF_DIR_OUT, NULL);
+		if (ret) {
+			/*
+			 * Mark this GPIO as non-requested for the
+			 * error handling code
+			 */
+			sdev->gpios[i] = ret;
+			goto gpio_cleanup;
+		}
+	}
+
+	ret = of_property_read_u32(np, "default-value", &sdev->value);
+	if (ret)
+		sdev->value = 0;
+
+	sdev->maxvalue = 1 << sdev->ngpios;
+
+	sdev->dev_attr.attr.name = "value";
+	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
+	sdev->dev_attr.show = gpio_7seg_show;
+	sdev->dev_attr.store = gpio_7seg_store;
+
+	ret = device_create_file(&pdev->dev, &sdev->dev_attr);
+	if (ret)
+		goto gpio_cleanup;
+
+	gpio_7seg_display_val(sdev, sdev->value);
+
+	platform_set_drvdata(pdev, sdev);
+	return 0;
+
+gpio_cleanup:
+	for (i = 0; i < sdev->ngpios; i++)
+		if (gpio_is_valid(sdev->gpios[i]))
+			gpio_free(sdev->gpios[i]);
+	return ret;
+}
+
+static int gpio_7seg_remove(struct platform_device *pdev)
+{
+	struct gpio_7seg_dev *sdev;
+	int i;
+
+	sdev = platform_get_drvdata(pdev);
+
+	device_remove_file(&pdev->dev, &sdev->dev_attr);
+
+	for (i = 0; i < sdev->ngpios; i++)
+		if (gpio_is_valid(sdev->gpios[i]))
+			gpio_free(sdev->gpios[i]);
+	return 0;
+}
+
+static const struct of_device_id gpio_7seg_of_match_table[] = {
+	{ .compatible = "generic,gpio-7seg" },
+	{},
+};
+
+static struct platform_driver gpio_7seg_driver = {
+	.probe  = gpio_7seg_probe,
+	.remove = gpio_7seg_remove,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = "gpio-7seg",
+		.of_match_table = of_match_ptr(gpio_7seg_of_match_table),
+	},
+};
+
+module_platform_driver(gpio_7seg_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com>");
+MODULE_DESCRIPTION("Simple GPIO-connected 7-segment display driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Thomas Petazzoni]

This patch adds a driver to control 7-segment displays connected over
GPIOs through a BCD encoder. Typically, up to four GPIOs go into a BCD
encoder, that drives the 7-segment display to display a numeric value
between 0 and 2^ngpios-1.

Once this driver is enabled, it creates a 'value' field in the sysfs
directory of the device, which allows to set the current value
displayed by the 7-segment.

It has been successfully tested on the Marvell Armada 370 and Marvell
Armada XP evaluation boards. Both of them have a 7-segment display
that can be controlled using 3 GPIOs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
 drivers/misc/Kconfig                               |   13 ++
 drivers/misc/Makefile                              |    1 +
 drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
 4 files changed, 200 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/gpio-7seg.txt
 create mode 100644 drivers/misc/gpio-7seg.c

diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
new file mode 100644
index 0000000..107d178
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
@@ -0,0 +1,18 @@
+* 7-segment driver connected over GPIO through a BCD decoder
+
+Required properties:
+- compatible: "generic,gpio-7seg"
+- gpios: list of GPIOs to use to control the 7-segment display
+
+Optional properties:
+- default-value: default value shown by the 7-segment display at boot
+  time. If not defined, defaults to 0.
+
+Example:
+
+gpio-7seg at 0 {
+	compatible = "generic,gpio-7seg";
+	status = "okay";
+	gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
+	default-value = <5>;
+};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b151b7c..19f9d22 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -93,6 +93,19 @@ config ATMEL_TCB_CLKSRC_BLOCK
 	  TC can be used for other purposes, such as PWM generation and
 	  interval timing.
 
+config GPIO_7SEG
+	tristate "GPIO-connected 7-segment display driver"
+	help
+	  This option enables a driver to control 7-segment displays
+	  connected over GPIOs through a BCD encoder. Typically, up to
+	  four GPIOs go into a BCD encoder, that drives the 7-segment
+	  display to display a numeric value between 0 and
+	  2^ngpios-1.
+
+	  Once this driver is enabled, it creates a 'value' field in
+	  the sysfs directory of the device, which allows to set the
+	  current value displayed by the 7-segment.
+
 config IBM_ASM
 	tristate "Device driver for IBM RSA service processor"
 	depends on X86 && PCI && INPUT
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 2129377..b789358 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_INTEL_MID_PTI)	+= pti.o
 obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
+obj-$(CONFIG_GPIO_7SEG)         += gpio-7seg.o
 obj-$(CONFIG_BMP085)		+= bmp085.o
 obj-$(CONFIG_BMP085_I2C)	+= bmp085-i2c.o
 obj-$(CONFIG_BMP085_SPI)	+= bmp085-spi.o
diff --git a/drivers/misc/gpio-7seg.c b/drivers/misc/gpio-7seg.c
new file mode 100644
index 0000000..8cc5113
--- /dev/null
+++ b/drivers/misc/gpio-7seg.c
@@ -0,0 +1,168 @@
+/*
+ * Simple driver to control a 7 segment display connected through a
+ * BCD decoder using GPIOs.
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+
+struct gpio_7seg_dev {
+	struct device_attribute dev_attr;
+	int *gpios;
+	int ngpios;
+	int value;
+	int maxvalue;
+};
+
+static void gpio_7seg_display_val(struct gpio_7seg_dev *sdev,
+				  int value)
+{
+	int i;
+
+	for (i = 0; i < sdev->ngpios; i++) {
+		int bitval = (value & BIT(i)) ? 1 : 0;
+		gpio_set_value_cansleep(sdev->gpios[i], bitval);
+	}
+}
+
+static ssize_t gpio_7seg_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
+	return snprintf(buf, PAGE_SIZE, "%d\n", sdev->value);
+}
+
+static ssize_t gpio_7seg_store(struct device *dev, struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
+	unsigned long value;
+	char *end;
+
+	value = simple_strtoul(buf, &end, 0);
+	if (end == buf)
+		return -EINVAL;
+
+	if (value >= sdev->maxvalue)
+		return -EINVAL;
+
+	gpio_7seg_display_val(sdev, value);
+	sdev->value = value;
+
+	return count;
+}
+
+static int gpio_7seg_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct gpio_7seg_dev *sdev;
+	int ret, i;
+
+	/*
+	 * We only support being probed through the device tree, for
+	 * now
+	 */
+	if (!np)
+		return -EINVAL;
+
+	sdev = devm_kzalloc(&pdev->dev, sizeof(struct gpio_7seg_dev),
+			    GFP_KERNEL);
+	if (!sdev)
+		return -ENOMEM;
+
+	sdev->ngpios = of_gpio_count(np);
+	if (!sdev->ngpios)
+		return -EINVAL;
+
+	sdev->gpios = devm_kzalloc(&pdev->dev, sizeof(int) * sdev->ngpios,
+				   GFP_KERNEL);
+	if (!sdev->gpios)
+		return -ENOMEM;
+
+	for (i = 0; i < sdev->ngpios; i++) {
+		sdev->gpios[i] = of_get_gpio(np, i);
+		ret = gpio_request_one(sdev->gpios[i], GPIOF_DIR_OUT, NULL);
+		if (ret) {
+			/*
+			 * Mark this GPIO as non-requested for the
+			 * error handling code
+			 */
+			sdev->gpios[i] = ret;
+			goto gpio_cleanup;
+		}
+	}
+
+	ret = of_property_read_u32(np, "default-value", &sdev->value);
+	if (ret)
+		sdev->value = 0;
+
+	sdev->maxvalue = 1 << sdev->ngpios;
+
+	sdev->dev_attr.attr.name = "value";
+	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
+	sdev->dev_attr.show = gpio_7seg_show;
+	sdev->dev_attr.store = gpio_7seg_store;
+
+	ret = device_create_file(&pdev->dev, &sdev->dev_attr);
+	if (ret)
+		goto gpio_cleanup;
+
+	gpio_7seg_display_val(sdev, sdev->value);
+
+	platform_set_drvdata(pdev, sdev);
+	return 0;
+
+gpio_cleanup:
+	for (i = 0; i < sdev->ngpios; i++)
+		if (gpio_is_valid(sdev->gpios[i]))
+			gpio_free(sdev->gpios[i]);
+	return ret;
+}
+
+static int gpio_7seg_remove(struct platform_device *pdev)
+{
+	struct gpio_7seg_dev *sdev;
+	int i;
+
+	sdev = platform_get_drvdata(pdev);
+
+	device_remove_file(&pdev->dev, &sdev->dev_attr);
+
+	for (i = 0; i < sdev->ngpios; i++)
+		if (gpio_is_valid(sdev->gpios[i]))
+			gpio_free(sdev->gpios[i]);
+	return 0;
+}
+
+static const struct of_device_id gpio_7seg_of_match_table[] = {
+	{ .compatible = "generic,gpio-7seg" },
+	{},
+};
+
+static struct platform_driver gpio_7seg_driver = {
+	.probe  = gpio_7seg_probe,
+	.remove = gpio_7seg_remove,
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = "gpio-7seg",
+		.of_match_table = of_match_ptr(gpio_7seg_of_match_table),
+	},
+};
+
+module_platform_driver(gpio_7seg_driver);
+
+MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com>");
+MODULE_DESCRIPTION("Simple GPIO-connected 7-segment display driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH 3.9 2/3] arm: mvebu: add the 7-segment display DT informations to Armada 370 DB

[Thomas Petazzoni]

Now that we have a 7-segment display driver in the kernel, use it on
the Marvell Armada 370 DB evaluation board.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-db.dts |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index 0004402..c433072 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -59,5 +59,10 @@
 			phy = <&phy1>;
 			phy-mode = "rgmii-id";
 		};
+		gpio-7seg@0 {
+			compatible = "generic,gpio-7seg";
+			status = "okay";
+			gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH 3.9 2/3] arm: mvebu: add the 7-segment display DT informations to Armada 370 DB

[Thomas Petazzoni]

Now that we have a 7-segment display driver in the kernel, use it on
the Marvell Armada 370 DB evaluation board.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-db.dts |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index 0004402..c433072 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -59,5 +59,10 @@
 			phy = <&phy1>;
 			phy-mode = "rgmii-id";
 		};
+		gpio-7seg at 0 {
+			compatible = "generic,gpio-7seg";
+			status = "okay";
+			gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH 3.9 3/3] arm: mvebu: add the 7-segment display DT informations to Armada XP DB

[Thomas Petazzoni]

Now that we have a 7-segment display driver in the kernel, use it on
the Marvell Armada XP DB evaluation board.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-db.dts |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
index 8e53b25..8654c1b 100644
--- a/arch/arm/boot/dts/armada-xp-db.dts
+++ b/arch/arm/boot/dts/armada-xp-db.dts
@@ -90,5 +90,10 @@
 			phy = <&phy3>;
 			phy-mode = "sgmii";
 		};
+		gpio-7seg@0 {
+			compatible = "generic,gpio-7seg";
+			status = "okay";
+			gpios = <&gpio0 26 0 &gpio0 27 0 &gpio1 16 0>;
+		};
 	};
 };
-- 
1.7.9.5

[PATCH 3.9 3/3] arm: mvebu: add the 7-segment display DT informations to Armada XP DB

[Thomas Petazzoni]

Now that we have a 7-segment display driver in the kernel, use it on
the Marvell Armada XP DB evaluation board.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-db.dts |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
index 8e53b25..8654c1b 100644
--- a/arch/arm/boot/dts/armada-xp-db.dts
+++ b/arch/arm/boot/dts/armada-xp-db.dts
@@ -90,5 +90,10 @@
 			phy = <&phy3>;
 			phy-mode = "sgmii";
 		};
+		gpio-7seg at 0 {
+			compatible = "generic,gpio-7seg";
+			status = "okay";
+			gpios = <&gpio0 26 0 &gpio0 27 0 &gpio1 16 0>;
+		};
 	};
 };
-- 
1.7.9.5

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 05:42:05PM +0100, Thomas Petazzoni wrote:
> Dear drivers/{char,misc}/ maintainers,
> 
> This patch series implements a simple driver for 7-segment displays
> connected over GPIOs through a BCD encoder. The userspace interface of
> the driver is very simple: just a "value" file in the sysfs directory
> of the device, which can be written to (changes the value shown on the
> 7-segment display) and read (gives the current value).

If you ever add/remove/modify sysfs files, you have to also do the same
for the Documentation/ABI/ files as well, please redo that in this patch
series.

But, the bigger question is, why is this a kernel driver at all?  Can't
you do this from userspace today without any new kernel code?

thanks,

greg k-h

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 05:42:05PM +0100, Thomas Petazzoni wrote:
> Dear drivers/{char,misc}/ maintainers,
> 
> This patch series implements a simple driver for 7-segment displays
> connected over GPIOs through a BCD encoder. The userspace interface of
> the driver is very simple: just a "value" file in the sysfs directory
> of the device, which can be written to (changes the value shown on the
> 7-segment display) and read (gives the current value).

If you ever add/remove/modify sysfs files, you have to also do the same
for the Documentation/ABI/ files as well, please redo that in this patch
series.

But, the bigger question is, why is this a kernel driver at all?  Can't
you do this from userspace today without any new kernel code?

thanks,

greg k-h

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 08:48:45 -0800, Greg Kroah-Hartman wrote:

> If you ever add/remove/modify sysfs files, you have to also do the same
> for the Documentation/ABI/ files as well, please redo that in this patch
> series.

Sure.

> But, the bigger question is, why is this a kernel driver at all?  Can't
> you do this from userspace today without any new kernel code?

Indeed, it can be done from userspace since we're just controlling
GPIOs. Having a kernel driver allows to describe this device in the
Device Tree, like all other devices, and have it "magically" appear,
with a convenient user-space interface.

Not having a kernel driver means that gazillions of applications
re-invent the same piece of code over and over again, have to hardcode
the GPIO numbers for a given piece of hardware, while the kernel
abstract all of this very nicely.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 08:48:45 -0800, Greg Kroah-Hartman wrote:

> If you ever add/remove/modify sysfs files, you have to also do the same
> for the Documentation/ABI/ files as well, please redo that in this patch
> series.

Sure.

> But, the bigger question is, why is this a kernel driver at all?  Can't
> you do this from userspace today without any new kernel code?

Indeed, it can be done from userspace since we're just controlling
GPIOs. Having a kernel driver allows to describe this device in the
Device Tree, like all other devices, and have it "magically" appear,
with a convenient user-space interface.

Not having a kernel driver means that gazillions of applications
re-invent the same piece of code over and over again, have to hardcode
the GPIO numbers for a given piece of hardware, while the kernel
abstract all of this very nicely.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 06:07:08PM +0100, Thomas Petazzoni wrote:
> Dear Greg Kroah-Hartman,
> 
> On Mon, 7 Jan 2013 08:48:45 -0800, Greg Kroah-Hartman wrote:
> 
> > If you ever add/remove/modify sysfs files, you have to also do the same
> > for the Documentation/ABI/ files as well, please redo that in this patch
> > series.
> 
> Sure.
> 
> > But, the bigger question is, why is this a kernel driver at all?  Can't
> > you do this from userspace today without any new kernel code?
> 
> Indeed, it can be done from userspace since we're just controlling
> GPIOs. Having a kernel driver allows to describe this device in the
> Device Tree, like all other devices, and have it "magically" appear,
> with a convenient user-space interface.

Ok, that means you want to use the kernel-standard userspace interface
for displays, right?  If so, why not use it?  I thought we already
supported LCD displays already, isn't there an interface for it?  Don't
create random magic sysfs files without really thinking about it.

> Not having a kernel driver means that gazillions of applications
> re-invent the same piece of code over and over again, have to hardcode
> the GPIO numbers for a given piece of hardware, while the kernel
> abstract all of this very nicely.

That sounds like a wonderful use of a userspace library to do this
properly.  Much like libusb does, right?

I still think as this can be done in userspace, it probably should be.

thanks,

greg k-h

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 06:07:08PM +0100, Thomas Petazzoni wrote:
> Dear Greg Kroah-Hartman,
> 
> On Mon, 7 Jan 2013 08:48:45 -0800, Greg Kroah-Hartman wrote:
> 
> > If you ever add/remove/modify sysfs files, you have to also do the same
> > for the Documentation/ABI/ files as well, please redo that in this patch
> > series.
> 
> Sure.
> 
> > But, the bigger question is, why is this a kernel driver at all?  Can't
> > you do this from userspace today without any new kernel code?
> 
> Indeed, it can be done from userspace since we're just controlling
> GPIOs. Having a kernel driver allows to describe this device in the
> Device Tree, like all other devices, and have it "magically" appear,
> with a convenient user-space interface.

Ok, that means you want to use the kernel-standard userspace interface
for displays, right?  If so, why not use it?  I thought we already
supported LCD displays already, isn't there an interface for it?  Don't
create random magic sysfs files without really thinking about it.

> Not having a kernel driver means that gazillions of applications
> re-invent the same piece of code over and over again, have to hardcode
> the GPIO numbers for a given piece of hardware, while the kernel
> abstract all of this very nicely.

That sounds like a wonderful use of a userspace library to do this
properly.  Much like libusb does, right?

I still think as this can be done in userspace, it probably should be.

thanks,

greg k-h

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:

> > Indeed, it can be done from userspace since we're just controlling
> > GPIOs. Having a kernel driver allows to describe this device in the
> > Device Tree, like all other devices, and have it "magically" appear,
> > with a convenient user-space interface.
> 
> Ok, that means you want to use the kernel-standard userspace interface
> for displays, right?  If so, why not use it?  I thought we already
> supported LCD displays already, isn't there an interface for it?  Don't
> create random magic sysfs files without really thinking about it.

Well, the LCD interface is about outputting pixel into a video memory.
Quite different from:

echo 6 > /sys/[...]/gpio-7seg.1/value

No?

> > Not having a kernel driver means that gazillions of applications
> > re-invent the same piece of code over and over again, have to hardcode
> > the GPIO numbers for a given piece of hardware, while the kernel
> > abstract all of this very nicely.
> 
> That sounds like a wonderful use of a userspace library to do this
> properly.  Much like libusb does, right?
> 
> I still think as this can be done in userspace, it probably should be.

Understood. Patches discarded.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:

> > Indeed, it can be done from userspace since we're just controlling
> > GPIOs. Having a kernel driver allows to describe this device in the
> > Device Tree, like all other devices, and have it "magically" appear,
> > with a convenient user-space interface.
> 
> Ok, that means you want to use the kernel-standard userspace interface
> for displays, right?  If so, why not use it?  I thought we already
> supported LCD displays already, isn't there an interface for it?  Don't
> create random magic sysfs files without really thinking about it.

Well, the LCD interface is about outputting pixel into a video memory.
Quite different from:

echo 6 > /sys/[...]/gpio-7seg.1/value

No?

> > Not having a kernel driver means that gazillions of applications
> > re-invent the same piece of code over and over again, have to hardcode
> > the GPIO numbers for a given piece of hardware, while the kernel
> > abstract all of this very nicely.
> 
> That sounds like a wonderful use of a userspace library to do this
> properly.  Much like libusb does, right?
> 
> I still think as this can be done in userspace, it probably should be.

Understood. Patches discarded.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:

> > > Not having a kernel driver means that gazillions of applications
> > > re-invent the same piece of code over and over again, have to hardcode
> > > the GPIO numbers for a given piece of hardware, while the kernel
> > > abstract all of this very nicely.
> > 
> > That sounds like a wonderful use of a userspace library to do this
> > properly.  Much like libusb does, right?
> > 
> > I still think as this can be done in userspace, it probably should be.
> 
> Understood. Patches discarded.

Thinking more about this. How would your userspace library know on which
GPIOs your 7-seg segment device is connected? Should it parse the
Device Tree from userspace? Given by the user-space application who
would have to hardcode the GPIO numbers, completely defeating the
hardware abstraction layer that the kernel intends to be?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:

> > > Not having a kernel driver means that gazillions of applications
> > > re-invent the same piece of code over and over again, have to hardcode
> > > the GPIO numbers for a given piece of hardware, while the kernel
> > > abstract all of this very nicely.
> > 
> > That sounds like a wonderful use of a userspace library to do this
> > properly.  Much like libusb does, right?
> > 
> > I still think as this can be done in userspace, it probably should be.
> 
> Understood. Patches discarded.

Thinking more about this. How would your userspace library know on which
GPIOs your 7-seg segment device is connected? Should it parse the
Device Tree from userspace? Given by the user-space application who
would have to hardcode the GPIO numbers, completely defeating the
hardware abstraction layer that the kernel intends to be?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:

> > Not having a kernel driver means that gazillions of applications
> > re-invent the same piece of code over and over again, have to hardcode
> > the GPIO numbers for a given piece of hardware, while the kernel
> > abstract all of this very nicely.
> 
> That sounds like a wonderful use of a userspace library to do this
> properly.  Much like libusb does, right?
> 
> I still think as this can be done in userspace, it probably should be.

So that's why we have drivers/usb/misc/usbsevseg.c in the kernel, which
uses sysfs files to output text or numbers on a 7-segment display,
while it could all be done using libusb from userspace? Seems like back
in 2008 you Signed-off-by on the patch adding this driver :-)

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:

> > Not having a kernel driver means that gazillions of applications
> > re-invent the same piece of code over and over again, have to hardcode
> > the GPIO numbers for a given piece of hardware, while the kernel
> > abstract all of this very nicely.
> 
> That sounds like a wonderful use of a userspace library to do this
> properly.  Much like libusb does, right?
> 
> I still think as this can be done in userspace, it probably should be.

So that's why we have drivers/usb/misc/usbsevseg.c in the kernel, which
uses sysfs files to output text or numbers on a 7-segment display,
while it could all be done using libusb from userspace? Seems like back
in 2008 you Signed-off-by on the patch adding this driver :-)

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

RE: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[H Hartley Sweeten]

On Monday, January 07, 2013 10:44 AM, Thomas Petazzoni wrote:
> On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
>
>>>> Not having a kernel driver means that gazillions of applications
>>>> re-invent the same piece of code over and over again, have to hardcode
>>>> the GPIO numbers for a given piece of hardware, while the kernel
>>>> abstract all of this very nicely.
>>> 
>>> That sounds like a wonderful use of a userspace library to do this
>>> properly.  Much like libusb does, right?
>>> 
>>> I still think as this can be done in userspace, it probably should be.
>> 
>> Understood. Patches discarded.
>
> Thinking more about this. How would your userspace library know on which
> GPIOs your 7-seg segment device is connected? Should it parse the
> Device Tree from userspace? Given by the user-space application who
> would have to hardcode the GPIO numbers, completely defeating the
> hardware abstraction layer that the kernel intends to be?

Maybe an 'init' of 'config' function in the library should allow the user to
define the GPIOs that are connected to the 7-seg display? Or store the
GPIOs in some config file that the library parses when it's loaded.

This does require the user to know what GPIOs are connected and
forces then to do a bit of setup to use the display. But the actual use of
the display is still abstracted.

The library should also allow for multiple 7-seg displays. The old PC "post"
diagnostic boards usually had two displays on them.

Just my 2 cents....

Regards,
Hartley

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[H Hartley Sweeten]

On Monday, January 07, 2013 10:44 AM, Thomas Petazzoni wrote:
> On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
>
>>>> Not having a kernel driver means that gazillions of applications
>>>> re-invent the same piece of code over and over again, have to hardcode
>>>> the GPIO numbers for a given piece of hardware, while the kernel
>>>> abstract all of this very nicely.
>>> 
>>> That sounds like a wonderful use of a userspace library to do this
>>> properly.  Much like libusb does, right?
>>> 
>>> I still think as this can be done in userspace, it probably should be.
>> 
>> Understood. Patches discarded.
>
> Thinking more about this. How would your userspace library know on which
> GPIOs your 7-seg segment device is connected? Should it parse the
> Device Tree from userspace? Given by the user-space application who
> would have to hardcode the GPIO numbers, completely defeating the
> hardware abstraction layer that the kernel intends to be?

Maybe an 'init' of 'config' function in the library should allow the user to
define the GPIOs that are connected to the 7-seg display? Or store the
GPIOs in some config file that the library parses when it's loaded.

This does require the user to know what GPIOs are connected and
forces then to do a bit of setup to use the display. But the actual use of
the display is still abstracted.

The library should also allow for multiple 7-seg displays. The old PC "post"
diagnostic boards usually had two displays on them.

Just my 2 cents....

Regards,
Hartley

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Jason Cooper]

On Mon, Jan 07, 2013 at 11:56:19AM -0600, H Hartley Sweeten wrote:
> On Monday, January 07, 2013 10:44 AM, Thomas Petazzoni wrote:
> > On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
> >
> >>>> Not having a kernel driver means that gazillions of applications
> >>>> re-invent the same piece of code over and over again, have to hardcode
> >>>> the GPIO numbers for a given piece of hardware, while the kernel
> >>>> abstract all of this very nicely.
> >>> 
> >>> That sounds like a wonderful use of a userspace library to do this
> >>> properly.  Much like libusb does, right?
> >>> 
> >>> I still think as this can be done in userspace, it probably should be.
> >> 
> >> Understood. Patches discarded.
> >
> > Thinking more about this. How would your userspace library know on which
> > GPIOs your 7-seg segment device is connected? Should it parse the
> > Device Tree from userspace? Given by the user-space application who
> > would have to hardcode the GPIO numbers, completely defeating the
> > hardware abstraction layer that the kernel intends to be?
> 
> Maybe an 'init' of 'config' function in the library should allow the user to
> define the GPIOs that are connected to the 7-seg display? Or store the
> GPIOs in some config file that the library parses when it's loaded.
> 
> This does require the user to know what GPIOs are connected and
> forces then to do a bit of setup to use the display. But the actual use of
> the display is still abstracted.
> 
> The library should also allow for multiple 7-seg displays. The old PC "post"
> diagnostic boards usually had two displays on them.

However, Thomas has a valid point.  The DT is supposed to describe the
hardware.  The seven segment display is wired to three gpios on the
board.  Hence, it should be described in the DT for inferior OS's that
would handle this device in kernelspace.

Perhaps the library should be parsing /proc/devicetree ?

thx,

Jason.

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Jason Cooper]

On Mon, Jan 07, 2013 at 11:56:19AM -0600, H Hartley Sweeten wrote:
> On Monday, January 07, 2013 10:44 AM, Thomas Petazzoni wrote:
> > On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
> >
> >>>> Not having a kernel driver means that gazillions of applications
> >>>> re-invent the same piece of code over and over again, have to hardcode
> >>>> the GPIO numbers for a given piece of hardware, while the kernel
> >>>> abstract all of this very nicely.
> >>> 
> >>> That sounds like a wonderful use of a userspace library to do this
> >>> properly.  Much like libusb does, right?
> >>> 
> >>> I still think as this can be done in userspace, it probably should be.
> >> 
> >> Understood. Patches discarded.
> >
> > Thinking more about this. How would your userspace library know on which
> > GPIOs your 7-seg segment device is connected? Should it parse the
> > Device Tree from userspace? Given by the user-space application who
> > would have to hardcode the GPIO numbers, completely defeating the
> > hardware abstraction layer that the kernel intends to be?
> 
> Maybe an 'init' of 'config' function in the library should allow the user to
> define the GPIOs that are connected to the 7-seg display? Or store the
> GPIOs in some config file that the library parses when it's loaded.
> 
> This does require the user to know what GPIOs are connected and
> forces then to do a bit of setup to use the display. But the actual use of
> the display is still abstracted.
> 
> The library should also allow for multiple 7-seg displays. The old PC "post"
> diagnostic boards usually had two displays on them.

However, Thomas has a valid point.  The DT is supposed to describe the
hardware.  The seven segment display is wired to three gpios on the
board.  Hence, it should be described in the DT for inferior OS's that
would handle this device in kernelspace.

Perhaps the library should be parsing /proc/devicetree ?

thx,

Jason.

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Russell King - ARM Linux]

On Mon, Jan 07, 2013 at 06:43:52PM +0100, Thomas Petazzoni wrote:
> On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
> > > > Not having a kernel driver means that gazillions of applications
> > > > re-invent the same piece of code over and over again, have to hardcode
> > > > the GPIO numbers for a given piece of hardware, while the kernel
> > > > abstract all of this very nicely.
> > > 
> > > That sounds like a wonderful use of a userspace library to do this
> > > properly.  Much like libusb does, right?
> > > 
> > > I still think as this can be done in userspace, it probably should be.
> > 
> > Understood. Patches discarded.
> 
> Thinking more about this. How would your userspace library know on which
> GPIOs your 7-seg segment device is connected? Should it parse the
> Device Tree from userspace? Given by the user-space application who
> would have to hardcode the GPIO numbers, completely defeating the
> hardware abstraction layer that the kernel intends to be?

An application using the library gives it a list of GPIOs corresponding to
the segments.  If you think about it, the library could also know about a
group of 7-segs and output a multi-digit number in a sane manner.

There's also alphanumeric segmented displays too.  Do we want yet another
different kernel driver for that or should that also be handled in
userspace by the library?  What about a mixture of numeric and alphanumeric
displays?

Some radios I've seen have a mixture of these two types of display.  Others
have partial displays too - take for instance a clock display.  The hours
display only needs to display nothing or 1, or maybe nothing, 1, 2 (yes,
you can get displays with segments missing.)

The contary point to this is... what about 7-segment displays which are
scanned, where the cathodes of each display are commoned (so segment 'a'
is shared between each 7-seg display) and the anodes are sequenced?
Those are popular in certain consumer equipment (like radios) and I
suspect such a display wouldn't be possible from userspace.  _However_
again, there's nothing saying that it will always be a 7-seg display -
it could be again a mixture.

In the case of vacuum flourescent displays, these are very commonly
scanned affairs (gate scanning with commoned anodes for the segments)
in the manner I describe above, and included are segments for various
functions (eg, a separate segment for the weekday name.)  We've all
seen them on video recorders and such like over the years:
http://en.wikipedia.org/wiki/File:Vacuum_fluorescent_1.jpg

So really, there's a whole pile of display classes here that need to be
thought about before anyone goes creating a single interface for just
one specific display type.

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Russell King - ARM Linux]

On Mon, Jan 07, 2013 at 06:43:52PM +0100, Thomas Petazzoni wrote:
> On Mon, 7 Jan 2013 18:40:22 +0100, Thomas Petazzoni wrote:
> > > > Not having a kernel driver means that gazillions of applications
> > > > re-invent the same piece of code over and over again, have to hardcode
> > > > the GPIO numbers for a given piece of hardware, while the kernel
> > > > abstract all of this very nicely.
> > > 
> > > That sounds like a wonderful use of a userspace library to do this
> > > properly.  Much like libusb does, right?
> > > 
> > > I still think as this can be done in userspace, it probably should be.
> > 
> > Understood. Patches discarded.
> 
> Thinking more about this. How would your userspace library know on which
> GPIOs your 7-seg segment device is connected? Should it parse the
> Device Tree from userspace? Given by the user-space application who
> would have to hardcode the GPIO numbers, completely defeating the
> hardware abstraction layer that the kernel intends to be?

An application using the library gives it a list of GPIOs corresponding to
the segments.  If you think about it, the library could also know about a
group of 7-segs and output a multi-digit number in a sane manner.

There's also alphanumeric segmented displays too.  Do we want yet another
different kernel driver for that or should that also be handled in
userspace by the library?  What about a mixture of numeric and alphanumeric
displays?

Some radios I've seen have a mixture of these two types of display.  Others
have partial displays too - take for instance a clock display.  The hours
display only needs to display nothing or 1, or maybe nothing, 1, 2 (yes,
you can get displays with segments missing.)

The contary point to this is... what about 7-segment displays which are
scanned, where the cathodes of each display are commoned (so segment 'a'
is shared between each 7-seg display) and the anodes are sequenced?
Those are popular in certain consumer equipment (like radios) and I
suspect such a display wouldn't be possible from userspace.  _However_
again, there's nothing saying that it will always be a 7-seg display -
it could be again a mixture.

In the case of vacuum flourescent displays, these are very commonly
scanned affairs (gate scanning with commoned anodes for the segments)
in the manner I describe above, and included are segments for various
functions (eg, a separate segment for the weekday name.)  We've all
seen them on video recorders and such like over the years:
http://en.wikipedia.org/wiki/File:Vacuum_fluorescent_1.jpg

So really, there's a whole pile of display classes here that need to be
thought about before anyone goes creating a single interface for just
one specific display type.

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 06:53:57PM +0100, Thomas Petazzoni wrote:
> Dear Greg Kroah-Hartman,
> 
> On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:
> 
> > > Not having a kernel driver means that gazillions of applications
> > > re-invent the same piece of code over and over again, have to hardcode
> > > the GPIO numbers for a given piece of hardware, while the kernel
> > > abstract all of this very nicely.
> > 
> > That sounds like a wonderful use of a userspace library to do this
> > properly.  Much like libusb does, right?
> > 
> > I still think as this can be done in userspace, it probably should be.
> 
> So that's why we have drivers/usb/misc/usbsevseg.c in the kernel, which
> uses sysfs files to output text or numbers on a 7-segment display,
> while it could all be done using libusb from userspace? Seems like back
> in 2008 you Signed-off-by on the patch adding this driver :-)

I don't remember what I committed a month ago, let alone 4 years ago.
So trying to use that as an excuse as to why I should take your patch
isn't a really good idea, sorry :)

As Russell points out, if you want this in the kernel, it needs to have
a good userspace api, and that needs a lot more work than just a single
sysfs file.

thanks,

greg k-h

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Greg Kroah-Hartman]

On Mon, Jan 07, 2013 at 06:53:57PM +0100, Thomas Petazzoni wrote:
> Dear Greg Kroah-Hartman,
> 
> On Mon, 7 Jan 2013 09:23:40 -0800, Greg Kroah-Hartman wrote:
> 
> > > Not having a kernel driver means that gazillions of applications
> > > re-invent the same piece of code over and over again, have to hardcode
> > > the GPIO numbers for a given piece of hardware, while the kernel
> > > abstract all of this very nicely.
> > 
> > That sounds like a wonderful use of a userspace library to do this
> > properly.  Much like libusb does, right?
> > 
> > I still think as this can be done in userspace, it probably should be.
> 
> So that's why we have drivers/usb/misc/usbsevseg.c in the kernel, which
> uses sysfs files to output text or numbers on a 7-segment display,
> while it could all be done using libusb from userspace? Seems like back
> in 2008 you Signed-off-by on the patch adding this driver :-)

I don't remember what I committed a month ago, let alone 4 years ago.
So trying to use that as an excuse as to why I should take your patch
isn't a really good idea, sorry :)

As Russell points out, if you want this in the kernel, it needs to have
a good userspace api, and that needs a lot more work than just a single
sysfs file.

thanks,

greg k-h

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 10:36:42 -0800, Greg Kroah-Hartman wrote:

> I don't remember what I committed a month ago, let alone 4 years ago.
> So trying to use that as an excuse as to why I should take your patch
> isn't a really good idea, sorry :)

That was a shameless try from me, which I knew would not work :-)

> As Russell points out, if you want this in the kernel, it needs to have
> a good userspace api, and that needs a lot more work than just a single
> sysfs file.

Ok, thanks.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Thomas Petazzoni]

Dear Greg Kroah-Hartman,

On Mon, 7 Jan 2013 10:36:42 -0800, Greg Kroah-Hartman wrote:

> I don't remember what I committed a month ago, let alone 4 years ago.
> So trying to use that as an excuse as to why I should take your patch
> isn't a really good idea, sorry :)

That was a shameless try from me, which I knew would not work :-)

> As Russell points out, if you want this in the kernel, it needs to have
> a good userspace api, and that needs a lot more work than just a single
> sysfs file.

Ok, thanks.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Arnd Bergmann]

On Monday 07 January 2013, Thomas Petazzoni wrote:

>  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
>  drivers/misc/Kconfig                               |   13 ++
>  drivers/misc/Makefile                              |    1 +
>  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++

I wonder if it would make sense to merge this into the LED subsystem
rather than having it as a standalone driver.

> diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> new file mode 100644
> index 0000000..107d178
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> @@ -0,0 +1,18 @@
> +* 7-segment driver connected over GPIO through a BCD decoder
> +
> +Required properties:
> +- compatible: "generic,gpio-7seg"
> +- gpios: list of GPIOs to use to control the 7-segment display

Maybe list the minimum and maximum number of gpio lines here?

> +	sdev->dev_attr.attr.name = "value";
> +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> +	sdev->dev_attr.show = gpio_7seg_show;
> +	sdev->dev_attr.store = gpio_7seg_store;

Any reason why you are not using the DEVICE_ATTR macro?

	Arnd

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Arnd Bergmann]

On Monday 07 January 2013, Thomas Petazzoni wrote:

>  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
>  drivers/misc/Kconfig                               |   13 ++
>  drivers/misc/Makefile                              |    1 +
>  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++

I wonder if it would make sense to merge this into the LED subsystem
rather than having it as a standalone driver.

> diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> new file mode 100644
> index 0000000..107d178
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> @@ -0,0 +1,18 @@
> +* 7-segment driver connected over GPIO through a BCD decoder
> +
> +Required properties:
> +- compatible: "generic,gpio-7seg"
> +- gpios: list of GPIOs to use to control the 7-segment display

Maybe list the minimum and maximum number of gpio lines here?

> +	sdev->dev_attr.attr.name = "value";
> +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> +	sdev->dev_attr.show = gpio_7seg_show;
> +	sdev->dev_attr.store = gpio_7seg_store;

Any reason why you are not using the DEVICE_ATTR macro?

	Arnd

Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:

> >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> >  drivers/misc/Kconfig                               |   13 ++
> >  drivers/misc/Makefile                              |    1 +
> >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> 
> I wonder if it would make sense to merge this into the LED subsystem
> rather than having it as a standalone driver.

Hum, maybe. How do you see it fitting inside the LED subsystem? The
purpose of the device is quite different in its userspace to kernel
interface, no?

> > diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> > new file mode 100644
> > index 0000000..107d178
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> > @@ -0,0 +1,18 @@
> > +* 7-segment driver connected over GPIO through a BCD decoder
> > +
> > +Required properties:
> > +- compatible: "generic,gpio-7seg"
> > +- gpios: list of GPIOs to use to control the 7-segment display
> 
> Maybe list the minimum and maximum number of gpio lines here?

Indeed. Seems like it should be anywhere between 0 and 32 GPIOs.

> > +	sdev->dev_attr.attr.name = "value";
> > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > +	sdev->dev_attr.show = gpio_7seg_show;
> > +	sdev->dev_attr.store = gpio_7seg_store;
> 
> Any reason why you are not using the DEVICE_ATTR macro?

DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
device, which is dynamically allocated, so I thought that using
DEVICE_ATTR was not possible. But now that you point this, I realize
that I was stupid. I can perfectly live with one single global 'struct
device_attribute' that I register to several devices using
device_create_file(), no?

That said, is it worth the effort to continue polishing this driver,
after seeing the feedback from Greg about it? I don't mind working on
it more and fix the problems of course, but if the general answer is
that we don't want such a driver in the kernel, then I'd better not
spend more time on this.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:

> >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> >  drivers/misc/Kconfig                               |   13 ++
> >  drivers/misc/Makefile                              |    1 +
> >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> 
> I wonder if it would make sense to merge this into the LED subsystem
> rather than having it as a standalone driver.

Hum, maybe. How do you see it fitting inside the LED subsystem? The
purpose of the device is quite different in its userspace to kernel
interface, no?

> > diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> > new file mode 100644
> > index 0000000..107d178
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> > @@ -0,0 +1,18 @@
> > +* 7-segment driver connected over GPIO through a BCD decoder
> > +
> > +Required properties:
> > +- compatible: "generic,gpio-7seg"
> > +- gpios: list of GPIOs to use to control the 7-segment display
> 
> Maybe list the minimum and maximum number of gpio lines here?

Indeed. Seems like it should be anywhere between 0 and 32 GPIOs.

> > +	sdev->dev_attr.attr.name = "value";
> > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > +	sdev->dev_attr.show = gpio_7seg_show;
> > +	sdev->dev_attr.store = gpio_7seg_store;
> 
> Any reason why you are not using the DEVICE_ATTR macro?

DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
device, which is dynamically allocated, so I thought that using
DEVICE_ATTR was not possible. But now that you point this, I realize
that I was stupid. I can perfectly live with one single global 'struct
device_attribute' that I register to several devices using
device_create_file(), no?

That said, is it worth the effort to continue polishing this driver,
after seeing the feedback from Greg about it? I don't mind working on
it more and fix the problems of course, but if the general answer is
that we don't want such a driver in the kernel, then I'd better not
spend more time on this.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Arnd Bergmann]

On Monday 07 January 2013, Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
> 
> On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
> 
> > >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> > >  drivers/misc/Kconfig                               |   13 ++
> > >  drivers/misc/Makefile                              |    1 +
> > >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> > 
> > I wonder if it would make sense to merge this into the LED subsystem
> > rather than having it as a standalone driver.
> 
> Hum, maybe. How do you see it fitting inside the LED subsystem? The
> purpose of the device is quite different in its userspace to kernel
> interface, no?

The LED subsystem can be used by both in-kernel and by user space
front-ends. It currently supports on/off, blinking and variable
brightness LEDs. Adding 7-segment LEDs may be a stretch but also
would fit into the same basic interface I think.

In addition, it would not have to be GPIO based, although that may
be the only implementation we need for the foreseeable future.
Well, unless you make a driver for PC-style "port-0x80" PCI
cards.


> > > +	sdev->dev_attr.attr.name = "value";
> > > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > +	sdev->dev_attr.show = gpio_7seg_show;
> > > +	sdev->dev_attr.store = gpio_7seg_store;
> > 
> > Any reason why you are not using the DEVICE_ATTR macro?
> 
> DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> device, which is dynamically allocated, so I thought that using
> DEVICE_ATTR was not possible. But now that you point this, I realize
> that I was stupid. I can perfectly live with one single global 'struct
> device_attribute' that I register to several devices using
> device_create_file(), no?

Correct.

> That said, is it worth the effort to continue polishing this driver,
> after seeing the feedback from Greg about it? I don't mind working on
> it more and fix the problems of course, but if the general answer is
> that we don't want such a driver in the kernel, then I'd better not
> spend more time on this.

I only saw the discussion after I had sent my reply. I think that
this driver in the form of a drivers/misc sysfs driver is not worth
pursuing, but it may still be possible to integrate it nicely into
the LED framework.

	Arnd

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Arnd Bergmann]

On Monday 07 January 2013, Thomas Petazzoni wrote:
> Dear Arnd Bergmann,
> 
> On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
> 
> > >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> > >  drivers/misc/Kconfig                               |   13 ++
> > >  drivers/misc/Makefile                              |    1 +
> > >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> > 
> > I wonder if it would make sense to merge this into the LED subsystem
> > rather than having it as a standalone driver.
> 
> Hum, maybe. How do you see it fitting inside the LED subsystem? The
> purpose of the device is quite different in its userspace to kernel
> interface, no?

The LED subsystem can be used by both in-kernel and by user space
front-ends. It currently supports on/off, blinking and variable
brightness LEDs. Adding 7-segment LEDs may be a stretch but also
would fit into the same basic interface I think.

In addition, it would not have to be GPIO based, although that may
be the only implementation we need for the foreseeable future.
Well, unless you make a driver for PC-style "port-0x80" PCI
cards.


> > > +	sdev->dev_attr.attr.name = "value";
> > > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > +	sdev->dev_attr.show = gpio_7seg_show;
> > > +	sdev->dev_attr.store = gpio_7seg_store;
> > 
> > Any reason why you are not using the DEVICE_ATTR macro?
> 
> DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> device, which is dynamically allocated, so I thought that using
> DEVICE_ATTR was not possible. But now that you point this, I realize
> that I was stupid. I can perfectly live with one single global 'struct
> device_attribute' that I register to several devices using
> device_create_file(), no?

Correct.

> That said, is it worth the effort to continue polishing this driver,
> after seeing the feedback from Greg about it? I don't mind working on
> it more and fix the problems of course, but if the general answer is
> that we don't want such a driver in the kernel, then I'd better not
> spend more time on this.

I only saw the discussion after I had sent my reply. I think that
this driver in the form of a drivers/misc sysfs driver is not worth
pursuing, but it may still be possible to integrate it nicely into
the LED framework.

	Arnd

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Linus Walleij]

On Mon, Jan 7, 2013 at 5:42 PM, Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> This patch series implements a simple driver for 7-segment displays
> connected over GPIOs through a BCD encoder. The userspace interface of
> the driver is very simple: just a "value" file in the sysfs directory
> of the device, which can be written to (changes the value shown on the
> 7-segment display) and read (gives the current value).

The ARM Integrator AP and CP base boards I have here have
7-segment displays as well. But it's not on GPIO but dedicated
registers, so I could implement these as, e.g. LEDs.

But with this framework I'd just make a driver that pokes directly
into the registers.

However as Russell points out, this needs to be thought through.
I have a period on my display between the two characters, so
how do we model that period?

Yours,
Linus Walleij

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Linus Walleij]

On Mon, Jan 7, 2013 at 5:42 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> This patch series implements a simple driver for 7-segment displays
> connected over GPIOs through a BCD encoder. The userspace interface of
> the driver is very simple: just a "value" file in the sysfs directory
> of the device, which can be written to (changes the value shown on the
> 7-segment display) and read (gives the current value).

The ARM Integrator AP and CP base boards I have here have
7-segment displays as well. But it's not on GPIO but dedicated
registers, so I could implement these as, e.g. LEDs.

But with this framework I'd just make a driver that pokes directly
into the registers.

However as Russell points out, this needs to be thought through.
I have a period on my display between the two characters, so
how do we model that period?

Yours,
Linus Walleij

Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Grant Likely]

On Mon,  7 Jan 2013 17:42:06 +0100, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> This patch adds a driver to control 7-segment displays connected over
> GPIOs through a BCD encoder. Typically, up to four GPIOs go into a BCD
> encoder, that drives the 7-segment display to display a numeric value
> between 0 and 2^ngpios-1.
> 
> Once this driver is enabled, it creates a 'value' field in the sysfs
> directory of the device, which allows to set the current value
> displayed by the 7-segment.
> 
> It has been successfully tested on the Marvell Armada 370 and Marvell
> Armada XP evaluation boards. Both of them have a 7-segment display
> that can be controlled using 3 GPIOs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
>  drivers/misc/Kconfig                               |   13 ++
>  drivers/misc/Makefile                              |    1 +
>  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
>  4 files changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/gpio-7seg.txt
>  create mode 100644 drivers/misc/gpio-7seg.c
> 
> diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> new file mode 100644
> index 0000000..107d178
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> @@ -0,0 +1,18 @@
> +* 7-segment driver connected over GPIO through a BCD decoder
> +
> +Required properties:
> +- compatible: "generic,gpio-7seg"
> +- gpios: list of GPIOs to use to control the 7-segment display
> +
> +Optional properties:
> +- default-value: default value shown by the 7-segment display at boot
> +  time. If not defined, defaults to 0.
> +
> +Example:
> +
> +gpio-7seg@0 {
> +	compatible = "generic,gpio-7seg";
> +	status = "okay";
> +	gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
> +	default-value = <5>;
> +};

Nice, I would use this. As an extension, I could also see wanting to
support directly attached LED displays; one GPIO per segment (but I'm
not suggesting you do that; I'm just thinking out loud).

However, rather than a completely seperate driver, can you piggyback on
the existing gpio-leds driver? It would The binding document should also be in
bindings/leds, and the driver in drivers/leds.

g.

> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index b151b7c..19f9d22 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -93,6 +93,19 @@ config ATMEL_TCB_CLKSRC_BLOCK
>  	  TC can be used for other purposes, such as PWM generation and
>  	  interval timing.
>  
> +config GPIO_7SEG
> +	tristate "GPIO-connected 7-segment display driver"
> +	help
> +	  This option enables a driver to control 7-segment displays
> +	  connected over GPIOs through a BCD encoder. Typically, up to
> +	  four GPIOs go into a BCD encoder, that drives the 7-segment
> +	  display to display a numeric value between 0 and
> +	  2^ngpios-1.
> +
> +	  Once this driver is enabled, it creates a 'value' field in
> +	  the sysfs directory of the device, which allows to set the
> +	  current value displayed by the 7-segment.
> +
>  config IBM_ASM
>  	tristate "Device driver for IBM RSA service processor"
>  	depends on X86 && PCI && INPUT
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 2129377..b789358 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_INTEL_MID_PTI)	+= pti.o
>  obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
>  obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
>  obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
> +obj-$(CONFIG_GPIO_7SEG)         += gpio-7seg.o
>  obj-$(CONFIG_BMP085)		+= bmp085.o
>  obj-$(CONFIG_BMP085_I2C)	+= bmp085-i2c.o
>  obj-$(CONFIG_BMP085_SPI)	+= bmp085-spi.o
> diff --git a/drivers/misc/gpio-7seg.c b/drivers/misc/gpio-7seg.c
> new file mode 100644
> index 0000000..8cc5113
> --- /dev/null
> +++ b/drivers/misc/gpio-7seg.c
> @@ -0,0 +1,168 @@
> +/*
> + * Simple driver to control a 7 segment display connected through a
> + * BCD decoder using GPIOs.
> + *
> + * Copyright (C) 2012 Marvell
> + *
> + * Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +
> +struct gpio_7seg_dev {
> +	struct device_attribute dev_attr;
> +	int *gpios;
> +	int ngpios;
> +	int value;
> +	int maxvalue;
> +};
> +
> +static void gpio_7seg_display_val(struct gpio_7seg_dev *sdev,
> +				  int value)
> +{
> +	int i;
> +
> +	for (i = 0; i < sdev->ngpios; i++) {
> +		int bitval = (value & BIT(i)) ? 1 : 0;
> +		gpio_set_value_cansleep(sdev->gpios[i], bitval);
> +	}
> +}
> +
> +static ssize_t gpio_7seg_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
> +	return snprintf(buf, PAGE_SIZE, "%d\n", sdev->value);
> +}
> +
> +static ssize_t gpio_7seg_store(struct device *dev, struct device_attribute *attr,
> +			       const char *buf, size_t count)
> +{
> +	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
> +	unsigned long value;
> +	char *end;
> +
> +	value = simple_strtoul(buf, &end, 0);
> +	if (end == buf)
> +		return -EINVAL;
> +
> +	if (value >= sdev->maxvalue)
> +		return -EINVAL;
> +
> +	gpio_7seg_display_val(sdev, value);
> +	sdev->value = value;
> +
> +	return count;
> +}
> +
> +static int gpio_7seg_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct gpio_7seg_dev *sdev;
> +	int ret, i;
> +
> +	/*
> +	 * We only support being probed through the device tree, for
> +	 * now
> +	 */
> +	if (!np)
> +		return -EINVAL;
> +
> +	sdev = devm_kzalloc(&pdev->dev, sizeof(struct gpio_7seg_dev),
> +			    GFP_KERNEL);
> +	if (!sdev)
> +		return -ENOMEM;
> +
> +	sdev->ngpios = of_gpio_count(np);
> +	if (!sdev->ngpios)
> +		return -EINVAL;
> +
> +	sdev->gpios = devm_kzalloc(&pdev->dev, sizeof(int) * sdev->ngpios,
> +				   GFP_KERNEL);
> +	if (!sdev->gpios)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < sdev->ngpios; i++) {
> +		sdev->gpios[i] = of_get_gpio(np, i);
> +		ret = gpio_request_one(sdev->gpios[i], GPIOF_DIR_OUT, NULL);
> +		if (ret) {
> +			/*
> +			 * Mark this GPIO as non-requested for the
> +			 * error handling code
> +			 */
> +			sdev->gpios[i] = ret;
> +			goto gpio_cleanup;
> +		}
> +	}
> +
> +	ret = of_property_read_u32(np, "default-value", &sdev->value);
> +	if (ret)
> +		sdev->value = 0;
> +
> +	sdev->maxvalue = 1 << sdev->ngpios;
> +
> +	sdev->dev_attr.attr.name = "value";
> +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> +	sdev->dev_attr.show = gpio_7seg_show;
> +	sdev->dev_attr.store = gpio_7seg_store;
> +
> +	ret = device_create_file(&pdev->dev, &sdev->dev_attr);
> +	if (ret)
> +		goto gpio_cleanup;
> +
> +	gpio_7seg_display_val(sdev, sdev->value);
> +
> +	platform_set_drvdata(pdev, sdev);
> +	return 0;
> +
> +gpio_cleanup:
> +	for (i = 0; i < sdev->ngpios; i++)
> +		if (gpio_is_valid(sdev->gpios[i]))
> +			gpio_free(sdev->gpios[i]);
> +	return ret;
> +}
> +
> +static int gpio_7seg_remove(struct platform_device *pdev)
> +{
> +	struct gpio_7seg_dev *sdev;
> +	int i;
> +
> +	sdev = platform_get_drvdata(pdev);
> +
> +	device_remove_file(&pdev->dev, &sdev->dev_attr);
> +
> +	for (i = 0; i < sdev->ngpios; i++)
> +		if (gpio_is_valid(sdev->gpios[i]))
> +			gpio_free(sdev->gpios[i]);
> +	return 0;
> +}
> +
> +static const struct of_device_id gpio_7seg_of_match_table[] = {
> +	{ .compatible = "generic,gpio-7seg" },
> +	{},
> +};
> +
> +static struct platform_driver gpio_7seg_driver = {
> +	.probe  = gpio_7seg_probe,
> +	.remove = gpio_7seg_remove,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = "gpio-7seg",
> +		.of_match_table = of_match_ptr(gpio_7seg_of_match_table),
> +	},
> +};
> +
> +module_platform_driver(gpio_7seg_driver);
> +
> +MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> +MODULE_DESCRIPTION("Simple GPIO-connected 7-segment display driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Grant Likely]

On Mon,  7 Jan 2013 17:42:06 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> This patch adds a driver to control 7-segment displays connected over
> GPIOs through a BCD encoder. Typically, up to four GPIOs go into a BCD
> encoder, that drives the 7-segment display to display a numeric value
> between 0 and 2^ngpios-1.
> 
> Once this driver is enabled, it creates a 'value' field in the sysfs
> directory of the device, which allows to set the current value
> displayed by the 7-segment.
> 
> It has been successfully tested on the Marvell Armada 370 and Marvell
> Armada XP evaluation boards. Both of them have a 7-segment display
> that can be controlled using 3 GPIOs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
>  drivers/misc/Kconfig                               |   13 ++
>  drivers/misc/Makefile                              |    1 +
>  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
>  4 files changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/gpio-7seg.txt
>  create mode 100644 drivers/misc/gpio-7seg.c
> 
> diff --git a/Documentation/devicetree/bindings/misc/gpio-7seg.txt b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> new file mode 100644
> index 0000000..107d178
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/gpio-7seg.txt
> @@ -0,0 +1,18 @@
> +* 7-segment driver connected over GPIO through a BCD decoder
> +
> +Required properties:
> +- compatible: "generic,gpio-7seg"
> +- gpios: list of GPIOs to use to control the 7-segment display
> +
> +Optional properties:
> +- default-value: default value shown by the 7-segment display at boot
> +  time. If not defined, defaults to 0.
> +
> +Example:
> +
> +gpio-7seg at 0 {
> +	compatible = "generic,gpio-7seg";
> +	status = "okay";
> +	gpios = <&gpio1 27 0 &gpio1 28 0 &gpio1 29 0>;
> +	default-value = <5>;
> +};

Nice, I would use this. As an extension, I could also see wanting to
support directly attached LED displays; one GPIO per segment (but I'm
not suggesting you do that; I'm just thinking out loud).

However, rather than a completely seperate driver, can you piggyback on
the existing gpio-leds driver? It would The binding document should also be in
bindings/leds, and the driver in drivers/leds.

g.

> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index b151b7c..19f9d22 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -93,6 +93,19 @@ config ATMEL_TCB_CLKSRC_BLOCK
>  	  TC can be used for other purposes, such as PWM generation and
>  	  interval timing.
>  
> +config GPIO_7SEG
> +	tristate "GPIO-connected 7-segment display driver"
> +	help
> +	  This option enables a driver to control 7-segment displays
> +	  connected over GPIOs through a BCD encoder. Typically, up to
> +	  four GPIOs go into a BCD encoder, that drives the 7-segment
> +	  display to display a numeric value between 0 and
> +	  2^ngpios-1.
> +
> +	  Once this driver is enabled, it creates a 'value' field in
> +	  the sysfs directory of the device, which allows to set the
> +	  current value displayed by the 7-segment.
> +
>  config IBM_ASM
>  	tristate "Device driver for IBM RSA service processor"
>  	depends on X86 && PCI && INPUT
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 2129377..b789358 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_INTEL_MID_PTI)	+= pti.o
>  obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
>  obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
>  obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
> +obj-$(CONFIG_GPIO_7SEG)         += gpio-7seg.o
>  obj-$(CONFIG_BMP085)		+= bmp085.o
>  obj-$(CONFIG_BMP085_I2C)	+= bmp085-i2c.o
>  obj-$(CONFIG_BMP085_SPI)	+= bmp085-spi.o
> diff --git a/drivers/misc/gpio-7seg.c b/drivers/misc/gpio-7seg.c
> new file mode 100644
> index 0000000..8cc5113
> --- /dev/null
> +++ b/drivers/misc/gpio-7seg.c
> @@ -0,0 +1,168 @@
> +/*
> + * Simple driver to control a 7 segment display connected through a
> + * BCD decoder using GPIOs.
> + *
> + * Copyright (C) 2012 Marvell
> + *
> + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +
> +struct gpio_7seg_dev {
> +	struct device_attribute dev_attr;
> +	int *gpios;
> +	int ngpios;
> +	int value;
> +	int maxvalue;
> +};
> +
> +static void gpio_7seg_display_val(struct gpio_7seg_dev *sdev,
> +				  int value)
> +{
> +	int i;
> +
> +	for (i = 0; i < sdev->ngpios; i++) {
> +		int bitval = (value & BIT(i)) ? 1 : 0;
> +		gpio_set_value_cansleep(sdev->gpios[i], bitval);
> +	}
> +}
> +
> +static ssize_t gpio_7seg_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
> +	return snprintf(buf, PAGE_SIZE, "%d\n", sdev->value);
> +}
> +
> +static ssize_t gpio_7seg_store(struct device *dev, struct device_attribute *attr,
> +			       const char *buf, size_t count)
> +{
> +	struct gpio_7seg_dev *sdev = dev_get_drvdata(dev);
> +	unsigned long value;
> +	char *end;
> +
> +	value = simple_strtoul(buf, &end, 0);
> +	if (end == buf)
> +		return -EINVAL;
> +
> +	if (value >= sdev->maxvalue)
> +		return -EINVAL;
> +
> +	gpio_7seg_display_val(sdev, value);
> +	sdev->value = value;
> +
> +	return count;
> +}
> +
> +static int gpio_7seg_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct gpio_7seg_dev *sdev;
> +	int ret, i;
> +
> +	/*
> +	 * We only support being probed through the device tree, for
> +	 * now
> +	 */
> +	if (!np)
> +		return -EINVAL;
> +
> +	sdev = devm_kzalloc(&pdev->dev, sizeof(struct gpio_7seg_dev),
> +			    GFP_KERNEL);
> +	if (!sdev)
> +		return -ENOMEM;
> +
> +	sdev->ngpios = of_gpio_count(np);
> +	if (!sdev->ngpios)
> +		return -EINVAL;
> +
> +	sdev->gpios = devm_kzalloc(&pdev->dev, sizeof(int) * sdev->ngpios,
> +				   GFP_KERNEL);
> +	if (!sdev->gpios)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < sdev->ngpios; i++) {
> +		sdev->gpios[i] = of_get_gpio(np, i);
> +		ret = gpio_request_one(sdev->gpios[i], GPIOF_DIR_OUT, NULL);
> +		if (ret) {
> +			/*
> +			 * Mark this GPIO as non-requested for the
> +			 * error handling code
> +			 */
> +			sdev->gpios[i] = ret;
> +			goto gpio_cleanup;
> +		}
> +	}
> +
> +	ret = of_property_read_u32(np, "default-value", &sdev->value);
> +	if (ret)
> +		sdev->value = 0;
> +
> +	sdev->maxvalue = 1 << sdev->ngpios;
> +
> +	sdev->dev_attr.attr.name = "value";
> +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> +	sdev->dev_attr.show = gpio_7seg_show;
> +	sdev->dev_attr.store = gpio_7seg_store;
> +
> +	ret = device_create_file(&pdev->dev, &sdev->dev_attr);
> +	if (ret)
> +		goto gpio_cleanup;
> +
> +	gpio_7seg_display_val(sdev, sdev->value);
> +
> +	platform_set_drvdata(pdev, sdev);
> +	return 0;
> +
> +gpio_cleanup:
> +	for (i = 0; i < sdev->ngpios; i++)
> +		if (gpio_is_valid(sdev->gpios[i]))
> +			gpio_free(sdev->gpios[i]);
> +	return ret;
> +}
> +
> +static int gpio_7seg_remove(struct platform_device *pdev)
> +{
> +	struct gpio_7seg_dev *sdev;
> +	int i;
> +
> +	sdev = platform_get_drvdata(pdev);
> +
> +	device_remove_file(&pdev->dev, &sdev->dev_attr);
> +
> +	for (i = 0; i < sdev->ngpios; i++)
> +		if (gpio_is_valid(sdev->gpios[i]))
> +			gpio_free(sdev->gpios[i]);
> +	return 0;
> +}
> +
> +static const struct of_device_id gpio_7seg_of_match_table[] = {
> +	{ .compatible = "generic,gpio-7seg" },
> +	{},
> +};
> +
> +static struct platform_driver gpio_7seg_driver = {
> +	.probe  = gpio_7seg_probe,
> +	.remove = gpio_7seg_remove,
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = "gpio-7seg",
> +		.of_match_table = of_match_ptr(gpio_7seg_of_match_table),
> +	},
> +};
> +
> +module_platform_driver(gpio_7seg_driver);
> +
> +MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com>");
> +MODULE_DESCRIPTION("Simple GPIO-connected 7-segment display driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Grant Likely]

On Mon, 7 Jan 2013 21:41:48 +0000, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Monday 07 January 2013, Thomas Petazzoni wrote:
> > Dear Arnd Bergmann,
> > 
> > On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
> > 
> > > >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> > > >  drivers/misc/Kconfig                               |   13 ++
> > > >  drivers/misc/Makefile                              |    1 +
> > > >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> > > 
> > > I wonder if it would make sense to merge this into the LED subsystem
> > > rather than having it as a standalone driver.
> > 
> > Hum, maybe. How do you see it fitting inside the LED subsystem? The
> > purpose of the device is quite different in its userspace to kernel
> > interface, no?
> 
> The LED subsystem can be used by both in-kernel and by user space
> front-ends. It currently supports on/off, blinking and variable
> brightness LEDs. Adding 7-segment LEDs may be a stretch but also
> would fit into the same basic interface I think.
> 
> In addition, it would not have to be GPIO based, although that may
> be the only implementation we need for the foreseeable future.
> Well, unless you make a driver for PC-style "port-0x80" PCI
> cards.
> 
> 
> > > > +	sdev->dev_attr.attr.name = "value";
> > > > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > > +	sdev->dev_attr.show = gpio_7seg_show;
> > > > +	sdev->dev_attr.store = gpio_7seg_store;
> > > 
> > > Any reason why you are not using the DEVICE_ATTR macro?
> > 
> > DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> > device, which is dynamically allocated, so I thought that using
> > DEVICE_ATTR was not possible. But now that you point this, I realize
> > that I was stupid. I can perfectly live with one single global 'struct
> > device_attribute' that I register to several devices using
> > device_create_file(), no?
> 
> Correct.
> 
> > That said, is it worth the effort to continue polishing this driver,
> > after seeing the feedback from Greg about it? I don't mind working on
> > it more and fix the problems of course, but if the general answer is
> > that we don't want such a driver in the kernel, then I'd better not
> > spend more time on this.
> 
> I only saw the discussion after I had sent my reply. I think that
> this driver in the form of a drivers/misc sysfs driver is not worth
> pursuing, but it may still be possible to integrate it nicely into
> the LED framework.

I also definitely think it should be a kernel driver. This is the sort
of low level device useful for debugging that I want available to other
kernel modules.

g.

[PATCH 3.9 1/3] misc: new driver for GPIO-connected 7-segment displays

[Grant Likely]

On Mon, 7 Jan 2013 21:41:48 +0000, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 07 January 2013, Thomas Petazzoni wrote:
> > Dear Arnd Bergmann,
> > 
> > On Mon, 7 Jan 2013 19:43:39 +0000, Arnd Bergmann wrote:
> > 
> > > >  .../devicetree/bindings/misc/gpio-7seg.txt         |   18 +++
> > > >  drivers/misc/Kconfig                               |   13 ++
> > > >  drivers/misc/Makefile                              |    1 +
> > > >  drivers/misc/gpio-7seg.c                           |  168 ++++++++++++++++++++
> > > 
> > > I wonder if it would make sense to merge this into the LED subsystem
> > > rather than having it as a standalone driver.
> > 
> > Hum, maybe. How do you see it fitting inside the LED subsystem? The
> > purpose of the device is quite different in its userspace to kernel
> > interface, no?
> 
> The LED subsystem can be used by both in-kernel and by user space
> front-ends. It currently supports on/off, blinking and variable
> brightness LEDs. Adding 7-segment LEDs may be a stretch but also
> would fit into the same basic interface I think.
> 
> In addition, it would not have to be GPIO based, although that may
> be the only implementation we need for the foreseeable future.
> Well, unless you make a driver for PC-style "port-0x80" PCI
> cards.
> 
> 
> > > > +	sdev->dev_attr.attr.name = "value";
> > > > +	sdev->dev_attr.attr.mode = S_IRUGO | S_IWUGO;
> > > > +	sdev->dev_attr.show = gpio_7seg_show;
> > > > +	sdev->dev_attr.store = gpio_7seg_store;
> > > 
> > > Any reason why you are not using the DEVICE_ATTR macro?
> > 
> > DEVICE_ATTR declares the structure, and I wanted one per gpio_7seg
> > device, which is dynamically allocated, so I thought that using
> > DEVICE_ATTR was not possible. But now that you point this, I realize
> > that I was stupid. I can perfectly live with one single global 'struct
> > device_attribute' that I register to several devices using
> > device_create_file(), no?
> 
> Correct.
> 
> > That said, is it worth the effort to continue polishing this driver,
> > after seeing the feedback from Greg about it? I don't mind working on
> > it more and fix the problems of course, but if the general answer is
> > that we don't want such a driver in the kernel, then I'd better not
> > spend more time on this.
> 
> I only saw the discussion after I had sent my reply. I think that
> this driver in the form of a drivers/misc sysfs driver is not worth
> pursuing, but it may still be possible to integrate it nicely into
> the LED framework.

I also definitely think it should be a kernel driver. This is the sort
of low level device useful for debugging that I want available to other
kernel modules.

g.

Re: [PATCH 3.9 3/3] arm: mvebu: add the 7-segment display DT informations to Armada XP DB

[Grant Likely]

On Mon,  7 Jan 2013 17:42:08 +0100, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Now that we have a 7-segment display driver in the kernel, use it on
> the Marvell Armada XP DB evaluation board.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/boot/dts/armada-xp-db.dts |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
> index 8e53b25..8654c1b 100644
> --- a/arch/arm/boot/dts/armada-xp-db.dts
> +++ b/arch/arm/boot/dts/armada-xp-db.dts
> @@ -90,5 +90,10 @@
>  			phy = <&phy3>;
>  			phy-mode = "sgmii";
>  		};
> +		gpio-7seg@0 {
> +			compatible = "generic,gpio-7seg";
> +			status = "okay";
> +			gpios = <&gpio0 26 0 &gpio0 27 0 &gpio1 16 0>;
> +		};

Nit: The node probably belongs at the base of the tree. A direct child
of the root node because it really doen't make sense as a child of
anything else. It's more of a system level device.

g.

[PATCH 3.9 3/3] arm: mvebu: add the 7-segment display DT informations to Armada XP DB

[Grant Likely]

On Mon,  7 Jan 2013 17:42:08 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Now that we have a 7-segment display driver in the kernel, use it on
> the Marvell Armada XP DB evaluation board.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-xp-db.dts |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
> index 8e53b25..8654c1b 100644
> --- a/arch/arm/boot/dts/armada-xp-db.dts
> +++ b/arch/arm/boot/dts/armada-xp-db.dts
> @@ -90,5 +90,10 @@
>  			phy = <&phy3>;
>  			phy-mode = "sgmii";
>  		};
> +		gpio-7seg at 0 {
> +			compatible = "generic,gpio-7seg";
> +			status = "okay";
> +			gpios = <&gpio0 26 0 &gpio0 27 0 &gpio1 16 0>;
> +		};

Nit: The node probably belongs at the base of the tree. A direct child
of the root node because it really doen't make sense as a child of
anything else. It's more of a system level device.

g.

Re: [PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Pavel Machek]

Hi!

> <thomas.petazzoni@free-electrons.com> wrote:
> 
> > This patch series implements a simple driver for 7-segment displays
> > connected over GPIOs through a BCD encoder. The userspace interface of
> > the driver is very simple: just a "value" file in the sysfs directory
> > of the device, which can be written to (changes the value shown on the
> > 7-segment display) and read (gives the current value).
> 
> The ARM Integrator AP and CP base boards I have here have
> 7-segment displays as well. But it's not on GPIO but dedicated
> registers, so I could implement these as, e.g. LEDs.
> 
> But with this framework I'd just make a driver that pokes directly
> into the registers.
> 
> However as Russell points out, this needs to be thought through.
> I have a period on my display between the two characters, so
> how do we model that period?

Well, 7 segment display can be used for 0-9 A-F (very readable) and
0-9 a-z if you stretch stuff a bit, as done on TI chronos for
example... Would it make sense to simply display a string? Up to four
letters with up-to 4 '.' space characters...

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 3.9] Driver for 7-segment displays connected over GPIOs

[Pavel Machek]

Hi!

> <thomas.petazzoni@free-electrons.com> wrote:
> 
> > This patch series implements a simple driver for 7-segment displays
> > connected over GPIOs through a BCD encoder. The userspace interface of
> > the driver is very simple: just a "value" file in the sysfs directory
> > of the device, which can be written to (changes the value shown on the
> > 7-segment display) and read (gives the current value).
> 
> The ARM Integrator AP and CP base boards I have here have
> 7-segment displays as well. But it's not on GPIO but dedicated
> registers, so I could implement these as, e.g. LEDs.
> 
> But with this framework I'd just make a driver that pokes directly
> into the registers.
> 
> However as Russell points out, this needs to be thought through.
> I have a period on my display between the two characters, so
> how do we model that period?

Well, 7 segment display can be used for 0-9 A-F (very readable) and
0-9 a-z if you stretch stuff a bit, as done on TI chronos for
example... Would it make sense to simply display a string? Up to four
letters with up-to 4 '.' space characters...

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html