[PATCH v6 0/3] auxdisplay: 7-segment LED display

[PATCH v6 0/3] auxdisplay: 7-segment LED display

[Chris Packham]

This series adds a driver for a 7-segment LED display.

I think I've addressed all of Andy's feedback in this round. I haven't
heard from the ARM maintainers on any of the previous rounds. I can
submit the dts change separately if required, I've mostly been including
it so there is an in-tree user of the driver I'm adding.

Chris Packham (3):
  auxdisplay: Add 7-segment LED display driver
  dt-bindings: auxdisplay: Add bindings for generic 7-segment LED
  ARM: dts: marvell: Add 7-segment LED display on x530

 .../bindings/auxdisplay/gpio-7-segment.yaml   |  55 +++++++++
 .../boot/dts/marvell/armada-385-atl-x530.dts  |  13 +-
 drivers/auxdisplay/Kconfig                    |  11 ++
 drivers/auxdisplay/Makefile                   |   1 +
 drivers/auxdisplay/seg-led-gpio.c             | 112 ++++++++++++++++++
 5 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/gpio-7-segment.yaml
 create mode 100644 drivers/auxdisplay/seg-led-gpio.c

-- 
2.43.2

[PATCH v6 1/3] auxdisplay: Add 7-segment LED display driver

[Chris Packham]

Add a driver for a 7-segment LED display. At the moment only one
character is supported but it should be possible to expand this to
support more characters and/or 14-segment displays in the future.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v6:
    - Enforce maximum number of GPIOs
    - Drop unnecessary 0 in initialiser
    Changes in v5:
    - Add depends on GPIOLIB || COMPILE_TEST
    - Validate that we get enough GPIOs
    Changes in v4:
    - Fix one more usage of 7 segment
    - Move ASCII art diagram to DT binding
    - Include map_to_7segment.h for map_to_seg7()
    - Use initialiser for values in seg_led_update
    Changes in v3:
    - "7 segment" -> "7-Segment" in Kconfig help text
    - Update after feedback from Andy
    - Use compatible = "gpio-7-segment" as suggested by Rob
    Changes in v2:
    - Rebased on top of auxdisplay/for-next dropping unnecessary code for 7
      segment maps.
    - Update for new linedisp_register() API
    - Include headers based on actual usage
    - Allow building as module
    - Use compatible = "generic-gpio-7seg" to keep checkpatch.pl happy

 drivers/auxdisplay/Kconfig        |  11 +++
 drivers/auxdisplay/Makefile       |   1 +
 drivers/auxdisplay/seg-led-gpio.c | 112 ++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 drivers/auxdisplay/seg-led-gpio.c

diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index d4be0a3695ce..151d95f96b11 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -211,6 +211,17 @@ config ARM_CHARLCD
 	  line and the Linux version on the second line, but that's
 	  still useful.
 
+config SEG_LED_GPIO
+	tristate "Generic 7-segment LED display"
+	depends on GPIOLIB || COMPILE_TEST
+	select LINEDISP
+	help
+	  This driver supports a generic 7-segment LED display made up
+	  of GPIO pins connected to the individual segments.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called seg-led-gpio.
+
 menuconfig PARPORT_PANEL
 	tristate "Parallel port LCD/Keypad Panel support"
 	depends on PARPORT
diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile
index a725010ca651..4a8ea41b0550 100644
--- a/drivers/auxdisplay/Makefile
+++ b/drivers/auxdisplay/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_PARPORT_PANEL)	+= panel.o
 obj-$(CONFIG_LCD2S)		+= lcd2s.o
 obj-$(CONFIG_LINEDISP)		+= line-display.o
 obj-$(CONFIG_MAX6959)		+= max6959.o
