<html><body><p>Having the driver handle the enable is better.   Looks good to me.<br><br><br>Regards,<br>Norman James<br>IBM - POWER Systems Architect<br>Phone: 1-512-286-6807 (T/L: 363-6807)<br>Internet: njames@us.ibm.com<br><br><br><img width="16" height="16" src="cid:1__=09BBF5C4DF8AB39E8f9e8a93df938690918c09B@" border="0" alt="Inactive hide details for Jeremy Kerr ---02/11/2016 03:03:22 AM---Rather than exposing an enable sysfs attribute, we can just s"><font color="#424282">Jeremy Kerr ---02/11/2016 03:03:22 AM---Rather than exposing an enable sysfs attribute, we can just set the VUART enabled bit when we bind t</font><br><br><font size="2" color="#5F5F5F">From:        </font><font size="2">Jeremy Kerr <jk@ozlabs.org></font><br><font size="2" color="#5F5F5F">To:        </font><font size="2">openbmc@lists.ozlabs.org</font><br><font size="2" color="#5F5F5F">Date:        </font><font size="2">02/11/2016 03:03 AM</font><br><font size="2" color="#5F5F5F">Subject:        </font><font size="2">[RFC PATCH 2/2] serial/aspeed-vuart: Perform enable/disable on driver bind/unbind</font><br><font size="2" color="#5F5F5F">Sent by:        </font><font size="2">"openbmc" <openbmc-bounces+njames=us.ibm.com@lists.ozlabs.org></font><br><hr width="100%" size="2" align="left" noshade style="color:#8091A5; "><br><br><br><tt>Rather than exposing an enable sysfs attribute, we can just set the<br>VUART enabled bit when we bind to the device, and clear it on unbind.<br><br>We don't want to do this on open/release, as the host may be using this<br>bit to configure serial output modes, which is independent of whether<br>the devices has been opened by BMC userspace.<br><br>Signed-off-by: Jeremy Kerr <jk@ozlabs.org></tt><br><tt>Reviewed-by:  Norman James <nkskjames@gmail.com><br>---<br> drivers/tty/serial/aspeed-vuart.c | 53 +++++++++++----------------------------<br> 1 file changed, 14 insertions(+), 39 deletions(-)<br><br>diff --git a/drivers/tty/serial/aspeed-vuart.c b/drivers/tty/serial/aspeed-vuart.c<br>index 73bb0e7..020c815 100644<br>--- a/drivers/tty/serial/aspeed-vuart.c<br>+++ b/drivers/tty/serial/aspeed-vuart.c<br>@@ -35,42 +35,6 @@ struct ast_vuart {<br>                  int                                                   line;<br> };<br> <br>-static ssize_t ast_vuart_show_enabled(struct device *dev,<br>-                                  struct device_attribute *attr, char *buf)<br>-{<br>-                 struct ast_vuart *vuart = dev_get_drvdata(dev);<br>-                 u8 reg;<br>-<br>-                 reg = readb(vuart->regs + AST_VUART_GCRA) & AST_VUART_GCRA_VUART_EN;<br>-<br>-                 return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg ? 1 : 0);<br>-}<br>-<br>-static ssize_t ast_vuart_set_enabled(struct device *dev,<br>-                                  struct device_attribute *attr,<br>-                                  const char *buf, size_t count)<br>-{<br>-                 struct ast_vuart *vuart = dev_get_drvdata(dev);<br>-                 unsigned long val;<br>-                 int err;<br>-                 u8 reg;<br>-<br>-                 err = kstrtoul(buf, 0, &val);<br>-                 if (err)<br>-                                  return err;<br>-<br>-                 reg = readb(vuart->regs + AST_VUART_GCRA);<br>-                 reg &= ~AST_VUART_GCRA_VUART_EN;<br>-                 if (val)<br>-                                  reg |= AST_VUART_GCRA_VUART_EN;<br>-                 writeb(reg, vuart->regs + AST_VUART_GCRA);<br>-<br>-                 return count;<br>-}<br>-<br>-static DEVICE_ATTR(enabled, S_IWUSR | S_IRUGO,<br>-                                  ast_vuart_show_enabled, ast_vuart_set_enabled);<br>-<br> static ssize_t ast_vuart_show_addr(struct device *dev,<br>                                   struct device_attribute *attr, char *buf)<br> {<br>@@ -144,6 +108,17 @@ static ssize_t ast_vuart_set_sirq(struct device *dev,<br> static DEVICE_ATTR(sirq, S_IWUSR | S_IRUGO,<br>                                   ast_vuart_show_sirq, ast_vuart_set_sirq);<br> <br>+static void ast_vuart_set_enabled(struct ast_vuart *vuart, bool enabled)<br>+{<br>+                 u8 reg;<br>+<br>+                 reg = readb(vuart->regs + AST_VUART_GCRA);<br>+                 reg &= ~AST_VUART_GCRA_VUART_EN;<br>+                 if (enabled)<br>+                                  reg |= AST_VUART_GCRA_VUART_EN;<br>+                 writeb(reg, vuart->regs + AST_VUART_GCRA);<br>+}<br>+<br> static void ast_vuart_set_host_tx_discard(struct ast_vuart *vuart, bool discard)<br> {<br>                  u8 reg;<br>@@ -304,6 +279,7 @@ static int ast_vuart_probe(struct platform_device *pdev)<br> <br> <br>                  vuart->line = rc;<br>+                 ast_vuart_set_enabled(vuart, true);<br>                  ast_vuart_set_host_tx_discard(vuart, true);<br>                  platform_set_drvdata(pdev, vuart);<br> <br>@@ -314,9 +290,6 @@ static int ast_vuart_probe(struct platform_device *pdev)<br>                  rc = device_create_file(&pdev->dev, &dev_attr_sirq);<br>                  if (rc)<br>                                   dev_warn(&pdev->dev, "can't create sirq file\n");<br>-                 rc = device_create_file(&pdev->dev, &dev_attr_enabled);<br>-                 if (rc)<br>-                                  dev_warn(&pdev->dev, "can't create enabled file\n");<br> <br>                  return 0;<br> <br>@@ -332,6 +305,8 @@ static int ast_vuart_remove(struct platform_device *pdev)<br> {<br>                  struct ast_vuart *vuart = platform_get_drvdata(pdev);<br> <br>+                 ast_vuart_set_enabled(vuart, false);<br>+<br>                  if (vuart->clk)<br>                                   clk_disable_unprepare(vuart->clk);<br>                  return 0;<br>-- <br>2.5.0<br><br>_______________________________________________<br>openbmc mailing list<br>openbmc@lists.ozlabs.org<br></tt><tt><a href="https://lists.ozlabs.org/listinfo/openbmc">https://lists.ozlabs.org/listinfo/openbmc</a></tt><tt><br></tt><br><BR>
</body></html>