openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] aspeed-vuart: generalized DT properties
@ 2021-04-02  0:47 Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense Zev Weiss
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  0:47 UTC (permalink / raw)
  To: Joel Stanley
  Cc: linux-aspeed, Zev Weiss, Andrew Jeffery, openbmc, linux-kernel,
	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-address and
aspeed,sirq.  The latter allows describing the SIRQ polarity (along
with the interrupt number) directly, providing a simpler replacement
for aspeed,sirq-polarity-sense.


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/

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-address and aspeed,sirq DT
    properties
  dt-bindings: serial: 8250: add aspeed,lpc-address and aspeed,sirq

 .../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] 7+ messages in thread

* [PATCH v3 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense
  2021-04-02  0:47 [PATCH v3 0/4] aspeed-vuart: generalized DT properties Zev Weiss
@ 2021-04-02  0:47 ` Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code Zev Weiss
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  0:47 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 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code
  2021-04-02  0:47 [PATCH v3 0/4] aspeed-vuart: generalized DT properties Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense Zev Weiss
@ 2021-04-02  0:47 ` Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 3/4] drivers/tty/serial/8250: add aspeed, lpc-address and aspeed, sirq DT properties Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed, sirq Zev Weiss
  3 siblings, 0 replies; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  0:47 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	[flat|nested] 7+ messages in thread

* [PATCH v3 3/4] drivers/tty/serial/8250: add aspeed, lpc-address and aspeed, sirq DT properties
  2021-04-02  0:47 [PATCH v3 0/4] aspeed-vuart: generalized DT properties Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code Zev Weiss
@ 2021-04-02  0:47 ` Zev Weiss
  2021-04-02  0:47 ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed, sirq Zev Weiss
  3 siblings, 0 replies; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  0:47 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..10b1f33386e6 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-address", &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-address property\n");
+		goto err_clk_disable;
+	}
+
+	rc = of_property_read_u32_array(np, "aspeed,sirq", 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,sirq 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,sirq 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	[flat|nested] 7+ messages in thread

* [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed, sirq
  2021-04-02  0:47 [PATCH v3 0/4] aspeed-vuart: generalized DT properties Zev Weiss
                   ` (2 preceding siblings ...)
  2021-04-02  0:47 ` [PATCH v3 3/4] drivers/tty/serial/8250: add aspeed, lpc-address and aspeed, sirq DT properties Zev Weiss
@ 2021-04-02  0:47 ` Zev Weiss
  2021-04-02  1:14   ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed,sirq Andrew Jeffery
  3 siblings, 1 reply; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  0:47 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,sirq 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..a6e01f9b745f 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-address
+        - required:
+            - aspeed,sirq
+        - required:
+            - aspeed,sirq-polarity-sense
     then:
       properties:
         compatible:
@@ -190,6 +195,20 @@ properties:
       applicable to aspeed,ast2500-vuart.
     deprecated: true
 
+  aspeed,lpc-address:
+    $ref: '/schemas/types.yaml#/definitions/uint32'
+    description: |
+      The VUART LPC address.  Only applicable to aspeed,ast2500-vuart.
+
+  aspeed,sirq:
+    $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-address = <0x3f8>;
+        aspeed,sirq = <4 IRQ_TYPE_LEVEL_LOW>;
     };
 
 ...
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed,sirq
  2021-04-02  0:47 ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed, sirq Zev Weiss
@ 2021-04-02  1:14   ` Andrew Jeffery
  2021-04-02  1:19     ` Zev Weiss
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Jeffery @ 2021-04-02  1:14 UTC (permalink / raw)
  To: Zev Weiss, Joel Stanley
  Cc: -,
	linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
	Lubomir Rintel, Rob Herring, linux-serial, linux-arm-kernel



On Fri, 2 Apr 2021, at 11:17, Zev Weiss wrote:
> These correspond to the existing lpc_address, sirq, and sirq_polarity
> sysfs attributes; the second element of aspeed,sirq 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..a6e01f9b745f 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-address

Why not aspeed,lpc-io-reg like the KCS binding?

There are some things we can do to improve it, but we shouldn't go and invent something different. I prefer aspeed,lpc-io-reg because it's name derives from the generic 'reg' property as does it's behaviour (if you assume a related `#size-cells = 0`).

> +        - required:
> +            - aspeed,sirq

Why not aspeed,lpc-interrupts like the KCS binding?