+obj-$(CONFIG_SEG_LED_GPIO)	+= seg-led-gpio.o
diff --git a/drivers/auxdisplay/seg-led-gpio.c b/drivers/auxdisplay/seg-led-gpio.c
new file mode 100644
index 000000000000..5dc2a006cac5
--- /dev/null
+++ b/drivers/auxdisplay/seg-led-gpio.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for a 7-segment LED display
+ *
+ * The decimal point LED present on some devices is currently not
+ * supported.
+ *
+ * Copyright (C) Allied Telesis Labs
+ */
+
+#include <linux/bitmap.h>
+#include <linux/container_of.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/map_to_7segment.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include "line-display.h"
+
+struct seg_led_priv {
+	struct linedisp linedisp;
+	struct delayed_work work;
+	struct gpio_descs *segment_gpios;
+};
+
+static void seg_led_update(struct work_struct *work)
+{
+	struct seg_led_priv *priv = container_of(work, struct seg_led_priv, work.work);
+	struct linedisp *linedisp = &priv->linedisp;
+	struct linedisp_map *map = linedisp->map;
+	DECLARE_BITMAP(values, 8) = { };
+
+	bitmap_set_value8(values, map_to_seg7(&map->map.seg7, linedisp->buf[0]), 0);
+
+	gpiod_set_array_value_cansleep(priv->segment_gpios->ndescs, priv->segment_gpios->desc,
+				       priv->segment_gpios->info, values);
+}
+
+static int seg_led_linedisp_get_map_type(struct linedisp *linedisp)
+{
+	struct seg_led_priv *priv = container_of(linedisp, struct seg_led_priv, linedisp);
+
+	INIT_DELAYED_WORK(&priv->work, seg_led_update);
+	return LINEDISP_MAP_SEG7;
+}
+
+static void seg_led_linedisp_update(struct linedisp *linedisp)
+{
+	struct seg_led_priv *priv = container_of(linedisp, struct seg_led_priv, linedisp);
+
+	schedule_delayed_work(&priv->work, 0);
+}
+
+static const struct linedisp_ops seg_led_linedisp_ops = {
+	.get_map_type = seg_led_linedisp_get_map_type,
+	.update = seg_led_linedisp_update,
+};
+
+static int seg_led_probe(struct platform_device *pdev)
+{
+	struct seg_led_priv *priv;
+	struct device *dev = &pdev->dev;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->segment_gpios = devm_gpiod_get_array(dev, "segment", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->segment_gpios))
+		return PTR_ERR(priv->segment_gpios);
+
+	if (priv->segment_gpios->ndescs < 7 || priv->segment_gpios->ndescs > 8)
+		return -EINVAL;
+
+	return linedisp_register(&priv->linedisp, dev, 1, &seg_led_linedisp_ops);
+}
+
+static int seg_led_remove(struct platform_device *pdev)
+{
+	struct seg_led_priv *priv = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&priv->work);
+	linedisp_unregister(&priv->linedisp);
+
+	return 0;
+}
+
+static const struct of_device_id seg_led_of_match[] = {
+	{ .compatible = "gpio-7-segment"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, seg_led_of_match);
+
+static struct platform_driver seg_led_driver = {
+	.probe = seg_led_probe,
+	.remove = seg_led_remove,
+	.driver = {
+		.name = "seg-led-gpio",
+		.of_match_table = seg_led_of_match,
+	},
+};
+module_platform_driver(seg_led_driver);
+
+MODULE_AUTHOR("Chris Packham <chris.packham@alliedtelesis.co.nz>");
+MODULE_DESCRIPTION("7 segment LED driver");
+MODULE_LICENSE("GPL");
-- 
2.43.2

[PATCH v6 2/3] dt-bindings: auxdisplay: Add bindings for generic 7-segment LED

[Chris Packham]

Add bindings for a generic 7-segment LED display using GPIOs.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---

Notes:
    Changes in v6:
    - Collect R-by from Geert and Rob
    Changes in v5:
    - Preserve formatting, maxItems set to 8, group GPIO specifiers
      as suggested by Geert
    Changes in v4:
    - Add ASCII art diagram showing arrangement of segments
    Changes in v3:
    - Set maxItems: 7
    - Expand description of segment-gpios property.
    - Use compatible = "gpio-7-segment" as suggested by Rob
    Changes in v2:
    - Use compatible = "generic-gpio-7seg" to keep checkpatch.pl happy

 .../bindings/auxdisplay/gpio-7-segment.yaml   | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/auxdisplay/gpio-7-segment.yaml

diff --git a/Documentation/devicetree/bindings/auxdisplay/gpio-7-segment.yaml b/Documentation/devicetree/bindings/auxdisplay/gpio-7-segment.yaml
new file mode 100644
index 000000000000..328954893c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/gpio-7-segment.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/gpio-7-segment.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO based LED segment display
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+properties:
+  compatible:
+    const: gpio-7-segment
+
+  segment-gpios:
+    description: |
+      An array of GPIOs one per segment. The first GPIO corresponds to the A
+      segment, the seventh GPIO corresponds to the G segment. Some LED blocks
+      also have a decimal point which can be specified as an optional eighth
+      segment.
+
+               -a-
+              |   |
+              f   b
+              |   |
+               -g-
+              |   |
+              e   c
+              |   |
+               -d-  dp
+
+    minItems: 7
+    maxItems: 8
+
+required:
+  - segment-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/gpio/gpio.h>
+
+    led-7seg {
+        compatible = "gpio-7-segment";
+        segment-gpios = <&gpio 0 GPIO_ACTIVE_LOW>,
+                        <&gpio 1 GPIO_ACTIVE_LOW>,
+                        <&gpio 2 GPIO_ACTIVE_LOW>,
+                        <&gpio 3 GPIO_ACTIVE_LOW>,
+                        <&gpio 4 GPIO_ACTIVE_LOW>,
+                        <&gpio 5 GPIO_ACTIVE_LOW>,
+                        <&gpio 6 GPIO_ACTIVE_LOW>;
+    };
-- 
2.43.2

[PATCH v6 3/3] ARM: dts: marvell: Add 7-segment LED display on x530

[Chris Packham]

The Allied Telesis x530 products have a 7-segment LED display which is
used for node identification when the devices are stacked. Represent
this as a gpio-7-segment device.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v6:
    - None
    Changes in v5:
    - group GPIO specifiers
    Changes in v4:
    - Use correct compatible name in commit message
    Changes in v3:
    - Use compatible = "gpio-7-segment" as suggested by Rob
    Changes in v2:
    - Use compatible = "generic-gpio-7seg" to keep checkpatch.pl happy

 arch/arm/boot/dts/marvell/armada-385-atl-x530.dts | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts b/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
index 5a9ab8410b7b..2fb7304039be 100644
--- a/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
@@ -43,6 +43,17 @@ uart0: serial@12000 {
 			};
 		};
 	};
