Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional

From: Robin Murphy <robin.murphy@arm.com>
To: Andre Przywara <andre.przywara@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	iommu@lists.linux-foundation.org
Subject: Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
Date: Wed, 27 Apr 2022 13:04:26 +0100	[thread overview]
Message-ID: <ffbcf1e0-cf46-186a-3862-9f90415a0ab0@arm.com> (raw)
In-Reply-To: <20220427112528.4097815-2-andre.przywara@arm.com>

On 2022-04-27 12:25, Andre Przywara wrote:
> The Page Request Interface (PRI) is an optional PCIe feature. As such, a
> SMMU would not need to handle it if the PCIe host bridge or the SMMU
> itself do not implement it. Also an SMMU could be connected to a platform
> device, without any PRI functionality whatsoever.
> In all cases there would be no SMMU PRI queue interrupt to be wired up
> to an interrupt controller.
> 
> Relax the binding to allow specifying three interrupts, omitting the PRI
> IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we
> would need to sacrifice the command queue sync interrupt as well, which
> might not be desired.
> The Linux driver does not care about any order at all, just picks IRQs
> based on their names, and treats all (wired) IRQs as optional.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>   .../bindings/iommu/arm,smmu-v3.yaml           | 21 ++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> index e87bfbcc69135..6b3111f1f06ce 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> @@ -37,12 +37,23 @@ properties:
>             hardware supports just a single, combined interrupt line.
>             If provided, then the combined interrupt will be used in preference to
>             any others.
> -      - minItems: 2
> +      - minItems: 1
>           items:
> -          - const: eventq     # Event Queue not empty
> -          - const: gerror     # Global Error activated
> -          - const: priq       # PRI Queue not empty
> -          - const: cmdq-sync  # CMD_SYNC complete
> +          - enum:
> +              - eventq     # Event Queue not empty
> +              - gerror     # Global Error activated
> +              - cmdq-sync  # CMD_SYNC complete
> +              - priq       # PRI Queue not empty
> +          - enum:
> +              - gerror
> +              - cmdq-sync
> +              - priq
> +          - enum:
> +              - cmdq-sync
> +              - priq
> +          - enum:
> +              - cmdq-sync
> +              - priq

Nit: the commit message doesn't mention this effective relaxation to 
allow "eventq, gerror, cmdq-sync, priq" as well - if there's a good 
reason for that it probably wants calling out, otherwise AFAICS the last 
item should stay as "- const: cmdq-sync" to preserve the existing 
canonical order for the 4-item case.

Otherwise,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Cheers,
Robin.

>   
>     '#iommu-cells':
>       const: 1