linux-kernel - Re: 回复: [PATCH v2 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts for cdns-csi2rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <swqtud4ohcsjjt4ofgrovsa5h7koriuvpmxb3hdemnndwems2a@nwiny4dvuzwg>
Date: Tue, 18 Feb 2025 21:07:31 +0530
From: Jai Luthra <jai.luthra@...ux.dev>
To: Changhuang Liang <changhuang.liang@...rfivetech.com>, 
	Yemike Abhilash Chandra <y-abhilashchandra@...com>
Cc: "mripard@...nel.org" <mripard@...nel.org>, 
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "mchehab@...nel.org" <mchehab@...nel.org>, 
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>, 
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "devarsht@...com" <devarsht@...com>, 
	"vaishnav.a@...com" <vaishnav.a@...com>, "r-donadkar@...com" <r-donadkar@...com>, 
	"u-kumar1@...com" <u-kumar1@...com>
Subject: Re: 回复: [PATCH v2 1/2] dt-bindings: media: cdns,csi2rx.yaml: Add optional interrupts
 for cdns-csi2rx

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@...com>
> > > ---
> > >
> > > 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.

> > 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.

> Best Regards,
> Changhuang

Thanks,
Jai

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)