diff mbox series

[v2,2/3] dt-bindings: serial: 8250: Add documentation for espi-enabled.

Message ID 20190731013404.243755-2-osk@google.com
State Superseded, archived
Headers show
Series [v2,1/3] drivers/tty/serial/8250: Make Aspeed VUART SIRQ polarity configurable | expand

Commit Message

Oskar Senft July 31, 2019, 1:34 a.m. UTC
Add documentation for 8250_aspeed_vuart's espi-enabled property that
enables to auto-configure the VUART's SIRQ polarity.

Signed-off-by: Oskar Senft <osk@google.com>
---
 Documentation/devicetree/bindings/serial/8250.txt | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Andrew Jeffery July 31, 2019, 3:12 a.m. UTC | #1
Hi Oskar,

On Wed, 31 Jul 2019, at 11:04, Oskar Senft wrote:
> Add documentation for 8250_aspeed_vuart's espi-enabled property that
> enables to auto-configure the VUART's SIRQ polarity.
> 
> Signed-off-by: Oskar Senft <osk@google.com>
> ---
>  Documentation/devicetree/bindings/serial/8250.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt 
> b/Documentation/devicetree/bindings/serial/8250.txt
> index 20d351f268ef..4b8b9e502179 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -56,6 +56,11 @@ Optional properties:
>  - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for 
> RTS/CTS/DTR/DSR/RI/DCD
>    line respectively. It will use specified GPIO instead of the 
> peripheral
>    function pin for the UART feature. If unsure, don't specify this 
> property.
> +- espi-enabled: Only applicable to aspeed,ast2500-vuart.

Bit of a bikeshed, but:

Given it's ASPEED-specific I expect you should use a vendor prefix for the
property, e.g. aspeed,espi-enabled.

However, as I understand it you want to determine what polarity the SIRQ
should be regardless of which of eSPI or LPC are enabled, so I don't think
the property name should be an explicit statement about eSPI. Maybe
"aspeed,sirq-polarity-sense"? Anyway, see the point below:

Please use ./scripts/get_maintainer.pl to determine where to send the
series - Copying just the linux-aspeed@ list for upstream patches is not
enough. For instance the series needs to at least go via the linux-serial@
list given that's the affected subsystem, and you're adding to the
devicetree binding so you need to send to the devicetree@ list as well
(you'll need an ack from Rob Herring). The get_maintainer.pl script will
give you all the information you need.

Cheers,

Andrew
Jeremy Kerr Sept. 5, 2019, 1:16 a.m. UTC | #2
Hi Oskar,

> Given it's ASPEED-specific I expect you should use a vendor prefix for the
> property, e.g. aspeed,espi-enabled.
> 
> However, as I understand it you want to determine what polarity the SIRQ
> should be regardless of which of eSPI or LPC are enabled, so I don't think
> the property name should be an explicit statement about eSPI. Maybe
> "aspeed,sirq-polarity-sense"?

Yep, +1 on Andrew's comments here. This property isn't an indication on
whether espi is enabled, but a method to detect it.

Cheers,


Jeremy
Oskar Senft Sept. 5, 2019, 2:41 p.m. UTC | #3
Hi Andrew and Jeremy

Thanks for both your input, that was helpful. I'm sorry it took me so long
to get back to this. I hope it's in better shape now.

> Given it's ASPEED-specific I expect you should use a vendor prefix for the
> > property, e.g. aspeed,espi-enabled.
>
That was a very good point.


> > However, as I understand it you want to determine what polarity the SIRQ
> > should be regardless of which of eSPI or LPC are enabled, so I don't
> think
> > the property name should be an explicit statement about eSPI. Maybe
> > "aspeed,sirq-polarity-sense"?
>
> Yep, +1 on Andrew's comments here. This property isn't an indication on
> whether espi is enabled, but a method to detect it.
>
I agree. I was so focused on functionality that I didn't look at this with
a wider view.

I'll send v3 of the patch that contains appropriate changes. I'll also
include further lists and individuals to get OKs as needed.

Thanks
Oskar.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
index 20d351f268ef..4b8b9e502179 100644
--- a/Documentation/devicetree/bindings/serial/8250.txt
+++ b/Documentation/devicetree/bindings/serial/8250.txt
@@ -56,6 +56,11 @@  Optional properties:
 - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD
   line respectively. It will use specified GPIO instead of the peripheral
   function pin for the UART feature. If unsure, don't specify this property.
+- espi-enabled: Only applicable to aspeed,ast2500-vuart. Value is a phandle to
+  aspeed,ast2500-scu syscon alongside register offset and bit number to
+  identify whether the system is in eSPI mode. This is used to auto-configure
+  SIRQ polarity on the vuart.
+  Example: espi-enabled = <&syscon 0x70 25>
 
 Note:
 * fsl,ns16550: