RE: [PATCH v3] media: platform: rzg2l-cru: rzg2l-video: Move request_irq() to probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Monday, August 26, 2024 10:56 AM
> Subject: Re: [PATCH v3] media: platform: rzg2l-cru: rzg2l-video: Move request_irq() to probe()
> 
> Hi Biju,
> 
> On Mon, Aug 26, 2024 at 11:50 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > On Mon, Aug 26, 2024 at 08:08:33AM +0000, Biju Das wrote:
> > > > On Monday, August 26, 2024 8:27 AM, claudiu beznea wrote:
> > > > > On 24.08.2024 21:21, Biju Das wrote:
> > > > > > @@ -270,9 +270,14 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
> > > > > >         cru->dev = &pdev->dev;
> > > > > >         cru->info = of_device_get_match_data(&pdev->dev);
> > > > > >
> > > > > > -       cru->image_conv_irq = platform_get_irq(pdev, 0);
> > > > > > -       if (cru->image_conv_irq < 0)
> > > > > > -               return cru->image_conv_irq;
> > > > > > +       irq = platform_get_irq(pdev, 0);
> > > > > > +       if (irq < 0)
> > > > > > +               return irq;
> > > > > > +
> > > > > > +       ret = devm_request_irq(&pdev->dev, irq, rzg2l_cru_irq, IRQF_SHARED,
> > > > > > +                              KBUILD_MODNAME, cru);
> > > > >
> > > > > Because this is requested w/ IRQF_SHARED the free_irq() ->
> > > > > __free_irq() [1] will call the IRQ handler to simulate an IRQ
> > > > > SHARE scenario where other device generate an interrupt.
> > >
> > > Good point, I had missed that.
> > >
> > > > Currently CSI driver is not registered any interrupts and CRU is the single user.
> > >
> > > Regardless, the fact that the IRQ is requested with IRQF_SHARED
> > > means that the IRQ handler needs to be prepared to be called at any
> > > time from the point of registration to the point the IRQ is freed. This is tested by
> CONFIG_DEBUG_SHIRQ=y, which you should enable for testing.
> >
> > For single user, testing CONFIG_DEBUG_SHIRQ=y this does not make any
> > sense. See [1] [1]
> > https://elixir.bootlin.com/linux/v6.11-rc5/source/kernel/irq/manage.c#
> > L1965
> >
> > >
> > > If you don't need to share the interrupt with any other device, you can drop the IRQF_SHARED.
> >
> > I will drop IRQF_SHARED flags instead as there is single user and will
> > send RFC for dropping calling IRQ handler for single user with
> > CONFIG_DEBUG_SHIRQ=y
> >
> > Please let me know is it fine for you?
> 
> IMHO the latter is not a good idea: if you register an interrupt handler with IRQF_SHARED, you should
> be prepared for the handler being called at any time, and test that. Regardless of whether there is
> only a single user or not.

OK, will drop sending RFC. 

Cheers,
Biju




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux