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@xxxxxx>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
---
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
