* [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display
@ 2016-10-17 17:10 Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 2/3] drivers: misc: Platform " Jaghathiswari Rankappagounder Natarajan
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-10-17 17:10 UTC (permalink / raw)
To: openbmc; +Cc: joel, raltherr, xow, 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 <jaghu@google.com>
---
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
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4617ddc..cf07817 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -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.
+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
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 724861b..718dc2b 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -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
diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.c
new file mode 100644
index 0000000..88df4a0
--- /dev/null
+++ b/drivers/misc/seven_seg_disp.c
@@ -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
+ len, loff_t *off)
+{
+ return 0;
+}
+
+static u16 convert_to_disp_data(char *buf)
+{
+ u8 low_display;
+ u8 high_display;
+ u16 led_value;
+
+ low_display = seven_seg_bits[hex_to_bin(buf[2])];
+
+ high_display = (buf[0] == '1') ?
+ special_seven_seg_bits[hex_to_bin(buf[1])] :
+ seven_seg_bits[hex_to_bin(buf[1])];
+
+ led_value = low_display | (high_display << 8);
+ if (buf[0] == '1') {
+ led_value |= LED_DOT | (LED_DOT << 8);
+ }
+
+ return led_value;
+}
+
+static ssize_t seven_seg_disp_write(struct file *filp, const char __user *buf,
+ size_t len, loff_t *off)
+{
+ 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;
+ }
+
+ if (copy_from_user(tmp, buf, length) != 0) {
+ return -EFAULT;
+ }
+
+ for (i = 0; i < MAX_DISP_CHAR_SIZE; i++) {
+ if (!isxdigit(tmp[i]))
+ return -EINVAL;
+ }
+
+ result = convert_to_disp_data(tmp);
+
+ 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,
+ 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) {
+ 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 <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display character driver");
diff --git a/drivers/misc/seven_seg_gen.h b/drivers/misc/seven_seg_gen.h
new file mode 100644
index 0000000..5748a18
--- /dev/null
+++ b/drivers/misc/seven_seg_gen.h
@@ -0,0 +1,32 @@
+/*
+ * Seven segment display support
+ * * 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
+
+#include <linux/device.h>
+#include <linux/cdev.h>
+
+#define DEFAULT_REFRESH_INTERVAL 1000
+
+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));
+
+void rem_cdev(struct seven_seg_disp_dev *disp_dev);
+
+#endif
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH linux v2 2/3] drivers: misc: Platform driver for seven segment display
2016-10-17 17:10 [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Jaghathiswari Rankappagounder Natarajan
@ 2016-10-17 17:10 ` Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 3/3] devicetree: Add devicetree changes to support seven segment display on zaius Jaghathiswari Rankappagounder Natarajan
2016-10-17 19:55 ` [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Rick Altherr
2 siblings, 0 replies; 5+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-10-17 17:10 UTC (permalink / raw)
To: openbmc; +Cc: joel, raltherr, xow, Jaghathiswari Rankappagounder Natarajan
Platform driver provides an API for displaying on a 7-segment display,
and implements the required bit-banging.
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
.../bindings/misc/seven_segment_disp.txt | 23 +++
drivers/misc/Kconfig | 7 +
drivers/misc/Makefile | 1 +
drivers/misc/seven_seg_gpio.c | 202 +++++++++++++++++++++
4 files changed, 233 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/seven_segment_disp.txt
create mode 100644 drivers/misc/seven_seg_gpio.c
diff --git a/Documentation/devicetree/bindings/misc/seven_segment_disp.txt b/Documentation/devicetree/bindings/misc/seven_segment_disp.txt
new file mode 100644
index 0000000..9c01b97
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/seven_segment_disp.txt
@@ -0,0 +1,23 @@
+Seven segment display support connected to GPIO lines
+
+Required properties:
+- compatible : should be "seven-seg-gpio-dev".
+- clock-gpios : Should specify the GPIO pin connected to the Clock line on the hardware.
+- data-gpios : Should specify the GPIO pin connected to Data line on the hardware.
+- clear-gpios : Should specify the GPIO pin connected to Clear line on the hardware.
+
+Optional properties:
+- refresh-interval-ms : The interval at which to refresh the display.
+ If this property is not present, the default value is 1000.
+
+Examples:
+
+#include <dt-bindings/gpio/gpio.h>
+
+seven-seg-disp {
+ compatible = "seven-seg-gpio-dev";
+ refresh-interval-ms = "1000";
+ clock-gpios = <&gpio 0 GPIO_ACTIVE_LOW>;
+ data-gpios = <&gpio 1 GPIO_ACTIVE_HIGH>;
+ clear-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+};
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index cf07817..7adb9e3 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -814,6 +814,13 @@ config ASPEED_BT_IPMI_HOST
help
Support for the Aspeed BT ipmi host.
+config SEVEN_SEGMENT_GPIO
+ tristate "Platform driver to update seven segment display"
+ depends on SEVEN_SEGMENT_DISPLAY
+ help
+ Support to periodically update display on two seven segment displays.
+ The hardware assumed is 74HC164 wired to two seven segment displays.
+
source "drivers/misc/c2port/Kconfig"
source "drivers/misc/eeprom/Kconfig"
source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 718dc2b..71e364d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -59,3 +59,4 @@ 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
+obj-$(CONFIG_SEVEN_SEGMENT_GPIO) += seven_seg_gpio.o
diff --git a/drivers/misc/seven_seg_gpio.c b/drivers/misc/seven_seg_gpio.c
new file mode 100644
index 0000000..f64f280
--- /dev/null
+++ b/drivers/misc/seven_seg_gpio.c
@@ -0,0 +1,202 @@
+/*
+ * Platform driver to periodically update two 7-segment LED
+ * * displays. Hardware assumed is 74HC164 shift register wired to two
+ * * 7-segment LED displays.
+ * *
+ * * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/sizes.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+#include <linux/gpio/consumer.h>
+
+#include "seven_seg_gen.h"
+
+#define DELAY_INTVL_US 1
+
+#define CLOCK_GPIO_NAME "clock"
+#define DATA_GPIO_NAME "data"
+#define CLEAR_GPIO_NAME "clear"
+
+struct seven_seg_gpio_info {
+ u8 disp_data_valid;
+ u16 curr_disp_value;
+ u16 refresh_interval;
+ struct mutex mutex;
+ struct timer_list update_timer;
+ struct gpio_desc *clock_gpio;
+ struct gpio_desc *data_gpio;
+ struct gpio_desc *clear_gpio;
+};
+
+static void update_seven_seg_gpio_data(struct device *dev, u16 data)
+{
+ struct platform_device *pdev;
+ struct seven_seg_gpio_info *gpio_info;
+
+ pdev = container_of(dev, struct platform_device, dev);
+ if (pdev == NULL) {
+ pr_err("invalid NULL platform_device\n");
+ return;
+ }
+
+ gpio_info = platform_get_drvdata(pdev);
+ if (gpio_info == NULL) {
+ pr_err("invalid NULL vpu_subdev_data\n");
+ return;
+ }
+
+ mutex_lock(&gpio_info->mutex);
+ gpio_info->curr_disp_value = data;
+ gpio_info->disp_data_valid = 1;
+ mutex_unlock(&gpio_info->mutex);
+
+}
+
+static void send_seven_seg_gpio_data(u16 disp_data,
+ struct seven_seg_gpio_info *gpio_info)
+{
+ int i;
+
+ gpiod_set_value(gpio_info->clear_gpio, 0);
+ udelay(DELAY_INTVL_US);
+ gpiod_set_value(gpio_info->clear_gpio, 1);
+ udelay(DELAY_INTVL_US);
+
+ for (i = 0; i < 16; i++) {
+ if (disp_data & 0x01) {
+ gpiod_set_value(gpio_info->data_gpio, 1);
+ } else {
+ gpiod_set_value(gpio_info->data_gpio, 0);
+ }
+ udelay(DELAY_INTVL_US);
+
+ gpiod_set_value(gpio_info->clock_gpio, 0);
+ udelay(DELAY_INTVL_US);
+ gpiod_set_value(gpio_info->clock_gpio, 1);
+ udelay(DELAY_INTVL_US);
+
+ disp_data >>= 1;
+ }
+}
+
+static void disp_refresh_timer_handler(unsigned long data)
+{
+ u8 valid = 0;
+ u16 disp_data;
+ struct seven_seg_gpio_info *gpio_info =
+ (struct seven_seg_gpio_info *)data;
+ mutex_lock(&gpio_info->mutex);
+ if (gpio_info->disp_data_valid) {
+ valid = 1;
+ disp_data = gpio_info->curr_disp_value;
+ }
+ mutex_unlock(&gpio_info->mutex);
+
+ if (valid)
+ send_seven_seg_gpio_data(disp_data, gpio_info);
+ mod_timer(&gpio_info->update_timer,
+ jiffies + msecs_to_jiffies(gpio_info->refresh_interval);
+}
+
+static const struct of_device_id of_seven_seg_gpio_match[] = {
+ { .compatible = "seven-seg-gpio-dev" },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, of_seven_seg_gpio_match);
+
+static int seven_seg_gpio_probe(struct platform_device *pdev)
+{
+ u16 interval;
+ int result;
+ struct seven_seg_gpio_info *gpio_info;
+ struct device *dev = &pdev->dev;
+ struct seven_seg_disp_dev *disp_dev;
+
+ gpio_info = devm_kzalloc(dev,
+ sizeof(struct seven_seg_gpio_info),
+ GFP_KERNEL);
+ if (gpio_info == NULL)
+ return -ENOMEM;
+
+ /* Requesting the clock gpio */
+ gpio_info->clock_gpio = devm_gpiod_get(dev, CLOCK_GPIO_NAME,
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(gpio_info->clock_gpio))
+ return PTR_ERR(gpio_info->clock_gpio);
+
+ /* Requesting the data gpio */
+ gpio_info->data_gpio = devm_gpiod_get(dev, DATA_GPIO_NAME,
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(gpio_info->data_gpio))
+ return PTR_ERR(gpio_info->data_gpio);
+
+ /* Requesting the clear gpio */
+ gpio_info->clear_gpio = devm_gpiod_get(dev, CLEAR_GPIO_NAME,
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(gpio_info->clear_gpio))
+ return PTR_ERR(gpio_info->clear_gpio);
+
+ result = of_property_read_u16(pdev->dev.of_node,
+ "refresh-interval-ms", &interval);
+ gpio_info->refresh_interval = result ? DEFAULT_REFRESH_INTERVAL :
+ interval;
+
+ /* Start timer to update seven segment display every second */
+ setup_timer(&gpio_info->update_timer, disp_refresh_timer_handler,
+ (unsigned long)gpio_info);
+ result = mod_timer(&gpio_info->update_timer,
+ jiffies +
+ msecs_to_jiffies(gpio_info->refresh_interval));
+ if (result)
+ return result;
+
+ mutex_init(&gpio_info->mutex);
+
+ platform_set_drvdata(pdev, gpio_info);
+
+ disp_dev = devm_kzalloc(dev, sizeof(struct seven_seg_disp_dev),
+ GFP_KERNEL);
+ setup_cdev(*dev, disp_dev, &update_seven_seg_gpio_data);
+
+ return 0;
+}
+
+static int seven_seg_gpio_remove(struct platform_device *pdev)
+{
+ struct seven_seg_gpio_info *gpio_info = platform_get_drvdata(pdev);
+ struct seven_seg_disp_dev *disp_dev =
+ container_of(&pdev->dev, struct seven_seg_disp_dev, parent);
+ rem_cdev(disp_dev);
+ del_timer_sync(&gpio_info->update_timer);
+ platform_set_drvdata(pdev, NULL);
+ return 0;
+}
+
+static struct platform_driver seven_seg_gpio_driver = {
+ .probe = seven_seg_gpio_probe,
+ .remove = seven_seg_gpio_remove,
+ .driver = {
+ .name = "seven-seg-gpio",
+ .of_match_table = of_seven_seg_gpio_match,
+ },
+};
+
+module_platform_driver(seven_seg_gpio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>");
+MODULE_DESCRIPTION("Seven segment display driver using GPIO config");
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH linux v2 3/3] devicetree: Add devicetree changes to support seven segment display on zaius
2016-10-17 17:10 [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 2/3] drivers: misc: Platform " Jaghathiswari Rankappagounder Natarajan
@ 2016-10-17 17:10 ` Jaghathiswari Rankappagounder Natarajan
2016-10-17 19:55 ` [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Rick Altherr
2 siblings, 0 replies; 5+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-10-17 17:10 UTC (permalink / raw)
To: openbmc; +Cc: joel, raltherr, xow, Jaghathiswari Rankappagounder Natarajan
Add clock, data and clear signal lines to control seven segment display on
zaius
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
---
arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index 4c4754b..7581540 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -1,6 +1,7 @@
/dts-v1/;
#include "aspeed-g5.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
/ {
model = "Zaius BMC";
@@ -58,6 +59,14 @@
};
};
};
+
+ seven-seg-disp {
+ compatible = "seven-seg-gpio-dev";
+ refresh-interval-ms = "1000";
+ clock-gpios = <&gpio ASPEED_GPIO(J, 0) GPIO_ACTIVE_HIGH>;
+ data-gpios = <&gpio ASPEED_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
+ clear-gpios = <&gpio ASPEED_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
+ };
};
&uart5 {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display
2016-10-17 17:10 [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 2/3] drivers: misc: Platform " Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 3/3] devicetree: Add devicetree changes to support seven segment display on zaius Jaghathiswari Rankappagounder Natarajan
@ 2016-10-17 19:55 ` Rick Altherr
2016-11-01 16:53 ` Jaghathiswari Rankappagounder Natarajan
2 siblings, 1 reply; 5+ messages in thread
From: Rick Altherr @ 2016-10-17 19:55 UTC (permalink / raw)
To: Jaghathiswari Rankappagounder Natarajan
Cc: OpenBMC Maillist, Joel Stanley, Xo Wang
[-- Attachment #1: Type: text/plain, Size: 9668 bytes --]
On Mon, Oct 17, 2016 at 10:10 AM, Jaghathiswari Rankappagounder Natarajan <
jaghu@google.com> wrote:
> 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 <jaghu@google.com>
> ---
> 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
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4617ddc..cf07817 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -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.
>
> +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
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 724861b..718dc2b 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -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
> diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.c
> new file mode 100644
> index 0000000..88df4a0
> --- /dev/null
> +++ b/drivers/misc/seven_seg_disp.c
> @@ -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
> + len, loff_t *off)
> +{
>
May as well return the current value to them.
> + return 0;
> +}
> +
> +static u16 convert_to_disp_data(char *buf)
> +{
> + u8 low_display;
> + u8 high_display;
> + u16 led_value;
> +
> + low_display = seven_seg_bits[hex_to_bin(buf[2])];
> +
> + high_display = (buf[0] == '1') ?
> + special_seven_seg_bits[hex_to_bin(buf[1])] :
> + seven_seg_bits[hex_to_bin(buf[1])];
> +
> + led_value = low_display | (high_display << 8);
> + if (buf[0] == '1') {
> + led_value |= LED_DOT | (LED_DOT << 8);
> + }
> +
> + return led_value;
> +}
> +
> +static ssize_t seven_seg_disp_write(struct file *filp, const char __user
> *buf,
> + size_t len, loff_t *off)
> +{
> + 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;
> + }
> +
> + 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.
> + if (!isxdigit(tmp[i]))
> + return -EINVAL;
> + }
> +
> + result = convert_to_disp_data(tmp);
>
Pick a more descriptive name than 'result' and 'tmp'. Describe what the
variable contains.
> +
> + 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,
> + 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) {
> + 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 <jaghu@google.com
> >");
> +MODULE_DESCRIPTION("Seven segment display character driver");
> diff --git a/drivers/misc/seven_seg_gen.h b/drivers/misc/seven_seg_gen.h
> new file mode 100644
> index 0000000..5748a18
> --- /dev/null
> +++ b/drivers/misc/seven_seg_gen.h
>
Why is this header a different name from the .c?
> @@ -0,0 +1,32 @@
> +/*
> + * Seven segment display support
> + * * 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.
> +
> +#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.
> +
> +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.
> +
> +void rem_cdev(struct seven_seg_disp_dev *disp_dev);
> +
> +#endif
> --
> 2.8.0.rc3.226.g39d4020
>
>
[-- Attachment #2: Type: text/html, Size: 12978 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display
2016-10-17 19:55 ` [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Rick Altherr
@ 2016-11-01 16:53 ` Jaghathiswari Rankappagounder Natarajan
0 siblings, 0 replies; 5+ messages in thread
From: Jaghathiswari Rankappagounder Natarajan @ 2016-11-01 16:53 UTC (permalink / raw)
To: Rick Altherr; +Cc: OpenBMC Maillist, Joel Stanley, Xo Wang
[-- Attachment #1: Type: text/plain, Size: 10458 bytes --]
On Mon, Oct 17, 2016 at 12:55 PM, Rick Altherr <raltherr@google.com> wrote:
>
>
> On Mon, Oct 17, 2016 at 10:10 AM, Jaghathiswari Rankappagounder Natarajan
> <jaghu@google.com> wrote:
>
>> 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 <jaghu@google.com>
>> ---
>> 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
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 4617ddc..cf07817 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -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.
>>
>> +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
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 724861b..718dc2b 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -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
>> diff --git a/drivers/misc/seven_seg_disp.c b/drivers/misc/seven_seg_disp.
>> c
>> new file mode 100644
>> index 0000000..88df4a0
>> --- /dev/null
>> +++ b/drivers/misc/seven_seg_disp.c
>> @@ -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
>> + len, loff_t *off)
>> +{
>>
> May as well return the current value to them.
>
>
>> + return 0;
>> +}
>> +
>> +static u16 convert_to_disp_data(char *buf)
>> +{
>> + u8 low_display;
>> + u8 high_display;
>> + u16 led_value;
>> +
>> + low_display = seven_seg_bits[hex_to_bin(buf[2])];
>> +
>> + high_display = (buf[0] == '1') ?
>> + special_seven_seg_bits[hex_to_bin(buf[1])] :
>> + seven_seg_bits[hex_to_bin(buf[1])];
>> +
>> + led_value = low_display | (high_display << 8);
>> + if (buf[0] == '1') {
>> + led_value |= LED_DOT | (LED_DOT << 8);
>> + }
>> +
>> + return led_value;
>> +}
>> +
>> +static ssize_t seven_seg_disp_write(struct file *filp, const char __user
>> *buf,
>> + size_t len, loff_t *off)
>> +{
>> + 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;
>> + }
>> +
>> + 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.
>
>
I am checking if the user is writing MAX_DISP_CHAR_SIZE data (3 bytes of
data) in the previous step (length != MAX_DISP_CHAR_SIZE).
Wanted to make sure user is writing 3 bytes of data so that conversion
could happen correctly.
+ if (!isxdigit(tmp[i]))
>> + return -EINVAL;
>> + }
>> +
>> + result = convert_to_disp_data(tmp);
>>
> Pick a more descriptive name than 'result' and 'tmp'. Describe what the
> variable contains.
>
>
>> +
>> + 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,
>> + 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) {
>> + 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 <jaghu@google.com
>> >");
>> +MODULE_DESCRIPTION("Seven segment display character driver");
>> diff --git a/drivers/misc/seven_seg_gen.h b/drivers/misc/seven_seg_gen.h
>> new file mode 100644
>> index 0000000..5748a18
>> --- /dev/null
>> +++ b/drivers/misc/seven_seg_gen.h
>>
>
> Why is this header a different name from the .c?
>
>
>> @@ -0,0 +1,32 @@
>> +/*
>> + * Seven segment display support
>> + * * 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.
>
>
>> +
>> +#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.
>
If the refresh-interval-ms property is not provided in the device tree then
this value is used as default value(in file seven_seg_gpio.c)
> +
>> +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.
>
>
>> +
>> +void rem_cdev(struct seven_seg_disp_dev *disp_dev);
>> +
>> +#endif
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>>
>
[-- Attachment #2: Type: text/html, Size: 14749 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-11-01 16:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17 17:10 [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 2/3] drivers: misc: Platform " Jaghathiswari Rankappagounder Natarajan
2016-10-17 17:10 ` [PATCH linux v2 3/3] devicetree: Add devicetree changes to support seven segment display on zaius Jaghathiswari Rankappagounder Natarajan
2016-10-17 19:55 ` [PATCH linux v2 1/3] drivers: misc: Character driver for seven segment display Rick Altherr
2016-11-01 16:53 ` Jaghathiswari Rankappagounder Natarajan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.