The generic IRQ property is 'interrupts', so like aspeed,lpc-io-reg the interrupts proposal for KCS follows in name and form. I'm hiding it behind the aspeed vendor prefix for now while I'm not satisfied that I understand the requirements of non-aspeed parts. Similarly, I added the lpc prefix because we don't tend to describe the host devicetree in the BMC devicetree (and so there's no parent interrupt controller that we can reference via a phandle) and we need a way to differentiate from the local interrupts property.

I don't see a reason for either of them to differ from what we already have for KCS, and I don't see any reason to continue the sysfs naming scheme in the binding.

Eventually I want to distil an LPC peripheral binding schema from what we've developed for the peripherals supported by aspeed and nuvoton SoCs.

Cheers,

Andrew

> +        - required:
> +            - aspeed,sirq-polarity-sense
>      then:
>        properties:
>          compatible:
> @@ -190,6 +195,20 @@ properties:
>        applicable to aspeed,ast2500-vuart.
>      deprecated: true
>  
> +  aspeed,lpc-address:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: |
> +      The VUART LPC address.  Only applicable to aspeed,ast2500-vuart.
> +
> +  aspeed,sirq:
> +    $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-address = <0x3f8>;
> +        aspeed,sirq = <4 IRQ_TYPE_LEVEL_LOW>;
>      };
>  
>  ...
> -- 
> 2.31.1
> 
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed,sirq
  2021-04-02  1:14   ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed,sirq Andrew Jeffery
@ 2021-04-02  1:19     ` Zev Weiss
  0 siblings, 0 replies; 7+ messages in thread
From: Zev Weiss @ 2021-04-02  1:19 UTC (permalink / raw)
  To: Andrew Jeffery
  Cc: -,
	linux-aspeed, Greg Kroah-Hartman, openbmc, linux-kernel,
	Lubomir Rintel, Rob Herring, linux-serial, linux-arm-kernel

On Thu, Apr 01, 2021 at 08:14:39PM CDT, Andrew Jeffery wrote:
>
>
>On Fri, 2 Apr 2021, at 11:17, Zev Weiss wrote:
>> These correspond to the existing lpc_address, sirq, and sirq_polarity
>> sysfs attributes; the second element of aspeed,sirq 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..a6e01f9b745f 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-address
>
>Why not aspeed,lpc-io-reg like the KCS binding?
>
>There are some things we can do to improve it, but we shouldn't go and invent something different. I prefer aspeed,lpc-io-reg because it's name derives from the generic 'reg' property as does it's behaviour (if you assume a related `#size-cells = 0`).
>
>> +        - required:
>> +            - aspeed,sirq
>
>Why not aspeed,lpc-interrupts like the KCS binding?
>
>The generic IRQ property is 'interrupts', so like aspeed,lpc-io-reg the interrupts proposal for KCS follows in name and form. I'm hiding it behind the aspeed vendor prefix for now while I'm not satisfied that I understand the requirements of non-aspeed parts. Similarly, I added the lpc prefix because we don't tend to describe the host devicetree in the BMC devicetree (and so there's no parent interrupt controller that we can reference via a phandle) and we need a way to differentiate from the local interrupts property.
>
>I don't see a reason for either of them to differ from what we already have for KCS, and I don't see any reason to continue the sysfs naming scheme in the binding.
>

Ah, OK -- I was aiming for consistency with the existing vuart sysfs 
attributes, but if that's not a worthwhile concern I'm fine with 
aspeed,lpc-io-reg & aspeed,lpc-interrupts.


Zev


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-02  1:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02  0:47 [PATCH v3 0/4] aspeed-vuart: generalized DT properties Zev Weiss
2021-04-02  0:47 ` [PATCH v3 1/4] dt-bindings: serial: 8250: deprecate aspeed, sirq-polarity-sense Zev Weiss
2021-04-02  0:47 ` [PATCH v3 2/4] drivers/tty/serial/8250: refactor sirq and lpc address setting code Zev Weiss
2021-04-02  0:47 ` [PATCH v3 3/4] drivers/tty/serial/8250: add aspeed, lpc-address and aspeed, sirq DT properties Zev Weiss
2021-04-02  0:47 ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed, sirq Zev Weiss
2021-04-02  1:14   ` [PATCH v3 4/4] dt-bindings: serial: 8250: add aspeed, lpc-address and aspeed,sirq Andrew Jeffery
2021-04-02  1:19     ` Zev Weiss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).