Re: [PATCH v4 02/17] dt-bindings: gpu: Add Imagination Technologies PowerVR GPU

[Conor Dooley]

Hey Sarah,

Your series does not appear to be threaded. `git send-email` can be
passed, for example, a directory containing a whole series & will set
the correct in-reply-to headers so that the series is in a single
thread.

On Fri, Jul 14, 2023 at 03:25:26PM +0100, Sarah Walker wrote:
> Add the device tree binding documentation for the Series AXE GPU used in
> TI AM62 SoCs.

> Changes since v3:
> - Remove oneOf in compatible property
> - Remove power-supply (not used on AM62)
> 
> Changes since v2:
> - Add commit message description
> - Remove mt8173-gpu support (not currently supported)
> - Drop quotes from $id and $schema
> - Remove reg: minItems
> - Drop _clk suffixes from clock-names
> - Remove operating-points-v2 property and cooling-cells (not currently
>   used)
> - Add additionalProperties: false
> - Remove stray blank line at the end of file

The changelog should go below the --- line.

> Signed-off-by: Sarah Walker <sarah.walker@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/gpu/img,powervr.yaml  | 68 +++++++++++++++++++
>  MAINTAINERS                                   |  7 ++
>  2 files changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpu/img,powervr.yaml

> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr.yaml b/Documentation/devicetree/bindings/gpu/img,powervr.yaml
> new file mode 100644
> index 000000000000..3292a0440465
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2022 Imagination Technologies Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpu/img,powervr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Imagination Technologies PowerVR GPU
> +
> +maintainers:
> +  - Sarah Walker <sarah.walker@xxxxxxxxxx>

> +  interrupts:
> +    items:
> +      - description: GPU interrupt

The description here doesn't add any value, since there is only one
interrupt, so you can drop it and do maxItems: 1 as you have done
elsewhere.

> +  interrupt-names:
> +    items:
> +      - const: gpu

And this
items:
  - const: gpu
can just be
const: gpu

Although, if there is only one interrupt this is probably not
particularly helpful. Are there other implementations of this IP that
have more interrupts?

Otherwise, this looks good to me.

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature