drivers: misc: Character driver for seven segment display

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

Signed-off-by Jaghathiswari Rankappagounder Natarajan <[email protected]>

---
 drivers/misc/Kconfig          |   5 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_disp.c | 192 ++++++++++++++++++++++++++++++
++++++++++++
 drivers/misc/seven_seg_gen.h  |  32 +++++++
 4 files changed, 230 insertions(+)
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_gen.h

drivers/misc/Kconfig +0 -0

index 4617ddc..cf07817 100644
@@ -804,6 +804,11 @@ config PANEL_BOOT_MESSAGE
          An empty message will only clear the display at driver init

time. Any other
printf()-formatted message is valid with newline and escape
codes.

drivers/misc/Kconfig +5 -0

+config SEVEN_SEGMENT_DISPLAY
+       tristate "Character driver for seven segment display support"
+       help
+         Character driver for seven segment display support
+
 config ASPEED_BT_IPMI_HOST
        tristate "BT IPMI host driver"
        help

drivers/misc/Makefile +1 -0

index 724861b..718dc2b 100644
@@ -57,4 +57,5 @@ obj-$(CONFIG_ECHO)            += echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)  += vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)         += cxl/
 obj-$(CONFIG_PANEL)             += panel.o
+obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)    += seven_seg_disp.o
 obj-$(CONFIG_ASPEED_BT_IPMI_HOST)      += bt-host.o

drivers/misc/seven_seg_disp.c new file +71 -0

