Hi Jai,Changhuang,
Thanks for the reviews.
On 18/02/25 21:07, Jai Luthra wrote:
On Tue, Feb 18, 2025 at 09:35:50AM +0000, Changhuang Liang wrote:
Hi Jai, Abhilash
Hi Abhilash,
On Mon, Feb 17, 2025 at 06:30:12PM +0530, Yemike Abhilash Chandra wrote:
The Cadence CSI2RX IP exposes 3 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>
---
Changes in v2:
- Address Krzysztof's review comment to remove flexibility while adding
interrupts.
.../devicetree/bindings/media/cdns,csi2rx.yaml | 10
++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
index 2008a47c0580..f335429cbde9 100644
--- a/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
+++ b/Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
@@ -24,6 +24,16 @@ properties:
reg:
maxItems: 1
+ interrupts:
+ minItems: 3
+ maxItems: 3
+
+ interrupt-names:
+ items:
+ - const: info
+ - const: error
+ - const: monitor
+
How many interrupt lines are actually exposed by the Cadence IP?
I only see that the Cadence IP exposes two interrupt lines: irq and err_irq. If there are any mistakes,
please help correct them.
Thank you Changhuang for the quick confirmation.
This seems to match CSI_ERR_IRQ and CSI_IRQ in TI's integration.
It is not clear to me from the TRM [0]. From the "Table 12-1524. CSI_RX_IF
Hardware Requests" it looks like there are three separate interrupt lines
CSI_ERR_IRQ, CSI_IRQ and CSI_LEVEL, which are routed to the Arm core/GIC.
And four lines for the ASF submodule (?) that are not routed to GIC.
Abhilash,
What is unclear to me is where the third CSI_LEVEL interrupt line is coming
from?
The TRM description of the CSI_LEVEL interrupt says it is for PSI_L overflow
or VP0/VP1 frame/line mismatch, both of which are outside the Cadence CSI2RX,
instead belonging to the TI wrapper hardware, which has its own driver.
Yes, we will update the device tree bindings accordingly. We are
planning to handle the TI-specific interrupt line separately in another
series.
This does not match with the error, info and monitor line names mentioned
here.
In my understanding which interrupt lines are actually integrated will vary
from SoC to SoC, so please check what are the actual interrupt line names
exposed by the Cadence IP. Maybe Maxime knows more.
But I don't think it is correct to make all 3 mandatory together, as some
vendors may only integrate the error interrupt ignoring the rest.
Agreed.
I think by default there should only be two optional interrupt lines for
cdns-csi2rx, "irq" and "err_irq" which is common across vendors.
If this third TI-specific CSI_LEVEL interrupt *has* to be handled by
cdns-csi2rx driver, it would be better to create conditional bindings and
match data in the driver such that the irq is only requested if
"ti,j721e-csi2rx" compatible is present.
I will correct this in v3 by introducing bindings that allow vendors
the flexibility to use either two or three interrupt lines.
Thanks and Regards,
Yemike Abhilash Chandra
Best Regards,
Changhuang
Thanks,
Jai