+
+	led-7seg {
+		compatible = "gpio-7-segment";
+		segment-gpios = <&led_7seg_gpio 0 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 1 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 2 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 3 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 4 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 5 GPIO_ACTIVE_LOW>,
+				<&led_7seg_gpio 6 GPIO_ACTIVE_LOW>;
+	};
 };
 
 &pciec {
@@ -149,7 +160,7 @@ i2c@3 {
 			#size-cells = <0>;
 			reg = <3>;
 
-			gpio@20 {
+			led_7seg_gpio: gpio@20 {
 				compatible = "nxp,pca9554";
 				gpio-controller;
 				#gpio-cells = <2>;
-- 
2.43.2

Re: [PATCH v6 1/3] auxdisplay: Add 7-segment LED display driver

[Geert Uytterhoeven]

On Thu, Mar 7, 2024 at 8:51 PM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:
> Add a driver for a 7-segment LED display. At the moment only one
> character is supported but it should be possible to expand this to
> support more characters and/or 14-segment displays in the future.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>
> Notes:
>     Changes in v6:
>     - Enforce maximum number of GPIOs
>     - Drop unnecessary 0 in initialiser

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v6 0/3] auxdisplay: 7-segment LED display

[Andy Shevchenko]

On Fri, Mar 08, 2024 at 08:50:50AM +1300, Chris Packham wrote:
> This series adds a driver for a 7-segment LED display.

You were too fast to miss some tags, but they are related to the last patch
which according to Arnd's suggestion I'm not going to take right now.

So, the first two on their way, thanks!

-- 
With Best Regards,
Andy Shevchenko