Re: [PATCH 2/5] dt-bindings: Document VeriSilicon APB GPIO driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 12/02/2025 14:46, Nikolaos Pasaloukos wrote:
> diff --git a/Documentation/devicetree/bindings/gpio/vsi,apb-gpio.yaml b/Documentation/devicetree/bindings/gpio/vsi,apb-gpio.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..4a293b44e03895b6a45cb85f42c47c46b64f5638
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/vsi,apb-gpio.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/vsi,apb-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: VeriSilicon APB GPIO controller
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  VeriSilicon GPIO controllers have a configurable number of ports, each
> +  of which are intended to be represented as child nodes with the generic
> +  GPIO-controller properties as described in this bindings file.
> +
> +maintainers:
> +  - Nikolaos Pasaloukos <nikolaos.pasaloukos@blaize.com>
> +  - James Cowgill <james.cowgill@blaize.com>
> +  - Matthew Redfearn <matthew.redfearn@blaize.com>
> +  - Neil Jones <neil.jones@blaize.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^gpio@[0-9a-f]+$"
> +
> +  compatible:
> +    oneOf:

Drop

> +      - description: Verisilicon APB GPIO controller

Drop

> +        items:
> +          - enum:

Missing SoC compatible. Maybe IP block is simple enough to use the same
generic compatible for all implementations, but usually Rob was
suggesting that it is anyway not the best approach, so I would just make
it SoC specific only (and rename the file to match compatible). If the
fallback is really usable by every implementation, it could stay, but
OTOH, your 'ngpios' property already suggests that it is not enough to
use fallback alone.


> +
> +  reg:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  ngpios:
> +    default: 32
> +    minimum: 1
> +    maximum: 32
> +
> +  interrupts:
> +    description: |
> +      The interrupts to the parent controller raised when GPIOs generate
> +      the interrupts. Specify a single interrupt since the controller
> +      provides one combined interrupt for all GPIOs.

Drop description, obvious.

> +    maxItems: 1
> +
> +  gpio-line-names: true
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +



Best regards,
Krzysztof