new file mode 100644
index 0000000..88df4a0
@@ -0,0 +1,192 @@
+/*
+ * Seven segment display character driver
+ * * Copyright (c) 2016 Google, Inc
+ *
+ * * This program is free software; you can redistribute it and/or modify
+ * * it under the terms of the GNU General Public License version 2 as
+ * * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/ctype.h>
+
+#include "seven_seg_gen.h"
+
+#define MAX_DISP_CHAR_SIZE 3
+
+#define LED_DOT 0x01
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *  _       _   _       _   _   _   _   _   _       _       _   _
+ * | |   |  _|  _| |_| |_  |_    | |_| |_| |_| |_  |    _| |_  |_
+ * |_|   | |_   _|   |  _| |_|   | |_|   | | | |_| |_  |_| |_  |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 seven_seg_bits[] = {
+       0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
+       0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
+       };
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *      _       _   _                              _            _
+ *     |   |_  |_| |_  _   _   _   _   _   _   _  |_    _|  _| | |
+ *     |_  |_  |   |                               _|  |_| |_| | |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 special_seven_seg_bits[] = {
+       0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
+       0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
+       };
+
+static dev_t seven_seg_devno;
+static struct class *seven_seg_disp_class;
+
+static int seven_seg_disp_open(struct inode *inode, struct file *filp)
+{
+       struct seven_seg_disp_dev *disp_dev;
+
+       disp_dev = container_of(inode->i_cdev,
+                                struct seven_seg_disp_dev, cdev);
+       filp->private_data = disp_dev;
+       return 0;
+}
+
+static int seven_seg_disp_close(struct inode *inode, struct file *filp)
+{
+       filp->private_data = NULL;
+       return 0;
+}
+
+static ssize_t seven_seg_disp_read(struct file *filp, char __user *buf,

size_t

drivers/misc/seven_seg_disp.c new file +24 -0

+       return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+       u8 low_display;
+       u8 high_display;
+       u16 led_value;
+
+       low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+       high_display = (buf[0] == '1') ?
+       special_seven_seg_bits[hex_to_bin(buf[1])] :
+       seven_seg_bits[hex_to_bin(buf[1])];
+
+       led_value = low_display | (high_display << 8);
+       if (buf[0] == '1') {
+               led_value |= LED_DOT | (LED_DOT << 8);
+       }
+
+       return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user

*buf,

drivers/misc/seven_seg_disp.c new file +8 -0

+                               size_t len, loff_t *off)
+{
+       char tmp[MAX_DISP_CHAR_SIZE];
+       int length = len - 1;
+       int i;
+       u8 result;
+
+       struct seven_seg_disp_dev *disp_dev;

drivers/misc/seven_seg_gen.h new file +2 -0

@@ -0,0 +1,32 @@
+/*
+ * Seven segment display support

drivers/misc/seven_seg_gen.h new file +4 -1

+
+void rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--

2.8.0.rc3.226.g39d4020

Quoted 123 lines · Jaghathiswari Rankappagounder Natarajan

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

Signed-off-by Jaghathiswari Rankappagounder Natarajan <[email protected]>

---
 drivers/misc/Kconfig          |   5 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/seven_seg_disp.c | 192 ++++++++++++++++++++++++++++++
++++++++++++
 drivers/misc/seven_seg_gen.h  |  32 +++++++
 4 files changed, 230 insertions(+)
 create mode 100644 drivers/misc/seven_seg_disp.c
 create mode 100644 drivers/misc/seven_seg_gen.h

drivers/misc/Kconfig +0 -0

index 4617ddc..cf07817 100644
@@ -804,6 +804,11 @@ config PANEL_BOOT_MESSAGE
          An empty message will only clear the display at driver init

time. Any other
printf()-formatted message is valid with newline and escape
codes.

drivers/misc/Kconfig +5 -0

+config SEVEN_SEGMENT_DISPLAY
+       tristate "Character driver for seven segment display support"
+       help
+         Character driver for seven segment display support
+
 config ASPEED_BT_IPMI_HOST
        tristate "BT IPMI host driver"
        help

drivers/misc/Makefile +1 -0

index 724861b..718dc2b 100644
@@ -57,4 +57,5 @@ obj-$(CONFIG_ECHO)            += echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)  += vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)         += cxl/
 obj-$(CONFIG_PANEL)             += panel.o
+obj-$(CONFIG_SEVEN_SEGMENT_DISPLAY)    += seven_seg_disp.o
 obj-$(CONFIG_ASPEED_BT_IPMI_HOST)      += bt-host.o

drivers/misc/seven_seg_disp. renamed from drivers/misc/seven_seg_disp.c +0 -0

c
new file mode 100644
index 0000000..88df4a0

drivers/misc/seven_seg_disp.c new file +71 -0

@@ -0,0 +1,192 @@
+/*
+ * Seven segment display character driver
+ * * Copyright (c) 2016 Google, Inc
+ *
+ * * This program is free software; you can redistribute it and/or modify
+ * * it under the terms of the GNU General Public License version 2 as
+ * * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/kdev_t.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/ctype.h>
+
+#include "seven_seg_gen.h"
+
+#define MAX_DISP_CHAR_SIZE 3
+
+#define LED_DOT 0x01
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *  _       _   _       _   _   _   _   _   _       _       _   _
+ * | |   |  _|  _| |_| |_  |_    | |_| |_| |_| |_  |    _| |_  |_
+ * |_|   | |_   _|   |  _| |_|   | |_|   | | | |_| |_  |_| |_  |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 seven_seg_bits[] = {
+       0xFC, 0x60, 0xDA, 0xF2, 0x66, 0xB6, 0xBE, 0xE0,
+       0xFE, 0xF6, 0xEE, 0x3E, 0x9C, 0x7A, 0x9E, 0x8E
+       };
+
+/*
+ * 0   1   2   3   4   5   6   7   8   9   A   B   C   D   E   F
+ *      _       _   _                              _            _
+ *     |   |_  |_| |_  _   _   _   _   _   _   _  |_    _|  _| | |
+ *     |_  |_  |   |                               _|  |_| |_| | |
+ *
+ * data[7:1] = led[a:g]
+ */
+const u8 special_seven_seg_bits[] = {
+       0x00, 0x9C, 0x1E, 0xCE, 0x8E, 0x02, 0x02, 0x02,
+       0x02, 0x02, 0x02, 0x02, 0xB6, 0x7A, 0x7A, 0xEC
+       };
+
+static dev_t seven_seg_devno;
+static struct class *seven_seg_disp_class;
+
+static int seven_seg_disp_open(struct inode *inode, struct file *filp)
+{
+       struct seven_seg_disp_dev *disp_dev;
+
+       disp_dev = container_of(inode->i_cdev,
+                                struct seven_seg_disp_dev, cdev);
+       filp->private_data = disp_dev;
+       return 0;
+}
+
+static int seven_seg_disp_close(struct inode *inode, struct file *filp)
+{
+       filp->private_data = NULL;
+       return 0;
+}
+
+static ssize_t seven_seg_disp_read(struct file *filp, char __user *buf,

size_t

drivers/misc/seven_seg_disp.c

Quoted 2 lines

drivers/misc/seven_seg_disp.c new file +2 -0

+                               len, loff_t *off)
+{

May as well return the current value to them.

Quoted 38 lines

drivers/misc/seven_seg_disp.c new file +24 -0

+       return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+       u8 low_display;
+       u8 high_display;
+       u16 led_value;
+
+       low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+       high_display = (buf[0] == '1') ?
+       special_seven_seg_bits[hex_to_bin(buf[1])] :
+       seven_seg_bits[hex_to_bin(buf[1])];
+
+       led_value = low_display | (high_display << 8);
+       if (buf[0] == '1') {
+               led_value |= LED_DOT | (LED_DOT << 8);
+       }
+
+       return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user

*buf,

drivers/misc/seven_seg_disp.c new file +13 -0

+                               size_t len, loff_t *off)
+{
+       char tmp[MAX_DISP_CHAR_SIZE];
+       int length = len - 1;
+       int i;
+       u8 result;
+
+       struct seven_seg_disp_dev *disp_dev;
+
+       if (length != MAX_DISP_CHAR_SIZE) {
+               return -EINVAL;
+       }
+

drivers/misc/seven_seg_disp.c line 115

Quoted 6 lines

drivers/misc/seven_seg_disp.c new file +5 -0

+       if (copy_from_user(tmp, buf, length) != 0) {
+               return -EFAULT;
+       }
+
+       for (i = 0; i < MAX_DISP_CHAR_SIZE; i++) {

Only check the bytes actually provided by the user. If they do a 1-byte write, you'll read uninit data.

drivers/misc/seven_seg_disp.c line 120

Quoted 5 lines

drivers/misc/seven_seg_disp.c new file +4 -0

+                       return -EINVAL;
+       }
+
+       result = convert_to_disp_data(tmp);

Pick a more descriptive name than 'result' and 'tmp'. Describe what the variable contains.

drivers/misc/seven_seg_gen.h

Quoted 81 lines

drivers/misc/seven_seg_disp.c new file +30 -0

+
+       disp_dev = filp->private_data;
+       disp_dev->update_seven_seg_data(&disp_dev->parent, result);
+
+       return len;
+}
+
+static const struct file_operations seven_seg_disp_fops = {
+
+       .owner = THIS_MODULE,
+       .open = seven_seg_disp_open,
+       .release = seven_seg_disp_close,
+       .read = seven_seg_disp_read,
+       .write = seven_seg_disp_write
+};
+
+void rem_cdev(struct seven_seg_disp_dev *disp_dev)
+{
+       cdev_del(&disp_dev->cdev);
+       device_destroy(seven_seg_disp_class, seven_seg_devno);
+}
+
+int setup_cdev(struct device parent,
+       struct seven_seg_disp_dev *disp_dev,
+       void (*update_disp_data)(struct device *, u16 data))
+{
+       struct device *dev;
+       int err;
+
+       dev = device_create(seven_seg_disp_class, &parent,

seven_seg_devno,

drivers/misc/seven_seg_disp.c new file +17 -0

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

0) {

drivers/misc/seven_seg_disp.c new file +24 -0

+               return -1;
+       }
+
+       seven_seg_disp_class = class_create(THIS_MODULE, "disp_state");
+       if (seven_seg_disp_class == NULL) {
+               goto unregister;
+               return -1;
+       }
+
+unregister:
+       unregister_chrdev_region(seven_seg_devno, 1);
+       return -1;
+}
+
+static void __exit seven_seg_disp_exit(void)
+{
+       class_destroy(seven_seg_disp_class);
+       unregister_chrdev_region(seven_seg_devno, 1);
+}
+
+module_init(seven_seg_disp_init);
+module_exit(seven_seg_disp_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <[email protected]

Quoted 1 line

");

drivers/misc/seven_seg_disp.c new file +1 -0

+MODULE_DESCRIPTION("Seven segment display character driver");

drivers/misc/seven_seg_gen.h new file +0 -0

new file mode 100644
index 0000000..5748a18

Why is this header a different name from the .c?

Quoted 4 lines

drivers/misc/seven_seg_gen.h new file +2 -0

@@ -0,0 +1,32 @@
+/*
+ * Seven segment display support

drivers/misc/seven_seg_gen.h line 12

Quoted 10 lines

drivers/misc/seven_seg_gen.h new file +10 -0

+ * * Copyright (c) 2016 Google, Inc
+ *
+ * * This program is free software; you can redistribute it and/or modify
+ * * it under the terms of the GNU General Public License version 2 as
+ * * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef SEVEN_SEG_H
+#define SEVEN_SEG_H

Guards don't match header name.

drivers/misc/seven_seg_gen.h line 17

Quoted 6 lines

drivers/misc/seven_seg_gen.h new file +5 -0

+
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+#define DEFAULT_REFRESH_INTERVAL 1000

This doesn't seem to be used by any of the APIs provided.

+

drivers/misc/seven_seg_gen.h line 28

Quoted 11 lines

drivers/misc/seven_seg_gen.h new file +10 -0

+struct seven_seg_disp_dev {
+       struct device parent;
+       struct device *dev;
+       struct cdev cdev;
+       void (*update_seven_seg_data)(struct device *, u16 data);
+};
+
+int setup_cdev(struct device parent,
+       struct seven_seg_disp_dev *disp_dev,
+       void (*update_disp_data)(struct device *, u16 data));

Way too generic of a method name. This name will be exposed in any .c that includes this header. Pick something that describes what it is doing _and_ what subsystem it belongs to.

Quoted 8 lines

drivers/misc/seven_seg_gen.h new file +4 -1

+
+void rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--

2.8.0.rc3.226.g39d4020