<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 14, 2016 at 5:00 PM, Jaghathiswari Rankappagounder Natarajan <span dir="ltr"><<a href="mailto:jaghu@google.com" target="_blank">jaghu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>By this comment "want <span style="font-size:12.8px">this character device to attach to any compatible display drivers." , should I have a separate setup_character device function(add the platform device(display  driver for GPIO) as the parent of the character device)  and call this function from the corresponding display driver for GPIO? So display driver for GPIO will only setup this character device ?</span><br></div><div><br></div></div></blockquote><div>I'm getting confused by the terminology here.  I would expect a platform driver that matches on the device tree node, provides an API for displaying on a 7-segment display, and implements the bit-banging necessary to do so.  The character device driver would match on the previous platform driver and implement 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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>I have moved the function to convert character buffer to led display pattern to this char driver. The display is very specific for BMC post codes( characters '14' is displayed as digits 't4' and the DOT lights up on two segment LEDs ) and generic for host POST codes(characters '14' is displayed as '14' and DOT doesn't light up). This info is present in the lookup table.</div>When display driver for GPIO wants to use a character device with different conversion(say to display '14' as '14 with DOTs light up) it needs a different character device(which does conversion differently) ?<div><br></div></div></blockquote><div>Yes, the conversion should be associated with the character device.  My rationale is that the conversion used depends on how the user provided the data (free form string, fixed string, integer).  If the user API is different, it should have a different character device so the user-space app knows what API it needs to implement.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>should the mapping be like this char device(with a specific display pattern) can be used with any display driver(GPIO and SGPIO) and the GPIO driver/SGPIO driver can use any character device(with different display conversion patterns) ? is it better to do as above or keep as 1-1 mapping? or keep the conversion logic in BMC/Host side?<div><div class="h5"><br></div></div></div></div></blockquote><div><br></div><div>I was thinking of having a subsystem layer that abstracts interfacing with 7-segment displays.  It probably isn't necessary today but would make it simpler to implement other output drivers (using a different shift register with the Aspeed SGPIO hardware, etc) or implement different user-space APIs.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 12, 2016 at 6:15 PM, Rick Altherr <span dir="ltr"><<a href="mailto:raltherr@google.com" target="_blank">raltherr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-7487688587987672970m_-7121693134944063424gmail-h5">On Wed, Oct 12, 2016 at 5:41 PM, Jaghathiswari Rankappagounder Natarajan <span dir="ltr"><<a href="mailto:jaghu@google.com" target="_blank">jaghu@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Character driver - On state change, the BMC will write its POST code to the<br>
character device. The POST code is then sent to update-postcard platform<br>
driver for display on the POST card. The BMC POST code consists of 3 digits<br>
<br>
Signed-off-by: Jaghathiswari Rankappagounder Natarajan <<a href="mailto:jaghu@google.com" target="_blank">jaghu@google.com</a>><br>
---<br>
 drivers/misc/Kconfig               |   8 +++<br>
 drivers/misc/Makefile              |   1 +<br>
 drivers/misc/update-bmc-postc<wbr>ode.c | 123 ++++++++++++++++++++++++++++++<wbr>+++++++<br>
 3 files changed, 132 insertions(+)<br>
 create mode 100644 drivers/misc/update-bmc-postco<wbr>de.c<br>
<br>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig<br>
index d65b83f..69dc3b6 100644<br>
--- a/drivers/misc/Kconfig<br>
+++ b/drivers/misc/Kconfig<br>
@@ -815,6 +815,14 @@ config ASPEED_BT_IPMI_HOST<br>
        help<br>
          Support for the Aspeed BT ipmi host.<br>
<br>
+config ASPEED_UPDATE_BMC_POSTCODE<br>
+       tristate "Character driver for BMC POST code update"<br>
+       depends on ASPEED_UPDATE_POSTCARD<br>
+       help<br>
+        On state change, the BMC will write its POST code to the<br>
+        character device. The POST code is then sent to update-postcard platform<br>
+        driver for display on the POST card<br>
+<br></blockquote><div><br></div></div></div><div>Again, not specific to Aspeed.  Maybe this should be SEVEN_SEGMENT_DISPLAY and the other driver should be SEVEN_SEGMENT_SGPIO.</div><div><div class="m_-7487688587987672970m_-7121693134944063424gmail-h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 source "drivers/misc/c2port/Kconfig"<br>
 source "drivers/misc/eeprom/Kconfig"<br>
 source "drivers/misc/cb710/Kconfig"<br>
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile<br>
index 730c6c2..dce20cb 100644<br>
--- a/drivers/misc/Makefile<br>
+++ b/drivers/misc/Makefile<br>
@@ -59,3 +59,4 @@ obj-$(CONFIG_CXL_BASE)                += cxl/<br>
 obj-$(CONFIG_PANEL)             += panel.o<br>
 obj-$(CONFIG_ASPEED_UPDATE_PO<wbr>STCARD)    += update-postcard.o<br>
 obj-$(CONFIG_ASPEED_BT_IPMI_H<wbr>OST)      += bt-host.o<br>
+obj-$(CONFIG_ASPEED_UPDATE_BM<wbr>C_POSTCODE)       += update-bmc-postcode.o<br>
diff --git a/drivers/misc/update-bmc-post<wbr>code.c b/drivers/misc/update-bmc-post<wbr>code.c<br>
new file mode 100644<br>
index 0000000..8e368a4<br>
--- /dev/null<br>
+++ b/drivers/misc/update-bmc-post<wbr>code.c<br>
@@ -0,0 +1,123 @@<br>
+/*<br>
+ * Character driver - On state change, the BMC will write its POST code to the<br>
+ * character device. The POST code is then sent to update-postcard platform<br>
+ * driver for display on the POST card. The BMC POST code consists of 3 digits.<br>
+ */<br>
+<br>
+#include <linux/module.h><br>
+#include <linux/version.h><br>
+#include <linux/kernel.h><br>
+#include <linux/types.h><br>
+#include <linux/kdev_t.h><br>
+#include <linux/fs.h><br>
+#include <linux/device.h><br>
+#include <linux/cdev.h><br>
+#include <linux/uaccess.h><br>
+#include <linux/ctype.h><br>
+<br>
+#include "update-postcard.h"<br>
+<br>
+#define MAX_POSTCODE_SIZE 3<br>
+<br>
+static dev_t bmc_state_dev;<br>
+static struct cdev c_dev;<br>
+static struct class *bmc_state_class;<br>
+<br>
+static int bmc_state_open(struct inode *i, struct file *f)<br>
+{<br>
+       return 0;<br>
+}<br>
+<br>
+static int bmc_state_close(struct inode *i, struct file *f)<br>
+{<br>
+       return 0;<br>
+}<br>
+<br>
+static ssize_t bmc_state_read(struct file *f, char __user *buf, size_t<br>
+                               len, loff_t *off)<br>
+{<br>
+       return 0;<br>
+}<br>
+<br>
+/* Write post code from bmc to the correct variable */<br>
+static ssize_t bmc_state_write(struct file *f, const char __user *buf,<br>
+                               size_t len, loff_t *off)<br>
+{<br>
+       char tmp[MAX_POSTCODE_SIZE];<br>
+       int length = len - 1;<br>
+       int i;<br>
+<br>
+       if (length != MAX_POSTCODE_SIZE) {<br>
+               return -EINVAL;<br>
+       }<br>
+<br>
+       if (copy_from_user(tmp, buf, length) != 0) {<br>
+               return -EFAULT;<br>
+       }<br>
+<br>
+       for (i = 0; i < MAX_POSTCODE_SIZE; i++) {<br>
+               if (!isxdigit(tmp[i]))<br>
+                       return -EINVAL;<br>
+       }<br>
+<br>
+       update_postcode(tmp);<br></blockquote><div><br></div></div></div><div>As written, this is hard-coded to the bit-bang driver in your other patch.  The concept is fairly generic though.  If we switched to using a 74HC165 that is compatible with the Aspeed's built-in SGPIO hardware, ideally we'd only need to write a driver for the SGPIO.  That implies you want this character device to attach to any compatible display drivers.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="m_-7487688587987672970m_-7121693134944063424gmail-h5">
+       return len;<br>
+}<br>
+<br>
+static const struct file_operations bmc_state_fops = {<br>
+<br>
+       .owner = THIS_MODULE,<br>
+       .open = bmc_state_open,<br>
+       .release = bmc_state_close,<br>
+       .read = bmc_state_read,<br>
+       .write = bmc_state_write<br>
+};<br>
+<br>
+static int __init update_bmc_pc_init(void)<br>
+{<br>
+       if (alloc_chrdev_region(&bmc_stat<wbr>e_dev, 0, 1, "bmc_state") < 0) {<br>
+               return -1;<br>
+       }<br>
+<br>
+       bmc_state_class = class_create(THIS_MODULE, "bmc_state");<br>
+       if (bmc_state_class == NULL) {<br>
+               goto unregister;<br>
+               return -1;<br>
+       }<br>
+<br>
+       if (device_create(bmc_state_class<wbr>, NULL, bmc_state_dev,<br>
+               NULL, "current_state") == NULL) {<br>
+               goto class_destroy;<br>
+               return -1;<br>
+       }<br>
+<br>
+       cdev_init(&c_dev, &bmc_state_fops);<br>
+       if (cdev_add(&c_dev, bmc_state_dev, 1) == -1) {<br>
+               goto device_destroy;<br>
+               return -1;<br>
+       }<br>
+<br>
+       return 0;<br>
+<br>
+device_destroy:<br>
+               device_destroy(bmc_state_clas<wbr>s, bmc_state_dev);<br>
+class_destroy:<br>
+               class_destroy(bmc_state_class<wbr>);<br>
+unregister:<br>
+               unregister_chrdev_region(bmc_<wbr>state_dev, 1);<br>
+       return -1;<br>
+}<br>
+<br>
+static void __exit update_bmc_pc_exit(void)<br>
+{<br>
+       cdev_del(&c_dev);<br>
+       device_destroy(bmc_state_clas<wbr>s, bmc_state_dev);<br>
+       class_destroy(bmc_state_class<wbr>);<br>
+       unregister_chrdev_region(bmc_<wbr>state_dev, 1);<br>
+}<br>
+<br>
+module_init(update_bmc_pc_ini<wbr>t);<br>
+module_exit(update_bmc_pc_exi<wbr>t);<br>
+MODULE_LICENSE("GPL");<br>
+MODULE_AUTHOR("Jaghathiswari Rankappagounder Natarajan <<a href="mailto:jaghu@google.com" target="_blank">jaghu@google.com</a>>");<br>
+MODULE_DESCRIPTION("Character driver - update bmc postcode");<br>
--<br>
2.8.0.rc3.226.g39d4020<br>
<br></div></div>
______________________________<wbr>_________________<br>
openbmc mailing list<br>
<a href="mailto:openbmc@lists.ozlabs.org" target="_blank">openbmc@lists.ozlabs.org</a><br>
<a href="https://lists.ozlabs.org/listinfo/openbmc" rel="noreferrer" target="_blank">https://lists.ozlabs.org/listi<wbr>nfo/openbmc</a><br>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div></div>