mbox series

[v2,0/2] serial: pl011: Add xilinx uart

Message ID 20220720142612.19779-1-shubhrajyoti.datta@xilinx.com
Headers show
Series serial: pl011: Add xilinx uart | expand

Message

Shubhrajyoti Datta July 20, 2022, 2:26 p.m. UTC
-Support uart peripheral in Xilinx Versal SOC.
Add the dt-binding for the same

v2: 
Update the commit message to reflect the AXI limitation.

Raviteja Narayanam (1):
  serial: pl011: Add support for Xilinx Uart

Shubhrajyoti Datta (1):
  dt-bindings: serial: pl011: Add 'arm,xlnx-uart'

 .../devicetree/bindings/serial/pl011.yaml     | 10 ++++--
 drivers/tty/serial/amba-pl011.c               | 33 +++++++++++++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)

Comments

Rob Herring July 20, 2022, 2:53 p.m. UTC | #1
On Wed, Jul 20, 2022 at 8:26 AM Shubhrajyoti Datta
<shubhrajyoti.datta@xilinx.com> wrote:
>
> The Xilinx Versal board uses the arm,pl011 ip. However the

s/ip/IP/

> axi port that it is connected to has a limitation that it allows
> only 32-bit accesses. So to differentiate we add a compatible.

Why not just use the standard 'reg-io-width' property?

> Add support for Uart used in Xilinx Versal SOCs as a platform
> device.

What's a platform device? Don't include Linuxisms in your bindings.

>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Signed-off-by: Raviteja Narayanam <raviteja.narayanam@xilinx.com>
> ---
>  Documentation/devicetree/bindings/serial/pl011.yaml | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/pl011.yaml b/Documentation/devicetree/bindings/serial/pl011.yaml
> index d8aed84abcd3..bf094ab93086 100644
> --- a/Documentation/devicetree/bindings/serial/pl011.yaml
> +++ b/Documentation/devicetree/bindings/serial/pl011.yaml
> @@ -24,9 +24,13 @@ select:
>
>  properties:
>    compatible:
> -    items:
> -      - const: arm,pl011
> -      - const: arm,primecell
> +    oneOf:
> +      - items:
> +          - const: arm,pl011
> +          - const: arm,primecell
> +      - items:
> +          - const: arm,pl011
> +          - const: arm,xlnx-uart # xilinx uart as platform device

First, this is backwards. compatible is most specific to least
specific. In your case Arm is not the vendor and just 'xlnx-uart' is
not very specific. You said this is for Versal SoC, so something like
'xlnx,versal-pl011' would be more appropriate. But again, I think
reg-io-width is all you need here.

The IP is still a Primecell block, so it should have 'arm,primecell'
still as the definition of 'arm,primecell' is that it has the ID
registers. Yes, that means Linux will create an amba_device instead,
but you can't be designing your binding to work-around Linux.

Rob