1. Patchew
  2. linux
  3. [PATCH v3 0/2] Enable support for error detection in CSI2RX
  4. View patch

[PATCH v3 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts for cdns-csi2rx

Yemike Abhilash Chandra posted 2 patches 11 months, 3 weeks ago
There is a newer version of this series
[PATCH v3 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts for cdns-csi2rx
Posted by Yemike Abhilash Chandra 11 months, 3 weeks ago 
The Cadence CSI2RX IP exposes 2 interrupts [0] 12.7 camera subsystem.
Enabling these interrupts will provide additional information about a CSI
packet or an individual frame. So, add support for optional interrupts
and interrupt-names properties.

[0]: http://www.ti.com/lit/pdf/spruil1

Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---

Changes in v3:
- Address Krzysztof's review comment to drop minItems from the bindings.
- Collect Acked-by from Krzysztof.

 Documentation/devicetree/bindings/media/cdns,csi2rx.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
index 2008a47c0580..e8d7eaf443d1 100644
--- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
+++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
@@ -24,6 +24,14 @@ properties:
   reg:
     maxItems: 1
 
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    items:
+      - const: irq
+      - const: error_irq
+
   clocks:
     items:
       - description: CSI2Rx system clock
-- 
2.34.1
Re: [PATCH v3 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts for cdns-csi2rx
Posted by Jai Luthra 11 months, 2 weeks ago 
Hi Abhilash,

Thanks for the patch.

On Fri, Feb 21, 2025 at 05:33:36PM +0530, Yemike Abhilash Chandra wrote:
> The Cadence CSI2RX IP exposes 2 interrupts [0] 12.7 camera subsystem.
> Enabling these interrupts will provide additional information about a CSI
> packet or an individual frame. So, add support for optional interrupts
> and interrupt-names properties.
> 
> [0]: http://www.ti.com/lit/pdf/spruil1
> 
> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> 
> Changes in v3:
> - Address Krzysztof's review comment to drop minItems from the bindings.
> - Collect Acked-by from Krzysztof.
> 
>  Documentation/devicetree/bindings/media/cdns,csi2rx.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> index 2008a47c0580..e8d7eaf443d1 100644
> --- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> +++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> @@ -24,6 +24,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:
> +    items:
> +      - const: irq
> +      - const: error_irq
> +

If I test these bindings with only one interrupt (error_irq) defined in the 
device tree, I get these errors:

  DTC [C] arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb
/home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupts: [[0, 187, 4]] is too short
        from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
/home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupt-names:0: 'irq' was expected
        from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
/home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupt-names: ['error_irq'] is too short
        from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
make[1]: Leaving directory '/home/darkapex/dev/linux2/out_clang'

There could be cases where only the error interrupt is integrated by the SoC, 
and the second interrupt is unconnected. IMHO it would make sense to keep the 
other interrupt optional:

diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml 
b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
index e8d7eaf443d1..054ed4b94312 100644
--- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
+++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
@@ -25,12 +25,14 @@ properties:
     maxItems: 1

   interrupts:
+    minItems: 1
     maxItems: 2

   interrupt-names:
+    minItems: 1
     items:
-      - const: irq
       - const: error_irq
+      - const: irq

   clocks:
     items:

>    clocks:
>      items:
>        - description: CSI2Rx system clock
> -- 
> 2.34.1
>
Re: [PATCH v3 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts for cdns-csi2rx
Posted by Yemike Abhilash Chandra 11 months, 2 weeks ago 
Hi Jai,
Thanks for the detailed review for the patch series.

On 24/02/25 12:50, Jai Luthra wrote:
> Hi Abhilash,
> 
> Thanks for the patch.
> 
> On Fri, Feb 21, 2025 at 05:33:36PM +0530, Yemike Abhilash Chandra wrote:
>> The Cadence CSI2RX IP exposes 2 interrupts [0] 12.7 camera subsystem.
>> Enabling these interrupts will provide additional information about a CSI
>> packet or an individual frame. So, add support for optional interrupts
>> and interrupt-names properties.
>>
>> [0]: http://www.ti.com/lit/pdf/spruil1
>>
>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>
>> Changes in v3:
>> - Address Krzysztof's review comment to drop minItems from the bindings.
>> - Collect Acked-by from Krzysztof.
>>
>>   Documentation/devicetree/bindings/media/cdns,csi2rx.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
>> index 2008a47c0580..e8d7eaf443d1 100644
>> --- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
>> +++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
>> @@ -24,6 +24,14 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> +  interrupts:
>> +    maxItems: 2
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: irq
>> +      - const: error_irq
>> +
> 
> If I test these bindings with only one interrupt (error_irq) defined in the
> device tree, I get these errors:
> 
>    DTC [C] arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb
> /home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupts: [[0, 187, 4]] is too short
>          from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
> /home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupt-names:0: 'irq' was expected
>          from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
> /home/darkapex/dev/linux2/out_clang/arch/arm64/boot/dts/ti/k3-am62a7-sk.dtb: csi-bridge@30101000: interrupt-names: ['error_irq'] is too short
>          from schema $id: http://devicetree.org/schemas/media/cdns,csi2rx.yaml#
> make[1]: Leaving directory '/home/darkapex/dev/linux2/out_clang'
> 
> There could be cases where only the error interrupt is integrated by the SoC,
> and the second interrupt is unconnected. IMHO it would make sense to keep the
> other interrupt optional:
> 

Initially, I had the flexibilty, but I removed that based on Krzysztof's
feedback, I was also not clear how these interrupts are integrated by
different vendors at that time.

But since this driver is shared among vendors, I think it is better to
have the flexibility. Since this is a change in dt-bindings I would I 
like to have Krzysztof's view on this discussion.

> diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> index e8d7eaf443d1..054ed4b94312 100644
> --- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> +++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
> @@ -25,12 +25,14 @@ properties:
>       maxItems: 1
> 
>     interrupts:
> +    minItems: 1
>       maxItems: 2
> 
>     interrupt-names:
> +    minItems: 1
>       items:
> -      - const: irq
>         - const: error_irq
> +      - const: irq
> 

Krzysztof, If you agree to the same I will use the above binidngs in
next version of the series

Thanks and Regards,
Yemike Abhilash Chandra.

>     clocks:
>       items:
> 
>>     clocks:
>>       items:
>>         - description: CSI2Rx system clock
>> -- 
>> 2.34.1
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.