| Message ID | 20240225213423.690561-2-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | auxdisplay: 7 segment LED display | expand |
On Sun, Feb 25, 2024 at 11:34 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. (I try to comment only on the things that will remain after rebasing on top of auxdisplay:for-next) ... > +config SEG_LED > + bool "Generic 7 segment LED display" Why can't it be a module? > + select LINEDISP > + help > + This driver supports a generic 7 segment LED display made up > + of GPIO pins connected to the individual segments. Checkpatch wants 3+ lines of help, I would add the module name (after changing it to be tristate, etc). ... > + * The GPIOs are wired to the 7 segments in a clock wise fashion starting from clockwise > + * the top. ... > + * The decimal point LED presnet on some devices is currently not present > + * supported. ... > +#include <linux/gpio/consumer.h> > +#include <linux/kernel.h> > +#include <linux/map_to_7segment.h> > +#include <linux/module.h> > +#include <linux/of.h> This is not used. And actually shouldn't be in a new code like this with rare exceptions. > +#include <linux/platform_device.h> This is rather semirandom, please use IWYU (Include What You Use) principle. We have, among others, container_of.h, types.h, err.h, bitmap.h, mod_devicetable.h. ... With sturct device *dev = &pdev->dev; the below code will be neater. > + priv = devm_kzalloc(&pdev->dev, struct_size(priv, curr, 1), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + priv->num_char = 1; > + > + platform_set_drvdata(pdev, priv); > + > + priv->segment_gpios = devm_gpiod_get_array(&pdev->dev, "segment", GPIOD_OUT_LOW); > + if (IS_ERR(priv->segment_gpios)) > + return PTR_ERR(priv->segment_gpios); ... > +static struct platform_driver seg_led_driver = { > + .probe = seg_led_probe, > + .remove = seg_led_remove, > + .driver = { > + .name = "seg-led", > + .of_match_table = seg_led_of_match, > + }, > +}; > + Redundant blank line. > +module_platform_driver(seg_led_driver); > + > +MODULE_AUTHOR("Chris Packham <chris.packham@alliedtelesis.co.nz>"); > +MODULE_DESCRIPTION("7 segment LED driver"); > +MODULE_LICENSE("GPL"); > + Seems like a redundant blank line at the end of the file.
Hi-- On 2/25/24 13:34, Chris Packham 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> > --- > drivers/auxdisplay/Kconfig | 7 ++ > drivers/auxdisplay/Makefile | 1 + > drivers/auxdisplay/seg-led.c | 152 +++++++++++++++++++++++++++++++++++ > 3 files changed, 160 insertions(+) > create mode 100644 drivers/auxdisplay/seg-led.c > > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig > index d944d5298eca..e826b5b15881 100644 > --- a/drivers/auxdisplay/Kconfig > +++ b/drivers/auxdisplay/Kconfig > @@ -197,6 +197,13 @@ config ARM_CHARLCD > line and the Linux version on the second line, but that's > still useful. > > +config SEG_LED > + bool "Generic 7 segment LED display" > + select LINEDISP > + help > + This driver supports a generic 7 segment LED display made up 7-segment > + of GPIO pins connected to the individual segments. > + > menuconfig PARPORT_PANEL > tristate "Parallel port LCD/Keypad Panel support" > depends on PARPORT > diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile > index 6968ed4d3f0a..808fdf156bd5 100644 > --- a/drivers/auxdisplay/Makefile > +++ b/drivers/auxdisplay/Makefile > @@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33) += ht16k33.o > obj-$(CONFIG_PARPORT_PANEL) += panel.o > obj-$(CONFIG_LCD2S) += lcd2s.o > obj-$(CONFIG_LINEDISP) += line-display.o > +obj-$(CONFIG_SEG_LED) += seg-led.o > diff --git a/drivers/auxdisplay/seg-led.c b/drivers/auxdisplay/seg-led.c > new file mode 100644 > index 000000000000..c0b302a09cbb > --- /dev/null > +++ b/drivers/auxdisplay/seg-led.c > @@ -0,0 +1,152 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for a 7 segment LED display > + * > + * The GPIOs are wired to the 7 segments in a clock wise fashion starting from > + * the top. > + * > + * -a- > + * | | > + * f b > + * | | > + * -g- > + * | | > + * e c > + * | | > + * -d- > + * > + * The decimal point LED presnet on some devices is currently not present > + * supported. > + * > + * Copyright (C) Allied Telesis Labs > + */
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig index d944d5298eca..e826b5b15881 100644 --- a/drivers/auxdisplay/Kconfig +++ b/drivers/auxdisplay/Kconfig @@ -197,6 +197,13 @@ config ARM_CHARLCD line and the Linux version on the second line, but that's still useful. +config SEG_LED + bool "Generic 7 segment LED display" + select LINEDISP + help + This driver supports a generic 7 segment LED display made up + of GPIO pins connected to the individual segments. + menuconfig PARPORT_PANEL tristate "Parallel port LCD/Keypad Panel support" depends on PARPORT diff --git a/drivers/auxdisplay/Makefile b/drivers/auxdisplay/Makefile index 6968ed4d3f0a..808fdf156bd5 100644 --- a/drivers/auxdisplay/Makefile +++ b/drivers/auxdisplay/Makefile @@ -14,3 +14,4 @@ obj-$(CONFIG_HT16K33) += ht16k33.o obj-$(CONFIG_PARPORT_PANEL) += panel.o obj-$(CONFIG_LCD2S) += lcd2s.o obj-$(CONFIG_LINEDISP) += line-display.o +obj-$(CONFIG_SEG_LED) += seg-led.o diff --git a/drivers/auxdisplay/seg-led.c b/drivers/auxdisplay/seg-led.c new file mode 100644 index 000000000000..c0b302a09cbb --- /dev/null +++ b/drivers/auxdisplay/seg-led.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for a 7 segment LED display + * + * The GPIOs are wired to the 7 segments in a clock wise fashion starting from + * the top. + * + * -a- + * | | + * f b + * | | + * -g- + * | | + * e c + * | | + * -d- + * + * The decimal point LED presnet on some devices is currently not + * supported. + * + * Copyright (C) Allied Telesis Labs + */ + +#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/map_to_7segment.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> + +#include "line-display.h" + +struct seg_led_priv { + struct gpio_descs *segment_gpios; + struct delayed_work work; + struct linedisp linedisp; + struct seg7_conversion_map seg7; + unsigned int map_size; + size_t num_char; + char curr[]; +}; + +static const SEG7_DEFAULT_MAP(initial_map_seg7); + +static ssize_t map_seg7_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct seg_led_priv *priv = dev_get_drvdata(dev); + + memcpy(buf, &priv->seg7, priv->map_size); + return priv->map_size; +} + +static ssize_t map_seg7_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t cnt) +{ + struct seg_led_priv *priv = dev_get_drvdata(dev); + + if (cnt != priv->map_size) + return -EINVAL; + + memcpy(&priv->seg7, buf, cnt); + return cnt; +} + +static DEVICE_ATTR_RW(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 void seg_led_update(struct work_struct *work) +{ + struct seg_led_priv *priv = container_of(work, struct seg_led_priv, work.work); + DECLARE_BITMAP(values, 8); + + values[0] = map_to_seg7(&priv->seg7, priv->curr[0]); + + gpiod_set_array_value_cansleep(priv->segment_gpios->ndescs, priv->segment_gpios->desc, + priv->segment_gpios->info, values); +} + +static const struct of_device_id seg_led_of_match[] = { + { .compatible = "generic,gpio-7seg"}, + {} +}; +MODULE_DEVICE_TABLE(of, seg_led_of_match); + +static int seg_led_probe(struct platform_device *pdev) +{ + struct seg_led_priv *priv; + int err; + + priv = devm_kzalloc(&pdev->dev, struct_size(priv, curr, 1), GFP_KERNEL); + if (!priv) + return -ENOMEM; + priv->num_char = 1; + + platform_set_drvdata(pdev, priv); + + priv->segment_gpios = devm_gpiod_get_array(&pdev->dev, "segment", GPIOD_OUT_LOW); + if (IS_ERR(priv->segment_gpios)) + return PTR_ERR(priv->segment_gpios); + + priv->seg7 = initial_map_seg7; + priv->map_size = sizeof(priv->seg7); + + err = device_create_file(&pdev->dev, &dev_attr_map_seg7); + if (err) + return err; + + INIT_DELAYED_WORK(&priv->work, seg_led_update); + + err = linedisp_register(&priv->linedisp, &pdev->dev, 1, priv->curr, + seg_led_linedisp_update); + if (err) { + device_remove_file(&pdev->dev, &dev_attr_map_seg7); + return err; + } + + return 0; +} + +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); + device_remove_file(&pdev->dev, &dev_attr_map_seg7); + + return 0; +} + +static struct platform_driver seg_led_driver = { + .probe = seg_led_probe, + .remove = seg_led_remove, + .driver = { + .name = "seg-led", + .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"); +
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> --- drivers/auxdisplay/Kconfig | 7 ++ drivers/auxdisplay/Makefile | 1 + drivers/auxdisplay/seg-led.c | 152 +++++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 drivers/auxdisplay/seg-led.c