[PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Zev Weiss]

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 c82bf6e133d30e0f9172a20807814fa28aef0f67.

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

Re: [PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Greg Kroah-Hartman]

On Fri, Apr 02, 2021 at 01:27:21PM -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 c82bf6e133d30e0f9172a20807814fa28aef0f67.

Please write that as:
	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(+)

What changed from previous versions?  That always goes below the ---
line.

Please fix up and send a v5.

thanks,

greg k-h

Re: [PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Zev Weiss]

On Wed, Apr 07, 2021 at 10:18:31AM CDT, Greg Kroah-Hartman wrote:
>On Fri, Apr 02, 2021 at 01:27:21PM -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 c82bf6e133d30e0f9172a20807814fa28aef0f67.
>
>Please write that as:
>	c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq polarity")
>

Ack, will do.

>> 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(+)
>
>What changed from previous versions?  That always goes below the ---
>line.
>

I included an overview of that in the cover letter 
(https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/); 
is it desirable to also have that duplicated in the individual patches 
in the series?


Zev
Zev

Re: [PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Greg Kroah-Hartman]

On Wed, Apr 07, 2021 at 11:56:04AM -0500, Zev Weiss wrote:
> On Wed, Apr 07, 2021 at 10:18:31AM CDT, Greg Kroah-Hartman wrote:
> > On Fri, Apr 02, 2021 at 01:27:21PM -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 c82bf6e133d30e0f9172a20807814fa28aef0f67.
> > 
> > Please write that as:
> > 	c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq polarity")
> > 
> 
> Ack, will do.
> 
> > > 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(+)
> > 
> > What changed from previous versions?  That always goes below the ---
> > line.
> > 
> 
> I included an overview of that in the cover letter (https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/);
> is it desirable to also have that duplicated in the individual patches in
> the series?

Any reason why you didn't include all of the relevant people and mailing
lists in that cover letter?  I've never seen it before :)

But yes, if you do send it to the right group, putting it in 00/XX is
fine.

thanks,

greg k-h

Re: [PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Zev Weiss]

On Wed, Apr 07, 2021 at 12:25:09PM CDT, Greg Kroah-Hartman wrote:
>On Wed, Apr 07, 2021 at 11:56:04AM -0500, Zev Weiss wrote:
>> On Wed, Apr 07, 2021 at 10:18:31AM CDT, Greg Kroah-Hartman wrote:
>> > On Fri, Apr 02, 2021 at 01:27:21PM -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 c82bf6e133d30e0f9172a20807814fa28aef0f67.
>> >
>> > Please write that as:
>> > 	c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq polarity")
>> >
>>
>> Ack, will do.
>>
>> > > 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(+)
>> >
>> > What changed from previous versions?  That always goes below the ---
>> > line.
>> >
>>
>> I included an overview of that in the cover letter (https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/);
>> is it desirable to also have that duplicated in the individual patches in
>> the series?
>
>Any reason why you didn't include all of the relevant people and mailing
>lists in that cover letter?  I've never seen it before :)
>

Ah -- is there a good rule of thumb (or git send-email trick) for that?  
Set-union of get_maintainer.pl over all the patches in the series?  


Zev

Re: [PATCH v4 1/4] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Greg Kroah-Hartman]

On Wed, Apr 07, 2021 at 12:55:29PM -0500, Zev Weiss wrote:
> On Wed, Apr 07, 2021 at 12:25:09PM CDT, Greg Kroah-Hartman wrote:
> > On Wed, Apr 07, 2021 at 11:56:04AM -0500, Zev Weiss wrote:
> > > On Wed, Apr 07, 2021 at 10:18:31AM CDT, Greg Kroah-Hartman wrote:
> > > > On Fri, Apr 02, 2021 at 01:27:21PM -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 c82bf6e133d30e0f9172a20807814fa28aef0f67.
> > > >
> > > > Please write that as:
> > > > 	c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq polarity")
> > > >
> > > 
> > > Ack, will do.
> > > 
> > > > > 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(+)
> > > >
> > > > What changed from previous versions?  That always goes below the ---
> > > > line.
> > > >
> > > 
> > > I included an overview of that in the cover letter (https://lore.kernel.org/openbmc/20210402182724.20848-1-zev@bewilderbeest.net/);
> > > is it desirable to also have that duplicated in the individual patches in
> > > the series?
> > 
> > Any reason why you didn't include all of the relevant people and mailing
> > lists in that cover letter?  I've never seen it before :)
> > 
> 
> Ah -- is there a good rule of thumb (or git send-email trick) for that?
> Set-union of get_maintainer.pl over all the patches in the series?

There might be a git send-email trick, but I don't know it :(

[PATCH v4 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code

[Zev Weiss]

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

[PATCH v4 3/4] drivers/tty/serial/8250: add aspeed,lpc-io-reg and aspeed,lpc-interrupts DT properties

[Zev Weiss]

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

[PATCH v4 4/4] dt-bindings: serial: 8250: add aspeed,lpc-io-reg and aspeed,lpc-interrupts

[Zev Weiss]

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