* [PATCH v5 0/4] serial: 8250_aspeed_vuart: generalized DT properties
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Jiri Slaby,
Lubomir Rintel, Rob Herring, devicetree, linux-serial
This series generalizes the aspeed-vuart driver's device tree
properties to cover all the attributes it currently exposes via sysfs.
The aspeed,sirq-polarity-sense property was a bit of a design mistake
in that it ties Aspeed VUART SIRQ polarity to SCU register bits that
aren't really inherently related to it; the first patch in this series
deprecates it (though we hope to eventually remove it).
The rest of the series adds two new properties, aspeed,lpc-io-reg and
aspeed,lpc-interrupts. The latter allows describing the SIRQ polarity
(along with the interrupt number) directly, providing a simpler
replacement for aspeed,sirq-polarity-sense.
Changes since v4 [3]:
- fixed commit reference formatting in commit message
Changes since v3 [2]:
- renamed properties to match aspeed,ast2400-kcs-bmc
Changes since v2 [0]:
- expanded to also handle sirq number and lpc address in addition to
sirq polarity
- added default settings if DT properties not specified
- refactored existing sysfs code slightly, adding range checks
- cleaned up 'make dt_binding_check' warnings
Changes since v1 [1]:
- deprecate and retain aspeed,sirq-polarity-sense instead of removing it
- drop e3c246d4i dts addition from this series
[0] https://lore.kernel.org/openbmc/20210401005702.28271-1-zev@bewilderbeest.net/
[1] https://lore.kernel.org/openbmc/20210330002338.335-1-zev@bewilderbeest.net/
[2] https://lore.kernel.org/openbmc/20210402004716.15961-1-zev@bewilderbeest.net/
[3] https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/
Zev Weiss (4):
dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense
drivers/tty/serial/8250: refactor sirq and lpc address setting code
drivers/tty/serial/8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts DT properties
dt-bindings: serial: 8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts
.../devicetree/bindings/serial/8250.yaml | 28 +++++-
drivers/tty/serial/8250/8250_aspeed_vuart.c | 95 +++++++++++++++----
2 files changed, 103 insertions(+), 20 deletions(-)
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH v5 0/4] serial: 8250_aspeed_vuart: generalized DT properties
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: devicetree, linux-aspeed, Zev Weiss, Andrew Jeffery,
Greg Kroah-Hartman, openbmc, linux-kernel, Lubomir Rintel,
Rob Herring, linux-serial, Jiri Slaby, linux-arm-kernel
This series generalizes the aspeed-vuart driver's device tree
properties to cover all the attributes it currently exposes via sysfs.
The aspeed,sirq-polarity-sense property was a bit of a design mistake
in that it ties Aspeed VUART SIRQ polarity to SCU register bits that
aren't really inherently related to it; the first patch in this series
deprecates it (though we hope to eventually remove it).
The rest of the series adds two new properties, aspeed,lpc-io-reg and
aspeed,lpc-interrupts. The latter allows describing the SIRQ polarity
(along with the interrupt number) directly, providing a simpler
replacement for aspeed,sirq-polarity-sense.
Changes since v4 [3]:
- fixed commit reference formatting in commit message
Changes since v3 [2]:
- renamed properties to match aspeed,ast2400-kcs-bmc
Changes since v2 [0]:
- expanded to also handle sirq number and lpc address in addition to
sirq polarity
- added default settings if DT properties not specified
- refactored existing sysfs code slightly, adding range checks
- cleaned up 'make dt_binding_check' warnings
Changes since v1 [1]:
- deprecate and retain aspeed,sirq-polarity-sense instead of removing it
- drop e3c246d4i dts addition from this series
[0] https://lore.kernel.org/openbmc/20210401005702.28271-1-zev@bewilderbeest.net/
[1] https://lore.kernel.org/openbmc/20210330002338.335-1-zev@bewilderbeest.net/
[2] https://lore.kernel.org/openbmc/20210402004716.15961-1-zev@bewilderbeest.net/
[3] https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/
Zev Weiss (4):
dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense
drivers/tty/serial/8250: refactor sirq and lpc address setting code
drivers/tty/serial/8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts DT properties
dt-bindings: serial: 8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts
.../devicetree/bindings/serial/8250.yaml | 28 +++++-
drivers/tty/serial/8250/8250_aspeed_vuart.c | 95 +++++++++++++++----
2 files changed, 103 insertions(+), 20 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 1:16 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Rob Herring,
Lubomir Rintel, -,
linux-serial
This property ties SIRQ polarity to SCU register bits that don't
necessarily have any direct relationship to it; the only use of it was
removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
polarity").
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
Documentation/devicetree/bindings/serial/8250.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..491b9297432d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -188,6 +188,7 @@ properties:
offset and bit number to identify how the SIRQ polarity should be
configured. One possible data source is the LPC/eSPI mode bit. Only
applicable to aspeed,ast2500-vuart.
+ deprecated: true
required:
- reg
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Rob Herring,
Lubomir Rintel, -,
linux-serial
This property ties SIRQ polarity to SCU register bits that don't
necessarily have any direct relationship to it; the only use of it was
removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
polarity").
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
Documentation/devicetree/bindings/serial/8250.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..491b9297432d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -188,6 +188,7 @@ properties:
offset and bit number to identify how the SIRQ polarity should be
configured. One possible data source is the LPC/eSPI mode bit. Only
applicable to aspeed,ast2500-vuart.
+ deprecated: true
required:
- reg
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: -,
linux-aspeed, Zev Weiss, Andrew Jeffery, Greg Kroah-Hartman,
openbmc, linux-kernel, Lubomir Rintel, Rob Herring, linux-serial,
linux-arm-kernel
This property ties SIRQ polarity to SCU register bits that don't
necessarily have any direct relationship to it; the only use of it was
removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
polarity").
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
Documentation/devicetree/bindings/serial/8250.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..491b9297432d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -188,6 +188,7 @@ properties:
offset and bit number to identify how the SIRQ polarity should be
configured. One possible data source is the LPC/eSPI mode bit. Only
applicable to aspeed,ast2500-vuart.
+ deprecated: true
required:
- reg
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 15:59 ` Rob Herring
-1 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 15:59 UTC (permalink / raw)
To: Zev Weiss
Cc: Lubomir Rintel, Greg Kroah-Hartman, linux-arm-kernel,
linux-serial, Rob Herring, Andrew Jeffery, openbmc, linux-aspeed,
-,
Joel Stanley, linux-kernel
On Wed, 07 Apr 2021 20:16:34 -0500, Zev Weiss wrote:
> This property ties SIRQ polarity to SCU register bits that don't
> necessarily have any direct relationship to it; the only use of it was
> removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
> polarity").
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> Documentation/devicetree/bindings/serial/8250.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
@ 2021-04-08 15:59 ` Rob Herring
0 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 15:59 UTC (permalink / raw)
To: Zev Weiss
Cc: Lubomir Rintel, Greg Kroah-Hartman, linux-arm-kernel,
linux-serial, Rob Herring, Andrew Jeffery, openbmc, linux-aspeed,
-,
Joel Stanley, linux-kernel
On Wed, 07 Apr 2021 20:16:34 -0500, Zev Weiss wrote:
> This property ties SIRQ polarity to SCU register bits that don't
> necessarily have any direct relationship to it; the only use of it was
> removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
> polarity").
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> Documentation/devicetree/bindings/serial/8250.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
@ 2021-04-08 15:59 ` Rob Herring
0 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 15:59 UTC (permalink / raw)
To: Zev Weiss
Cc: -,
linux-aspeed, Andrew Jeffery, Greg Kroah-Hartman, openbmc,
linux-kernel, Lubomir Rintel, Rob Herring, linux-serial,
linux-arm-kernel
On Wed, 07 Apr 2021 20:16:34 -0500, Zev Weiss wrote:
> This property ties SIRQ polarity to SCU register bits that don't
> necessarily have any direct relationship to it; the only use of it was
> removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
> polarity").
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> Documentation/devicetree/bindings/serial/8250.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 1:16 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
out of the sysfs store functions in preparation for adding DT
properties that will be poking the same registers. While we're at it,
these functions now provide some basic bounds-checking on their
arguments.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index c33e02cbde93..8433f8dbb186 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
}
+static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
+{
+ if (addr > U16_MAX)
+ return -EINVAL;
+
+ writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return 0;
+}
+
static ssize_t lpc_address_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
- unsigned long val;
+ u32 val;
int err;
- err = kstrtoul(buf, 0, &val);
+ err = kstrtou32(buf, 0, &val);
if (err)
return err;
- writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
- writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
-
- return count;
+ err = aspeed_vuart_set_lpc_address(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(lpc_address);
@@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
}
+static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
+{
+ u8 reg;
+
+ if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
+ return -EINVAL;
+
+ sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= sirq;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return 0;
+}
+
static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
unsigned long val;
int err;
- u8 reg;
err = kstrtoul(buf, 0, &val);
if (err)
return err;
- val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
- val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
-
- reg = readb(vuart->regs + ASPEED_VUART_GCRB);
- reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
- reg |= val;
- writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
-
- return count;
+ err = aspeed_vuart_set_sirq(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(sirq);
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
out of the sysfs store functions in preparation for adding DT
properties that will be poking the same registers. While we're at it,
these functions now provide some basic bounds-checking on their
arguments.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index c33e02cbde93..8433f8dbb186 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
}
+static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
+{
+ if (addr > U16_MAX)
+ return -EINVAL;
+
+ writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return 0;
+}
+
static ssize_t lpc_address_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
- unsigned long val;
+ u32 val;
int err;
- err = kstrtoul(buf, 0, &val);
+ err = kstrtou32(buf, 0, &val);
if (err)
return err;
- writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
- writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
-
- return count;
+ err = aspeed_vuart_set_lpc_address(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(lpc_address);
@@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
}
+static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
+{
+ u8 reg;
+
+ if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
+ return -EINVAL;
+
+ sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= sirq;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return 0;
+}
+
static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
unsigned long val;
int err;
- u8 reg;
err = kstrtoul(buf, 0, &val);
if (err)
return err;
- val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
- val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
-
- reg = readb(vuart->regs + ASPEED_VUART_GCRB);
- reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
- reg |= val;
- writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
-
- return count;
+ err = aspeed_vuart_set_sirq(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(sirq);
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: linux-aspeed, Zev Weiss, Andrew Jeffery, Greg Kroah-Hartman,
openbmc, linux-kernel, linux-serial, Jiri Slaby,
linux-arm-kernel
This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
out of the sysfs store functions in preparation for adding DT
properties that will be poking the same registers. While we're at it,
these functions now provide some basic bounds-checking on their
arguments.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index c33e02cbde93..8433f8dbb186 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
}
+static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
+{
+ if (addr > U16_MAX)
+ return -EINVAL;
+
+ writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
+ writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
+
+ return 0;
+}
+
static ssize_t lpc_address_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
- unsigned long val;
+ u32 val;
int err;
- err = kstrtoul(buf, 0, &val);
+ err = kstrtou32(buf, 0, &val);
if (err)
return err;
- writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
- writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
-
- return count;
+ err = aspeed_vuart_set_lpc_address(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(lpc_address);
@@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
}
+static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
+{
+ u8 reg;
+
+ if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
+ return -EINVAL;
+
+ sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
+ sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+
+ reg = readb(vuart->regs + ASPEED_VUART_GCRB);
+ reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
+ reg |= sirq;
+ writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
+
+ return 0;
+}
+
static ssize_t sirq_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct aspeed_vuart *vuart = dev_get_drvdata(dev);
unsigned long val;
int err;
- u8 reg;
err = kstrtoul(buf, 0, &val);
if (err)
return err;
- val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
- val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
-
- reg = readb(vuart->regs + ASPEED_VUART_GCRB);
- reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
- reg |= val;
- writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
-
- return count;
+ err = aspeed_vuart_set_sirq(vuart, val);
+ return err ? : count;
}
static DEVICE_ATTR_RW(sirq);
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-09 5:06 ` Andrew Jeffery
-1 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:06 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Greg Kroah-Hartman, Jiri Slaby, linux-serial
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> out of the sysfs store functions in preparation for adding DT
> properties that will be poking the same registers. While we're at it,
> these functions now provide some basic bounds-checking on their
> arguments.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
> 1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index c33e02cbde93..8433f8dbb186 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> }
>
> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
> +{
> + if (addr > U16_MAX)
> + return -EINVAL;
> +
> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return 0;
> +}
> +
> static ssize_t lpc_address_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> - unsigned long val;
> + u32 val;
> int err;
>
> - err = kstrtoul(buf, 0, &val);
> + err = kstrtou32(buf, 0, &val);
> if (err)
> return err;
>
> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> -
> - return count;
> + err = aspeed_vuart_set_lpc_address(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(lpc_address);
> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> }
>
> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
> +{
> + u8 reg;
> +
> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
> + return -EINVAL;
> +
> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
This might be less verbose if we reordered things a little:
```
sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
return -EINVAL;
sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
```
But otherwise it looks okay, so
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= sirq;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return 0;
> +}
> +
> static ssize_t sirq_store(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> unsigned long val;
> int err;
> - u8 reg;
>
> err = kstrtoul(buf, 0, &val);
> if (err)
> return err;
>
> - val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> - val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> -
> - reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> - reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> - reg |= val;
> - writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> -
> - return count;
> + err = aspeed_vuart_set_sirq(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(sirq);
> --
> 2.31.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 5:06 ` Andrew Jeffery
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:06 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Greg Kroah-Hartman, Jiri Slaby, linux-serial
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> out of the sysfs store functions in preparation for adding DT
> properties that will be poking the same registers. While we're at it,
> these functions now provide some basic bounds-checking on their
> arguments.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
> 1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index c33e02cbde93..8433f8dbb186 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> }
>
> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
> +{
> + if (addr > U16_MAX)
> + return -EINVAL;
> +
> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return 0;
> +}
> +
> static ssize_t lpc_address_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> - unsigned long val;
> + u32 val;
> int err;
>
> - err = kstrtoul(buf, 0, &val);
> + err = kstrtou32(buf, 0, &val);
> if (err)
> return err;
>
> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> -
> - return count;
> + err = aspeed_vuart_set_lpc_address(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(lpc_address);
> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> }
>
> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
> +{
> + u8 reg;
> +
> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
> + return -EINVAL;
> +
> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
This might be less verbose if we reordered things a little:
```
sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
return -EINVAL;
sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
```
But otherwise it looks okay, so
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= sirq;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return 0;
> +}
> +
> static ssize_t sirq_store(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> unsigned long val;
> int err;
> - u8 reg;
>
> err = kstrtoul(buf, 0, &val);
> if (err)
> return err;
>
> - val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> - val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> -
> - reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> - reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> - reg |= val;
> - writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> -
> - return count;
> + err = aspeed_vuart_set_sirq(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(sirq);
> --
> 2.31.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 5:06 ` Andrew Jeffery
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:06 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> out of the sysfs store functions in preparation for adding DT
> properties that will be poking the same registers. While we're at it,
> these functions now provide some basic bounds-checking on their
> arguments.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
> 1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index c33e02cbde93..8433f8dbb186 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> }
>
> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
> +{
> + if (addr > U16_MAX)
> + return -EINVAL;
> +
> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return 0;
> +}
> +
> static ssize_t lpc_address_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> - unsigned long val;
> + u32 val;
> int err;
>
> - err = kstrtoul(buf, 0, &val);
> + err = kstrtou32(buf, 0, &val);
> if (err)
> return err;
>
> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> -
> - return count;
> + err = aspeed_vuart_set_lpc_address(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(lpc_address);
> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> }
>
> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
> +{
> + u8 reg;
> +
> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
> + return -EINVAL;
> +
> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
This might be less verbose if we reordered things a little:
```
sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
return -EINVAL;
sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
```
But otherwise it looks okay, so
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= sirq;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return 0;
> +}
> +
> static ssize_t sirq_store(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> unsigned long val;
> int err;
> - u8 reg;
>
> err = kstrtoul(buf, 0, &val);
> if (err)
> return err;
>
> - val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> - val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> -
> - reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> - reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> - reg |= val;
> - writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> -
> - return count;
> + err = aspeed_vuart_set_sirq(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(sirq);
> --
> 2.31.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-09 5:06 ` Andrew Jeffery
(?)
@ 2021-04-09 7:01 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:01 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-serial
On Fri, Apr 09, 2021 at 12:06:16AM CDT, Andrew Jeffery wrote:
>
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
>> 1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index c33e02cbde93..8433f8dbb186 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
>> }
>>
>> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
>> +{
>> + if (addr > U16_MAX)
>> + return -EINVAL;
>> +
>> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> +
>> + return 0;
>> +}
>> +
>> static ssize_t lpc_address_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
>> - unsigned long val;
>> + u32 val;
>> int err;
>>
>> - err = kstrtoul(buf, 0, &val);
>> + err = kstrtou32(buf, 0, &val);
>> if (err)
>> return err;
>>
>> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> -
>> - return count;
>> + err = aspeed_vuart_set_lpc_address(vuart, val);
>> + return err ? : count;
>> }
>>
>> static DEVICE_ATTR_RW(lpc_address);
>> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
>> }
>>
>> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
>> +{
>> + u8 reg;
>> +
>> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
>> + return -EINVAL;
>> +
>> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>
>This might be less verbose if we reordered things a little:
>
>```
>sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
> return -EINVAL;
>sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>```
Hmm, that (or something similar, perhaps with a '~' on the mask in the
if condition?) does seem like it'd be a nice improvement, though I
suppose it'd also mean we'd fail to reject some way-out-of-range sirq
values (e.g. if it had its MSB set) -- so I think I'll leave it as is,
just in the name of thoroughness/paranoia?
>
>But otherwise it looks okay, so
>
>Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 7:01 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:01 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Greg Kroah-Hartman, Jiri Slaby, linux-serial
On Fri, Apr 09, 2021 at 12:06:16AM CDT, Andrew Jeffery wrote:
>
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
>> 1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index c33e02cbde93..8433f8dbb186 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
>> }
>>
>> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
>> +{
>> + if (addr > U16_MAX)
>> + return -EINVAL;
>> +
>> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> +
>> + return 0;
>> +}
>> +
>> static ssize_t lpc_address_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
>> - unsigned long val;
>> + u32 val;
>> int err;
>>
>> - err = kstrtoul(buf, 0, &val);
>> + err = kstrtou32(buf, 0, &val);
>> if (err)
>> return err;
>>
>> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> -
>> - return count;
>> + err = aspeed_vuart_set_lpc_address(vuart, val);
>> + return err ? : count;
>> }
>>
>> static DEVICE_ATTR_RW(lpc_address);
>> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
>> }
>>
>> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
>> +{
>> + u8 reg;
>> +
>> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
>> + return -EINVAL;
>> +
>> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>
>This might be less verbose if we reordered things a little:
>
>```
>sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
> return -EINVAL;
>sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>```
Hmm, that (or something similar, perhaps with a '~' on the mask in the
if condition?) does seem like it'd be a nice improvement, though I
suppose it'd also mean we'd fail to reject some way-out-of-range sirq
values (e.g. if it had its MSB set) -- so I think I'll leave it as is,
just in the name of thoroughness/paranoia?
>
>But otherwise it looks okay, so
>
>Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
Thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 7:01 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:01 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 09, 2021 at 12:06:16AM CDT, Andrew Jeffery wrote:
>
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
>> 1 file changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index c33e02cbde93..8433f8dbb186 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
>> }
>>
>> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
>> +{
>> + if (addr > U16_MAX)
>> + return -EINVAL;
>> +
>> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> +
>> + return 0;
>> +}
>> +
>> static ssize_t lpc_address_store(struct device *dev,
>> struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
>> - unsigned long val;
>> + u32 val;
>> int err;
>>
>> - err = kstrtoul(buf, 0, &val);
>> + err = kstrtou32(buf, 0, &val);
>> if (err)
>> return err;
>>
>> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
>> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
>> -
>> - return count;
>> + err = aspeed_vuart_set_lpc_address(vuart, val);
>> + return err ? : count;
>> }
>>
>> static DEVICE_ATTR_RW(lpc_address);
>> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
>> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
>> }
>>
>> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
>> +{
>> + u8 reg;
>> +
>> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
>> + return -EINVAL;
>> +
>> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>
>This might be less verbose if we reordered things a little:
>
>```
>sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
>if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
> return -EINVAL;
>sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
>```
Hmm, that (or something similar, perhaps with a '~' on the mask in the
if condition?) does seem like it'd be a nice improvement, though I
suppose it'd also mean we'd fail to reject some way-out-of-range sirq
values (e.g. if it had its MSB set) -- so I think I'll leave it as is,
just in the name of thoroughness/paranoia?
>
>But otherwise it looks okay, so
>
>Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
>
Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-09 7:01 ` Zev Weiss
(?)
(?)
@ 2021-04-09 7:11 ` Andrew Jeffery
-1 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 7:11 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 9 Apr 2021, at 16:31, Zev Weiss wrote:
> On Fri, Apr 09, 2021 at 12:06:16AM CDT, Andrew Jeffery wrote:
> >
> >
> >On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> >> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> >> out of the sysfs store functions in preparation for adding DT
> >> properties that will be poking the same registers. While we're at it,
> >> these functions now provide some basic bounds-checking on their
> >> arguments.
> >>
> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> >> ---
> >> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
> >> 1 file changed, 35 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> >> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> >> index c33e02cbde93..8433f8dbb186 100644
> >> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> >> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> >> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
> >> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> >> }
> >>
> >> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr)
> >> +{
> >> + if (addr > U16_MAX)
> >> + return -EINVAL;
> >> +
> >> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> >> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static ssize_t lpc_address_store(struct device *dev,
> >> struct device_attribute *attr,
> >> const char *buf, size_t count)
> >> {
> >> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> >> - unsigned long val;
> >> + u32 val;
> >> int err;
> >>
> >> - err = kstrtoul(buf, 0, &val);
> >> + err = kstrtou32(buf, 0, &val);
> >> if (err)
> >> return err;
> >>
> >> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> >> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> >> -
> >> - return count;
> >> + err = aspeed_vuart_set_lpc_address(vuart, val);
> >> + return err ? : count;
> >> }
> >>
> >> static DEVICE_ATTR_RW(lpc_address);
> >> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
> >> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> >> }
> >>
> >> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
> >> +{
> >> + u8 reg;
> >> +
> >> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
> >> + return -EINVAL;
> >> +
> >> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> >> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> >
> >This might be less verbose if we reordered things a little:
> >
> >```
> >sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> >if (sirq & ASPEED_VUART_GCRB_HOST_SIRQ_MASK)
> > return -EINVAL;
> >sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> >```
>
> Hmm, that (or something similar, perhaps with a '~' on the mask in the
> if condition?) does seem like it'd be a nice improvement, though I
> suppose it'd also mean we'd fail to reject some way-out-of-range sirq
> values (e.g. if it had its MSB set) -- so I think I'll leave it as is,
> just in the name of thoroughness/paranoia?
Yeah, fair enough. I was considering smaller errors :)
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-08 1:16 ` Zev Weiss
` (2 preceding siblings ...)
(?)
@ 2021-04-09 7:24 ` Andy Shevchenko
2021-04-09 7:38 ` Zev Weiss
-1 siblings, 1 reply; 41+ messages in thread
From: Andy Shevchenko @ 2021-04-09 7:24 UTC (permalink / raw)
To: Zev Weiss
Cc: linux-aspeed, Andrew Jeffery, Greg Kroah-Hartman, openbmc,
linux-kernel, linux-serial, Jiri Slaby, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 3827 bytes --]
On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> out of the sysfs store functions in preparation for adding DT
> properties that will be poking the same registers. While we're at it,
> these functions now provide some basic bounds-checking on their
> arguments.
>
>
Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
the subject line. I think I have told this already
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 51 ++++++++++++++-------
> 1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index c33e02cbde93..8433f8dbb186 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -72,22 +72,31 @@ static ssize_t lpc_address_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> }
>
> +static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32
> addr)
> +{
> + if (addr > U16_MAX)
> + return -EINVAL;
> +
> + writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> + writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> +
> + return 0;
> +}
> +
> static ssize_t lpc_address_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> - unsigned long val;
> + u32 val;
> int err;
>
> - err = kstrtoul(buf, 0, &val);
> + err = kstrtou32(buf, 0, &val);
> if (err)
> return err;
>
> - writeb(val >> 8, vuart->regs + ASPEED_VUART_ADDRH);
> - writeb(val >> 0, vuart->regs + ASPEED_VUART_ADDRL);
> -
> - return count;
> + err = aspeed_vuart_set_lpc_address(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(lpc_address);
> @@ -105,27 +114,37 @@ static ssize_t sirq_show(struct device *dev,
> return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> }
>
> +static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq)
> +{
> + u8 reg;
> +
> + if (sirq > (ASPEED_VUART_GCRB_HOST_SIRQ_MASK >>
> ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT))
> + return -EINVAL;
> +
> + sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> + sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> +
> + reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> + reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= sirq;
> + writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> +
> + return 0;
> +}
> +
> static ssize_t sirq_store(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> struct aspeed_vuart *vuart = dev_get_drvdata(dev);
> unsigned long val;
> int err;
> - u8 reg;
>
> err = kstrtoul(buf, 0, &val);
> if (err)
> return err;
>
> - val <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT;
> - val &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> -
> - reg = readb(vuart->regs + ASPEED_VUART_GCRB);
> - reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK;
> - reg |= val;
> - writeb(reg, vuart->regs + ASPEED_VUART_GCRB);
> -
> - return count;
> + err = aspeed_vuart_set_sirq(vuart, val);
> + return err ? : count;
> }
>
> static DEVICE_ATTR_RW(sirq);
> --
> 2.31.1
>
>
--
With Best Regards,
Andy Shevchenko
[-- Attachment #2: Type: text/html, Size: 4898 bytes --]
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-09 7:24 ` Andy Shevchenko
2021-04-09 7:38 ` Zev Weiss
@ 2021-04-09 7:38 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Andrew Jeffery, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
>On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
>
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>>
>
>Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
>the subject line. I think I have told this already
>
>
Ah, sorry -- I fixed the cover letter after your first comment (which
had definitely been under-tagged); for the patches themselves I was
following the example of the last patch in that particular area
(8d310c9107a2), though I guess that wasn't the right model to follow.
I'll use the requested format in the future.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 7:38 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Andrew Jeffery, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
>On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
>
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>>
>
>Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
>the subject line. I think I have told this already
>
>
Ah, sorry -- I fixed the cover letter after your first comment (which
had definitely been under-tagged); for the patches themselves I was
following the example of the last patch in that particular area
(8d310c9107a2), though I guess that wasn't the right model to follow.
I'll use the requested format in the future.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 7:38 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 7:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-aspeed, Andrew Jeffery, Greg Kroah-Hartman, openbmc,
linux-kernel, linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
>On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
>
>> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
>> out of the sysfs store functions in preparation for adding DT
>> properties that will be poking the same registers. While we're at it,
>> these functions now provide some basic bounds-checking on their
>> arguments.
>>
>>
>
>Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
>the subject line. I think I have told this already
>
>
Ah, sorry -- I fixed the cover letter after your first comment (which
had definitely been under-tagged); for the patches themselves I was
following the example of the last patch in that particular area
(8d310c9107a2), though I guess that wasn't the right model to follow.
I'll use the requested format in the future.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
2021-04-09 7:38 ` Zev Weiss
(?)
@ 2021-04-09 9:51 ` Andy Shevchenko
-1 siblings, 0 replies; 41+ messages in thread
From: Andy Shevchenko @ 2021-04-09 9:51 UTC (permalink / raw)
To: Zev Weiss
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Andrew Jeffery, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
On Fri, Apr 9, 2021 at 10:38 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
> >On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> >> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> >> out of the sysfs store functions in preparation for adding DT
> >> properties that will be poking the same registers. While we're at it,
> >> these functions now provide some basic bounds-checking on their
> >> arguments.
> >>
> >>
> >
> >Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
> >the subject line. I think I have told this already
> >
> >
>
> Ah, sorry -- I fixed the cover letter after your first comment (which
> had definitely been under-tagged); for the patches themselves I was
> following the example of the last patch in that particular area
> (8d310c9107a2), though I guess that wasn't the right model to follow.
> I'll use the requested format in the future.
Just random amount of most recent patches against 8250 driver:
e47eb5241a8f serial: 8250: Avoid new transfers when shutting down
e49950d3e737 serial: 8250_dma: use linear buffer for transmit
34255381fabd serial: 8250_port: Try to run DMA Rx on timeout condition
7d7dec450a66 8250_tegra: clean up tegra_uart_handle_break
c3ae3dc896fa serial: 8250_pci: Drop bogus __refdata annotation
d96f04d347e4 serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access
6e4e636e0e3e serial: 8250-mtk: Fix reference leak in mtk8250_probe
a609c58086e3 tty: serial: 8250: 8250_port: Move prototypes to shared location
6f9918504129 serial: 8250: 8250_omap: Fix unused variable warning
d4548b14dd7e serial: 8250: 8250_omap: Fix possible array out of bounds access
912ab37c7987 serial: 8250_mtk: Fix uart_get_baud_rate warning
439c7183e5b9 serial: 8250: 8250_omap: Disable RX interrupt after DMA enable
32ed248042d1 tty: serial: 8250: serial_cs: Remove unused/unchecked
variable 'err'
85985a3dcd74 serial: 8250_dw: Fix clk-notifier/port suspend deadlock
c8dff3aa8241 serial: 8250: Skip uninitialized TTY port baud rate update
7718453e3696 serial: 8250: Discard RTS/DTS setting from clock update method
409cc4541ade serial: 8250_fsl: Fix TX interrupt handling condition
3c5a87be170a serial: 8250_pci: Add Realtek 816a and 816b
ea4de367e57d tty: serial: 8250_mtk: set regshift for mmio32
57cee0713118 serial: 8250_pci: Remove unused function get_pci_irq()
11361610b005 serial: 8250_fsl: Add ACPI support
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 9:51 ` Andy Shevchenko
0 siblings, 0 replies; 41+ messages in thread
From: Andy Shevchenko @ 2021-04-09 9:51 UTC (permalink / raw)
To: Zev Weiss
Cc: Joel Stanley, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel, Andrew Jeffery, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
On Fri, Apr 9, 2021 at 10:38 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
> >On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> >> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> >> out of the sysfs store functions in preparation for adding DT
> >> properties that will be poking the same registers. While we're at it,
> >> these functions now provide some basic bounds-checking on their
> >> arguments.
> >>
> >>
> >
> >Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
> >the subject line. I think I have told this already
> >
> >
>
> Ah, sorry -- I fixed the cover letter after your first comment (which
> had definitely been under-tagged); for the patches themselves I was
> following the example of the last patch in that particular area
> (8d310c9107a2), though I guess that wasn't the right model to follow.
> I'll use the requested format in the future.
Just random amount of most recent patches against 8250 driver:
e47eb5241a8f serial: 8250: Avoid new transfers when shutting down
e49950d3e737 serial: 8250_dma: use linear buffer for transmit
34255381fabd serial: 8250_port: Try to run DMA Rx on timeout condition
7d7dec450a66 8250_tegra: clean up tegra_uart_handle_break
c3ae3dc896fa serial: 8250_pci: Drop bogus __refdata annotation
d96f04d347e4 serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access
6e4e636e0e3e serial: 8250-mtk: Fix reference leak in mtk8250_probe
a609c58086e3 tty: serial: 8250: 8250_port: Move prototypes to shared location
6f9918504129 serial: 8250: 8250_omap: Fix unused variable warning
d4548b14dd7e serial: 8250: 8250_omap: Fix possible array out of bounds access
912ab37c7987 serial: 8250_mtk: Fix uart_get_baud_rate warning
439c7183e5b9 serial: 8250: 8250_omap: Disable RX interrupt after DMA enable
32ed248042d1 tty: serial: 8250: serial_cs: Remove unused/unchecked
variable 'err'
85985a3dcd74 serial: 8250_dw: Fix clk-notifier/port suspend deadlock
c8dff3aa8241 serial: 8250: Skip uninitialized TTY port baud rate update
7718453e3696 serial: 8250: Discard RTS/DTS setting from clock update method
409cc4541ade serial: 8250_fsl: Fix TX interrupt handling condition
3c5a87be170a serial: 8250_pci: Add Realtek 816a and 816b
ea4de367e57d tty: serial: 8250_mtk: set regshift for mmio32
57cee0713118 serial: 8250_pci: Remove unused function get_pci_irq()
11361610b005 serial: 8250_fsl: Add ACPI support
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
@ 2021-04-09 9:51 ` Andy Shevchenko
0 siblings, 0 replies; 41+ messages in thread
From: Andy Shevchenko @ 2021-04-09 9:51 UTC (permalink / raw)
To: Zev Weiss
Cc: linux-aspeed, Andrew Jeffery, Greg Kroah-Hartman, openbmc,
linux-kernel, linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 9, 2021 at 10:38 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> On Fri, Apr 09, 2021 at 02:24:08AM CDT, Andy Shevchenko wrote:
> >On Thursday, April 8, 2021, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> >> This splits dedicated aspeed_vuart_set_{sirq,lpc_address}() functions
> >> out of the sysfs store functions in preparation for adding DT
> >> properties that will be poking the same registers. While we're at it,
> >> these functions now provide some basic bounds-checking on their
> >> arguments.
> >>
> >>
> >
> >Please, use prefix “serial: 8250_aspeed_vuart:” instead of what you have in
> >the subject line. I think I have told this already
> >
> >
>
> Ah, sorry -- I fixed the cover letter after your first comment (which
> had definitely been under-tagged); for the patches themselves I was
> following the example of the last patch in that particular area
> (8d310c9107a2), though I guess that wasn't the right model to follow.
> I'll use the requested format in the future.
Just random amount of most recent patches against 8250 driver:
e47eb5241a8f serial: 8250: Avoid new transfers when shutting down
e49950d3e737 serial: 8250_dma: use linear buffer for transmit
34255381fabd serial: 8250_port: Try to run DMA Rx on timeout condition
7d7dec450a66 8250_tegra: clean up tegra_uart_handle_break
c3ae3dc896fa serial: 8250_pci: Drop bogus __refdata annotation
d96f04d347e4 serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access
6e4e636e0e3e serial: 8250-mtk: Fix reference leak in mtk8250_probe
a609c58086e3 tty: serial: 8250: 8250_port: Move prototypes to shared location
6f9918504129 serial: 8250: 8250_omap: Fix unused variable warning
d4548b14dd7e serial: 8250: 8250_omap: Fix possible array out of bounds access
912ab37c7987 serial: 8250_mtk: Fix uart_get_baud_rate warning
439c7183e5b9 serial: 8250: 8250_omap: Disable RX interrupt after DMA enable
32ed248042d1 tty: serial: 8250: serial_cs: Remove unused/unchecked
variable 'err'
85985a3dcd74 serial: 8250_dw: Fix clk-notifier/port suspend deadlock
c8dff3aa8241 serial: 8250: Skip uninitialized TTY port baud rate update
7718453e3696 serial: 8250: Discard RTS/DTS setting from clock update method
409cc4541ade serial: 8250_fsl: Fix TX interrupt handling condition
3c5a87be170a serial: 8250_pci: Add Realtek 816a and 816b
ea4de367e57d tty: serial: 8250_mtk: set regshift for mmio32
57cee0713118 serial: 8250_pci: Remove unused function get_pci_irq()
11361610b005 serial: 8250_fsl: Add ACPI support
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed,lpc-io-reg and aspeed,lpc-interrupts DT properties
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 1:16 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
These allow describing all the Aspeed VUART attributes currently
available via sysfs. aspeed,sirq provides a replacement for the
deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8433f8dbb186..75ef006fa24b 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -28,6 +28,10 @@
#define ASPEED_VUART_ADDRL 0x28
#define ASPEED_VUART_ADDRH 0x2c
+#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
+#define ASPEED_VUART_DEFAULT_SIRQ 4
+#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
+
struct aspeed_vuart {
struct device *dev;
void __iomem *regs;
@@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
struct aspeed_vuart *vuart;
struct device_node *np;
struct resource *res;
- u32 clk, prop;
+ u32 clk, prop, sirq[2];
+ bool sirq_polarity;
int rc;
np = pdev->dev.of_node;
@@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
of_node_put(sirq_polarity_sense_args.np);
}
+ rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
+ if (rc < 0)
+ prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
+
+ rc = aspeed_vuart_set_lpc_address(vuart, prop);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
+ goto err_clk_disable;
+ }
+
+ rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
+ if (rc < 0) {
+ sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
+ sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
+ }
+
+ rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts property\n");
+ goto err_clk_disable;
+ }
+
+ switch (sirq[1]) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sirq_polarity = false;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ sirq_polarity = true;
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts property\n");
+ rc = -EINVAL;
+ goto err_clk_disable;
+ }
+
+ aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
+
aspeed_vuart_set_enabled(vuart, true);
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Jiri Slaby,
linux-serial
These allow describing all the Aspeed VUART attributes currently
available via sysfs. aspeed,sirq provides a replacement for the
deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8433f8dbb186..75ef006fa24b 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -28,6 +28,10 @@
#define ASPEED_VUART_ADDRL 0x28
#define ASPEED_VUART_ADDRH 0x2c
+#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
+#define ASPEED_VUART_DEFAULT_SIRQ 4
+#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
+
struct aspeed_vuart {
struct device *dev;
void __iomem *regs;
@@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
struct aspeed_vuart *vuart;
struct device_node *np;
struct resource *res;
- u32 clk, prop;
+ u32 clk, prop, sirq[2];
+ bool sirq_polarity;
int rc;
np = pdev->dev.of_node;
@@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
of_node_put(sirq_polarity_sense_args.np);
}
+ rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
+ if (rc < 0)
+ prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
+
+ rc = aspeed_vuart_set_lpc_address(vuart, prop);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
+ goto err_clk_disable;
+ }
+
+ rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
+ if (rc < 0) {
+ sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
+ sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
+ }
+
+ rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts property\n");
+ goto err_clk_disable;
+ }
+
+ switch (sirq[1]) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sirq_polarity = false;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ sirq_polarity = true;
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts property\n");
+ rc = -EINVAL;
+ goto err_clk_disable;
+ }
+
+ aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
+
aspeed_vuart_set_enabled(vuart, true);
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: linux-aspeed, Zev Weiss, Andrew Jeffery, Greg Kroah-Hartman,
openbmc, linux-kernel, linux-serial, Jiri Slaby,
linux-arm-kernel
These allow describing all the Aspeed VUART attributes currently
available via sysfs. aspeed,sirq provides a replacement for the
deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8433f8dbb186..75ef006fa24b 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -28,6 +28,10 @@
#define ASPEED_VUART_ADDRL 0x28
#define ASPEED_VUART_ADDRH 0x2c
+#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
+#define ASPEED_VUART_DEFAULT_SIRQ 4
+#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
+
struct aspeed_vuart {
struct device *dev;
void __iomem *regs;
@@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
struct aspeed_vuart *vuart;
struct device_node *np;
struct resource *res;
- u32 clk, prop;
+ u32 clk, prop, sirq[2];
+ bool sirq_polarity;
int rc;
np = pdev->dev.of_node;
@@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
of_node_put(sirq_polarity_sense_args.np);
}
+ rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
+ if (rc < 0)
+ prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
+
+ rc = aspeed_vuart_set_lpc_address(vuart, prop);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
+ goto err_clk_disable;
+ }
+
+ rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
+ if (rc < 0) {
+ sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
+ sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
+ }
+
+ rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts property\n");
+ goto err_clk_disable;
+ }
+
+ switch (sirq[1]) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sirq_polarity = false;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ sirq_polarity = true;
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts property\n");
+ rc = -EINVAL;
+ goto err_clk_disable;
+ }
+
+ aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
+
aspeed_vuart_set_enabled(vuart, true);
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-09 5:14 ` Andrew Jeffery
-1 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:14 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
Hi Zev,
A couple of minor comments:
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> These allow describing all the Aspeed VUART attributes currently
> available via sysfs. aspeed,sirq
aspeed,lpc-interrupts now
> provides a replacement for the
> deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 8433f8dbb186..75ef006fa24b 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -28,6 +28,10 @@
> #define ASPEED_VUART_ADDRL 0x28
> #define ASPEED_VUART_ADDRH 0x2c
>
> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
> +#define ASPEED_VUART_DEFAULT_SIRQ 4
> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
> +
> struct aspeed_vuart {
> struct device *dev;
> void __iomem *regs;
> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> struct aspeed_vuart *vuart;
> struct device_node *np;
> struct resource *res;
> - u32 clk, prop;
> + u32 clk, prop, sirq[2];
> + bool sirq_polarity;
> int rc;
>
> np = pdev->dev.of_node;
> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> of_node_put(sirq_polarity_sense_args.np);
> }
>
> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
> + if (rc < 0)
> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
> +
> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
> + goto err_clk_disable;
> + }
> +
> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
> + if (rc < 0) {
> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
> + }
> +
> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
> + goto err_clk_disable;
> + }
> +
> + switch (sirq[1]) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sirq_polarity = false;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + sirq_polarity = true;
> + break;
> + default:
> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
> property\n");
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
A bit ugly open-coding the mapping and error handling, maybe worth a helper?
Looks okay otherwise.
Cheers,
Andrew
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-09 5:14 ` Andrew Jeffery
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:14 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
Hi Zev,
A couple of minor comments:
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> These allow describing all the Aspeed VUART attributes currently
> available via sysfs. aspeed,sirq
aspeed,lpc-interrupts now
> provides a replacement for the
> deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 8433f8dbb186..75ef006fa24b 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -28,6 +28,10 @@
> #define ASPEED_VUART_ADDRL 0x28
> #define ASPEED_VUART_ADDRH 0x2c
>
> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
> +#define ASPEED_VUART_DEFAULT_SIRQ 4
> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
> +
> struct aspeed_vuart {
> struct device *dev;
> void __iomem *regs;
> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> struct aspeed_vuart *vuart;
> struct device_node *np;
> struct resource *res;
> - u32 clk, prop;
> + u32 clk, prop, sirq[2];
> + bool sirq_polarity;
> int rc;
>
> np = pdev->dev.of_node;
> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> of_node_put(sirq_polarity_sense_args.np);
> }
>
> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
> + if (rc < 0)
> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
> +
> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
> + goto err_clk_disable;
> + }
> +
> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
> + if (rc < 0) {
> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
> + }
> +
> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
> + goto err_clk_disable;
> + }
> +
> + switch (sirq[1]) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sirq_polarity = false;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + sirq_polarity = true;
> + break;
> + default:
> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
> property\n");
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
A bit ugly open-coding the mapping and error handling, maybe worth a helper?
Looks okay otherwise.
Cheers,
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-09 5:14 ` Andrew Jeffery
0 siblings, 0 replies; 41+ messages in thread
From: Andrew Jeffery @ 2021-04-09 5:14 UTC (permalink / raw)
To: Zev Weiss, Joel Stanley
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
Hi Zev,
A couple of minor comments:
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> These allow describing all the Aspeed VUART attributes currently
> available via sysfs. aspeed,sirq
aspeed,lpc-interrupts now
> provides a replacement for the
> deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 8433f8dbb186..75ef006fa24b 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -28,6 +28,10 @@
> #define ASPEED_VUART_ADDRL 0x28
> #define ASPEED_VUART_ADDRH 0x2c
>
> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
> +#define ASPEED_VUART_DEFAULT_SIRQ 4
> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
> +
> struct aspeed_vuart {
> struct device *dev;
> void __iomem *regs;
> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> struct aspeed_vuart *vuart;
> struct device_node *np;
> struct resource *res;
> - u32 clk, prop;
> + u32 clk, prop, sirq[2];
> + bool sirq_polarity;
> int rc;
>
> np = pdev->dev.of_node;
> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> of_node_put(sirq_polarity_sense_args.np);
> }
>
> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
> + if (rc < 0)
> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
> +
> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
> + goto err_clk_disable;
> + }
> +
> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
> + if (rc < 0) {
> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
> + }
> +
> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
> + goto err_clk_disable;
> + }
> +
> + switch (sirq[1]) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sirq_polarity = false;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + sirq_polarity = true;
> + break;
> + default:
> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
> property\n");
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
A bit ugly open-coding the mapping and error handling, maybe worth a helper?
Looks okay otherwise.
Cheers,
Andrew
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
2021-04-09 5:14 ` Andrew Jeffery
(?)
@ 2021-04-09 6:35 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 6:35 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Joel Stanley, linux-aspeed, Greg Kroah-Hartman, openbmc,
linux-kernel, linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 09, 2021 at 12:14:54AM CDT, Andrew Jeffery wrote:
>Hi Zev,
>
>A couple of minor comments:
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> These allow describing all the Aspeed VUART attributes currently
>> available via sysfs. aspeed,sirq
>
>aspeed,lpc-interrupts now
Ah right, thanks.
>
>> provides a replacement for the
>> deprecated aspeed,sirq-polarity-sense property.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index 8433f8dbb186..75ef006fa24b 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -28,6 +28,10 @@
>> #define ASPEED_VUART_ADDRL 0x28
>> #define ASPEED_VUART_ADDRH 0x2c
>>
>> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
>> +#define ASPEED_VUART_DEFAULT_SIRQ 4
>> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
>> +
>> struct aspeed_vuart {
>> struct device *dev;
>> void __iomem *regs;
>> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> struct aspeed_vuart *vuart;
>> struct device_node *np;
>> struct resource *res;
>> - u32 clk, prop;
>> + u32 clk, prop, sirq[2];
>> + bool sirq_polarity;
>> int rc;
>>
>> np = pdev->dev.of_node;
>> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> of_node_put(sirq_polarity_sense_args.np);
>> }
>>
>> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
>> + if (rc < 0)
>> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
>> +
>> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
>> + if (rc < 0) {
>> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
>> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
>> + }
>> +
>> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + switch (sirq[1]) {
>> + case IRQ_TYPE_LEVEL_LOW:
>> + sirq_polarity = false;
>> + break;
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + sirq_polarity = true;
>> + break;
>> + default:
>> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
>> property\n");
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>
>A bit ugly open-coding the mapping and error handling, maybe worth a helper?
>
Yeah, that sounds reasonable -- will do.
Thanks!
Zev
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-09 6:35 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 6:35 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Joel Stanley, linux-aspeed, Greg Kroah-Hartman, openbmc,
linux-kernel, linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 09, 2021 at 12:14:54AM CDT, Andrew Jeffery wrote:
>Hi Zev,
>
>A couple of minor comments:
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> These allow describing all the Aspeed VUART attributes currently
>> available via sysfs. aspeed,sirq
>
>aspeed,lpc-interrupts now
Ah right, thanks.
>
>> provides a replacement for the
>> deprecated aspeed,sirq-polarity-sense property.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index 8433f8dbb186..75ef006fa24b 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -28,6 +28,10 @@
>> #define ASPEED_VUART_ADDRL 0x28
>> #define ASPEED_VUART_ADDRH 0x2c
>>
>> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
>> +#define ASPEED_VUART_DEFAULT_SIRQ 4
>> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
>> +
>> struct aspeed_vuart {
>> struct device *dev;
>> void __iomem *regs;
>> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> struct aspeed_vuart *vuart;
>> struct device_node *np;
>> struct resource *res;
>> - u32 clk, prop;
>> + u32 clk, prop, sirq[2];
>> + bool sirq_polarity;
>> int rc;
>>
>> np = pdev->dev.of_node;
>> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> of_node_put(sirq_polarity_sense_args.np);
>> }
>>
>> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
>> + if (rc < 0)
>> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
>> +
>> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
>> + if (rc < 0) {
>> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
>> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
>> + }
>> +
>> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + switch (sirq[1]) {
>> + case IRQ_TYPE_LEVEL_LOW:
>> + sirq_polarity = false;
>> + break;
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + sirq_polarity = true;
>> + break;
>> + default:
>> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
>> property\n");
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>
>A bit ugly open-coding the mapping and error handling, maybe worth a helper?
>
Yeah, that sounds reasonable -- will do.
Thanks!
Zev
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v5 3/4] drivers/tty/serial/8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts DT properties
@ 2021-04-09 6:35 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-09 6:35 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
linux-serial, Jiri Slaby, linux-arm-kernel
On Fri, Apr 09, 2021 at 12:14:54AM CDT, Andrew Jeffery wrote:
>Hi Zev,
>
>A couple of minor comments:
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> These allow describing all the Aspeed VUART attributes currently
>> available via sysfs. aspeed,sirq
>
>aspeed,lpc-interrupts now
Ah right, thanks.
>
>> provides a replacement for the
>> deprecated aspeed,sirq-polarity-sense property.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index 8433f8dbb186..75ef006fa24b 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -28,6 +28,10 @@
>> #define ASPEED_VUART_ADDRL 0x28
>> #define ASPEED_VUART_ADDRH 0x2c
>>
>> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
>> +#define ASPEED_VUART_DEFAULT_SIRQ 4
>> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
>> +
>> struct aspeed_vuart {
>> struct device *dev;
>> void __iomem *regs;
>> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> struct aspeed_vuart *vuart;
>> struct device_node *np;
>> struct resource *res;
>> - u32 clk, prop;
>> + u32 clk, prop, sirq[2];
>> + bool sirq_polarity;
>> int rc;
>>
>> np = pdev->dev.of_node;
>> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> of_node_put(sirq_polarity_sense_args.np);
>> }
>>
>> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
>> + if (rc < 0)
>> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
>> +
>> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
>> + if (rc < 0) {
>> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
>> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
>> + }
>> +
>> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + switch (sirq[1]) {
>> + case IRQ_TYPE_LEVEL_LOW:
>> + sirq_polarity = false;
>> + break;
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + sirq_polarity = true;
>> + break;
>> + default:
>> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
>> property\n");
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>
>A bit ugly open-coding the mapping and error handling, maybe worth a helper?
>
Yeah, that sounds reasonable -- will do.
Thanks!
Zev
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed,lpc-io-reg and aspeed,lpc-interrupts
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 1:16 ` Zev Weiss
-1 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Rob Herring,
Lubomir Rintel, -,
linux-serial
These correspond to the existing lpc_address, sirq, and sirq_polarity
sysfs attributes; the second element of aspeed,lpc-interrupts provides
a replacement for the deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
.../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 491b9297432d..0e82b076e199 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -12,8 +12,13 @@ maintainers:
allOf:
- $ref: /schemas/serial.yaml#
- if:
- required:
- - aspeed,sirq-polarity-sense
+ anyOf:
+ - required:
+ - aspeed,lpc-io-reg
+ - required:
+ - aspeed,lpc-interrupts
+ - required:
+ - aspeed,sirq-polarity-sense
then:
properties:
compatible:
@@ -190,6 +195,20 @@ properties:
applicable to aspeed,ast2500-vuart.
deprecated: true
+ aspeed,lpc-io-reg:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The VUART LPC address. Only applicable to aspeed,ast2500-vuart.
+
+ aspeed,lpc-interrupts:
+ $ref: "/schemas/types.yaml#/definitions/uint32-array"
+ minItems: 2
+ maxItems: 2
+ description: |
+ A 2-cell property describing the VUART SIRQ number and SIRQ
+ polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
+ applicable to aspeed,ast2500-vuart.
+
required:
- reg
- interrupts
@@ -221,6 +240,7 @@ examples:
};
- |
#include <dt-bindings/clock/aspeed-clock.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
serial@1e787000 {
compatible = "aspeed,ast2500-vuart";
reg = <0x1e787000 0x40>;
@@ -228,7 +248,8 @@ examples:
interrupts = <8>;
clocks = <&syscon ASPEED_CLK_APB>;
no-loopback-test;
- aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
+ aspeed,lpc-io-reg = <0x3f8>;
+ aspeed,lpc-interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
};
...
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: openbmc, linux-arm-kernel, linux-aspeed, linux-kernel,
Andrew Jeffery, Zev Weiss, Greg Kroah-Hartman, Rob Herring,
Lubomir Rintel, -,
linux-serial
These correspond to the existing lpc_address, sirq, and sirq_polarity
sysfs attributes; the second element of aspeed,lpc-interrupts provides
a replacement for the deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
.../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 491b9297432d..0e82b076e199 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -12,8 +12,13 @@ maintainers:
allOf:
- $ref: /schemas/serial.yaml#
- if:
- required:
- - aspeed,sirq-polarity-sense
+ anyOf:
+ - required:
+ - aspeed,lpc-io-reg
+ - required:
+ - aspeed,lpc-interrupts
+ - required:
+ - aspeed,sirq-polarity-sense
then:
properties:
compatible:
@@ -190,6 +195,20 @@ properties:
applicable to aspeed,ast2500-vuart.
deprecated: true
+ aspeed,lpc-io-reg:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The VUART LPC address. Only applicable to aspeed,ast2500-vuart.
+
+ aspeed,lpc-interrupts:
+ $ref: "/schemas/types.yaml#/definitions/uint32-array"
+ minItems: 2
+ maxItems: 2
+ description: |
+ A 2-cell property describing the VUART SIRQ number and SIRQ
+ polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
+ applicable to aspeed,ast2500-vuart.
+
required:
- reg
- interrupts
@@ -221,6 +240,7 @@ examples:
};
- |
#include <dt-bindings/clock/aspeed-clock.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
serial@1e787000 {
compatible = "aspeed,ast2500-vuart";
reg = <0x1e787000 0x40>;
@@ -228,7 +248,8 @@ examples:
interrupts = <8>;
clocks = <&syscon ASPEED_CLK_APB>;
no-loopback-test;
- aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
+ aspeed,lpc-io-reg = <0x3f8>;
+ aspeed,lpc-interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
};
...
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts
@ 2021-04-08 1:16 ` Zev Weiss
0 siblings, 0 replies; 41+ messages in thread
From: Zev Weiss @ 2021-04-08 1:16 UTC (permalink / raw)
To: Joel Stanley
Cc: -,
linux-aspeed, Zev Weiss, Andrew Jeffery, Greg Kroah-Hartman,
openbmc, linux-kernel, Lubomir Rintel, Rob Herring, linux-serial,
linux-arm-kernel
These correspond to the existing lpc_address, sirq, and sirq_polarity
sysfs attributes; the second element of aspeed,lpc-interrupts provides
a replacement for the deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
.../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 491b9297432d..0e82b076e199 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -12,8 +12,13 @@ maintainers:
allOf:
- $ref: /schemas/serial.yaml#
- if:
- required:
- - aspeed,sirq-polarity-sense
+ anyOf:
+ - required:
+ - aspeed,lpc-io-reg
+ - required:
+ - aspeed,lpc-interrupts
+ - required:
+ - aspeed,sirq-polarity-sense
then:
properties:
compatible:
@@ -190,6 +195,20 @@ properties:
applicable to aspeed,ast2500-vuart.
deprecated: true
+ aspeed,lpc-io-reg:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The VUART LPC address. Only applicable to aspeed,ast2500-vuart.
+
+ aspeed,lpc-interrupts:
+ $ref: "/schemas/types.yaml#/definitions/uint32-array"
+ minItems: 2
+ maxItems: 2
+ description: |
+ A 2-cell property describing the VUART SIRQ number and SIRQ
+ polarity (IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_LEVEL_HIGH). Only
+ applicable to aspeed,ast2500-vuart.
+
required:
- reg
- interrupts
@@ -221,6 +240,7 @@ examples:
};
- |
#include <dt-bindings/clock/aspeed-clock.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
serial@1e787000 {
compatible = "aspeed,ast2500-vuart";
reg = <0x1e787000 0x40>;
@@ -228,7 +248,8 @@ examples:
interrupts = <8>;
clocks = <&syscon ASPEED_CLK_APB>;
no-loopback-test;
- aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
+ aspeed,lpc-io-reg = <0x3f8>;
+ aspeed,lpc-interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
};
...
--
2.31.1
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts
2021-04-08 1:16 ` Zev Weiss
(?)
@ 2021-04-08 16:00 ` Rob Herring
-1 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 16:00 UTC (permalink / raw)
To: Zev Weiss
Cc: linux-arm-kernel, openbmc, linux-kernel, Joel Stanley,
linux-aspeed, Lubomir Rintel, Rob Herring, linux-serial,
Greg Kroah-Hartman, Andrew Jeffery, -
On Wed, 07 Apr 2021 20:16:37 -0500, Zev Weiss wrote:
> These correspond to the existing lpc_address, sirq, and sirq_polarity
> sysfs attributes; the second element of aspeed,lpc-interrupts provides
> a replacement for the deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> .../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts
@ 2021-04-08 16:00 ` Rob Herring
0 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 16:00 UTC (permalink / raw)
To: Zev Weiss
Cc: linux-arm-kernel, openbmc, linux-kernel, Joel Stanley,
linux-aspeed, Lubomir Rintel, Rob Herring, linux-serial,
Greg Kroah-Hartman, Andrew Jeffery, -
On Wed, 07 Apr 2021 20:16:37 -0500, Zev Weiss wrote:
> These correspond to the existing lpc_address, sirq, and sirq_polarity
> sysfs attributes; the second element of aspeed,lpc-interrupts provides
> a replacement for the deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> .../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v5 4/4] dt-bindings: serial: 8250: add aspeed, lpc-io-reg and aspeed, lpc-interrupts
@ 2021-04-08 16:00 ` Rob Herring
0 siblings, 0 replies; 41+ messages in thread
From: Rob Herring @ 2021-04-08 16:00 UTC (permalink / raw)
To: Zev Weiss
Cc: -,
linux-aspeed, Andrew Jeffery, Greg Kroah-Hartman, openbmc,
linux-kernel, Lubomir Rintel, Rob Herring, linux-serial,
linux-arm-kernel
On Wed, 07 Apr 2021 20:16:37 -0500, Zev Weiss wrote:
> These correspond to the existing lpc_address, sirq, and sirq_polarity
> sysfs attributes; the second element of aspeed,lpc-interrupts provides
> a replacement for the deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> .../devicetree/bindings/serial/8250.yaml | 27 ++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 41+ messages in thread