<br><br>On Thursday, April 8, 2021, Zev Weiss <<a href="mailto:zev@bewilderbeest.net">zev@bewilderbeest.net</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This splits dedicated aspeed_vuart_set_{sirq,lpc_<wbr>address}() functions<br>
out of the sysfs store functions in preparation for adding DT<br>
properties that will be poking the same registers.  While we're at it,<br>
these functions now provide some basic bounds-checking on their<br>
arguments.<br>
<br></blockquote><div><br></div><div><br></div><div>Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in the subject line. I think I have told this already </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Signed-off-by: Zev Weiss <<a href="mailto:zev@bewilderbeest.net">zev@bewilderbeest.net</a>><br>
---<br>
 drivers/tty/serial/8250/8250_<wbr>aspeed_vuart.c | 51 ++++++++++++++-------<br>
 1 file changed, 35 insertions(+), 16 deletions(-)<br>
<br>
diff --git a/drivers/tty/serial/8250/<wbr>8250_aspeed_vuart.c b/drivers/tty/serial/8250/<wbr>8250_aspeed_vuart.c<br>
index c33e02cbde93..8433f8dbb186 100644<br>
--- a/drivers/tty/serial/8250/<wbr>8250_aspeed_vuart.c<br>
+++ b/drivers/tty/serial/8250/<wbr>8250_aspeed_vuart.c<br>
@@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,<br>
        return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);<br>
 }<br>
<br>
+static int aspeed_vuart_set_lpc_address(<wbr>struct aspeed_vuart *vuart, u32 addr)<br>
+{<br>
+       if (addr > U16_MAX)<br>
+               return -EINVAL;<br>
+<br>
+       writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);<br>
+       writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
 static ssize_t lpc_address_store(struct device *dev,<br>
                                 struct device_attribute *attr,<br>
                                 const char *buf, size_t count)<br>
 {<br>
        struct aspeed_vuart *vuart = dev_get_drvdata(dev);<br>
-       unsigned long val;<br>
+       u32 val;<br>
        int err;<br>
<br>
-       err = kstrtoul(buf, 0, &val);<br>
+       err = kstrtou32(buf, 0, &val);<br>
        if (err)<br>
                return err;<br>
<br>
-       writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);<br>
-       writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);<br>
-<br>
-       return count;<br>
+       err = aspeed_vuart_set_lpc_address(<wbr>vuart, val);<br>
+       return err ? : count;<br>
 }<br>
<br>
 static DEVICE_ATTR_RW(lpc_address);<br>
@@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,<br>
        return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);<br>
 }<br>
<br>
+static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)<br>
+{<br>
+       u8 reg;<br>
+<br>
+       if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>SHIFT))<br>
+               return -EINVAL;<br>
+<br>
+       sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>SHIFT;<br>
+       sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>MASK;<br>
+<br>
+       reg = readb(vuart->regs + ASPEED_VUART_GCRB);<br>
+       reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>MASK;<br>
+       reg |= sirq;<br>
+       writeb(reg, vuart->regs + ASPEED_VUART_GCRB);<br>
+<br>
+       return 0;<br>
+}<br>
+<br>
 static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,<br>
                          const char *buf, size_t count)<br>
 {<br>
        struct aspeed_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>
-       val <<= ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>SHIFT;<br>
-       val &= ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>MASK;<br>
-<br>
-       reg = readb(vuart->regs + ASPEED_VUART_GCRB);<br>
-       reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_<wbr>MASK;<br>
-       reg |= val;<br>
-       writeb(reg, vuart->regs + ASPEED_VUART_GCRB);<br>
-<br>
-       return count;<br>
+       err = aspeed_vuart_set_sirq(vuart, val);<br>
+       return err ? : count;<br>
 }<br>
<br>
 static DEVICE_ATTR_RW(sirq);<br>
-- <br>
2.31.1<br>
<br>
</blockquote><br><br>-- <br>With Best Regards,<br>Andy Shevchenko<br><br><br>