1. Patchew
  2. linux
  3. [PATCH v4 0/4] Fix dt-validate issues on qemu dtbdumps due to dt-bindings
  4. View patch

[PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible

Conor Dooley posted 4 patches 3 years, 7 months ago
[PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible
Posted by Conor Dooley 3 years, 7 months ago 
From: Conor Dooley <conor.dooley@microchip.com>

While "real" hardware might not use the compatible string "riscv,plic0"
it is present in the driver & QEMU uses it for automatically generated
virt machine dtbs. To avoid dt-validate problems with QEMU produced
dtbs, such as the following, add it to the binding.

riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
        'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
        'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
        'sifive,plic-1.0.0' was expected
        'thead,c900-plic' was expected
riscv-virt.dtb: plic@c000000: '#address-cells' is a required property

Reported-by: Rob Herring <robh@kernel.org>
Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml     | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
index 92e0f8c3eff2..99e01f4d0a69 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
@@ -66,6 +66,11 @@ properties:
           - enum:
               - allwinner,sun20i-d1-plic
           - const: thead,c900-plic
+      - items:
+          - const: sifive,plic-1.0.0
+          - const: riscv,plic0
+        deprecated: true
+        description: For the QEMU virt machine only
 
   reg:
     maxItems: 1
-- 
2.37.1
Re: [PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible
Posted by Heiko Stübner 3 years, 7 months ago 
Am Dienstag, 23. August 2022, 20:33:18 CEST schrieb Conor Dooley:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> While "real" hardware might not use the compatible string "riscv,plic0"
> it is present in the driver & QEMU uses it for automatically generated
> virt machine dtbs. To avoid dt-validate problems with QEMU produced
> dtbs, such as the following, add it to the binding.
> 
> riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
>         'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
>         'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
>         'sifive,plic-1.0.0' was expected
>         'thead,c900-plic' was expected
> riscv-virt.dtb: plic@c000000: '#address-cells' is a required property
> 
> Reported-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Re: [PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible
Posted by Heiko Stübner 3 years, 7 months ago 
Am Dienstag, 23. August 2022, 20:33:18 CEST schrieb Conor Dooley:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> While "real" hardware might not use the compatible string "riscv,plic0"
> it is present in the driver & QEMU uses it for automatically generated
> virt machine dtbs. To avoid dt-validate problems with QEMU produced
> dtbs, such as the following, add it to the binding.
> 
> riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
>         'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
>         'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
>         'sifive,plic-1.0.0' was expected
>         'thead,c900-plic' was expected
> riscv-virt.dtb: plic@c000000: '#address-cells' is a required property
> 
> Reported-by: Rob Herring <robh@kernel.org>
> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml     | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> index 92e0f8c3eff2..99e01f4d0a69 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> @@ -66,6 +66,11 @@ properties:
>            - enum:
>                - allwinner,sun20i-d1-plic
>            - const: thead,c900-plic
> +      - items:
> +          - const: sifive,plic-1.0.0
> +          - const: riscv,plic0
> +        deprecated: true

hmm, when setting this to deprecated, does this mean qemu was changed
to not use that compatible anymore?

I.e. reading deprecated I'd assume that this is kept around for old qemu builds?


Heiko

> +        description: For the QEMU virt machine only
>  
>    reg:
>      maxItems: 1
>
Re: [PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible
Posted by Conor.Dooley@microchip.com 3 years, 7 months ago 
On 24/08/2022 18:44, Heiko Stübner wrote:
> Am Dienstag, 23. August 2022, 20:33:18 CEST schrieb Conor Dooley:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> While "real" hardware might not use the compatible string "riscv,plic0"
>> it is present in the driver & QEMU uses it for automatically generated
>> virt machine dtbs. To avoid dt-validate problems with QEMU produced
>> dtbs, such as the following, add it to the binding.
>>
>> riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
>>         'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
>>         'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
>>         'sifive,plic-1.0.0' was expected
>>         'thead,c900-plic' was expected
>> riscv-virt.dtb: plic@c000000: '#address-cells' is a required property
>>
>> Reported-by: Rob Herring <robh@kernel.org>
>> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml     | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
>> index 92e0f8c3eff2..99e01f4d0a69 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
>> @@ -66,6 +66,11 @@ properties:
>>            - enum:
>>                - allwinner,sun20i-d1-plic
>>            - const: thead,c900-plic
>> +      - items:
>> +          - const: sifive,plic-1.0.0
>> +          - const: riscv,plic0
>> +        deprecated: true
> 
> hmm, when setting this to deprecated, does this mean qemu was changed
> to not use that compatible anymore?
> 
> I.e. reading deprecated I'd assume that this is kept around for old qemu builds?

I did not make that change to QEMU. From v1 [0]:

Rob:
> Conor:
>> In arm's virt.c they use the generic gic compatible & I don't see any
>> evidence of other archs using "qemu,foo" bindings. I suppose there's
>> always the option of just removing the "riscv,plic0" from the riscv's
>> virt.c
>
> I think we're pretty much stuck with what's in use already.

> I'm on the fence whether to mark it deprecated though if there is no 
> plan to 'fix' it. Doesn't really matter until the tools can selectively 
> remove deprecated properties from validation.

My interpretation was "do not use this compatible in any new devicetrees".

I don't really have any strong feelings here. Maybe the description is
sufficient?

Thanks,
Conor,

0 - https://lore.kernel.org/linux-riscv/20220809141436.GA1706120-robh@kernel.org/
> 
>> +        description: For the QEMU virt machine only
>>  
>>    reg:
>>      maxItems: 1
>>
> 
> 
> 
>
Re: [PATCH v4 2/4] dt-bindings: interrupt-controller: sifive,plic: add legacy riscv compatible
Posted by Heiko Stübner 3 years, 7 months ago 
Am Mittwoch, 24. August 2022, 19:55:17 CEST schrieb Conor.Dooley@microchip.com:
> On 24/08/2022 18:44, Heiko Stübner wrote:
> > Am Dienstag, 23. August 2022, 20:33:18 CEST schrieb Conor Dooley:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> While "real" hardware might not use the compatible string "riscv,plic0"
> >> it is present in the driver & QEMU uses it for automatically generated
> >> virt machine dtbs. To avoid dt-validate problems with QEMU produced
> >> dtbs, such as the following, add it to the binding.
> >>
> >> riscv-virt.dtb: plic@c000000: compatible: 'oneOf' conditional failed, one must be fixed:
> >>         'sifive,plic-1.0.0' is not one of ['sifive,fu540-c000-plic', 'starfive,jh7100-plic', 'canaan,k210-plic']
> >>         'sifive,plic-1.0.0' is not one of ['allwinner,sun20i-d1-plic']
> >>         'sifive,plic-1.0.0' was expected
> >>         'thead,c900-plic' was expected
> >> riscv-virt.dtb: plic@c000000: '#address-cells' is a required property
> >>
> >> Reported-by: Rob Herring <robh@kernel.org>
> >> Link: https://lore.kernel.org/linux-riscv/20220803170552.GA2250266-robh@kernel.org/
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>  .../bindings/interrupt-controller/sifive,plic-1.0.0.yaml     | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> >> index 92e0f8c3eff2..99e01f4d0a69 100644
> >> --- a/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> >> +++ b/Documentation/devicetree/bindings/interrupt-controller/sifive,plic-1.0.0.yaml
> >> @@ -66,6 +66,11 @@ properties:
> >>            - enum:
> >>                - allwinner,sun20i-d1-plic
> >>            - const: thead,c900-plic
> >> +      - items:
> >> +          - const: sifive,plic-1.0.0
> >> +          - const: riscv,plic0
> >> +        deprecated: true
> > 
> > hmm, when setting this to deprecated, does this mean qemu was changed
> > to not use that compatible anymore?
> > 
> > I.e. reading deprecated I'd assume that this is kept around for old qemu builds?
> 
> I did not make that change to QEMU. From v1 [0]:
> 
> Rob:
> > Conor:
> >> In arm's virt.c they use the generic gic compatible & I don't see any
> >> evidence of other archs using "qemu,foo" bindings. I suppose there's
> >> always the option of just removing the "riscv,plic0" from the riscv's
> >> virt.c
> >
> > I think we're pretty much stuck with what's in use already.
> 
> > I'm on the fence whether to mark it deprecated though if there is no 
> > plan to 'fix' it. Doesn't really matter until the tools can selectively 
> > remove deprecated properties from validation.
> 
> My interpretation was "do not use this compatible in any new devicetrees".
> 
> I don't really have any strong feelings here. Maybe the description is
> sufficient?

that makes sense then. Existing users can keep using it, but no-one should
create new usages of it, so this looks good then

Heiko

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.