Re: saa7134: race between device initialization and first interrupt

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

 



Hi,

Am Samstag, den 10.01.2009, 07:37 -0500 schrieb Andy Walls:
> On Sat, 2009-01-10 at 13:02 +0100, Marcin Slusarz wrote:
> > On Fri, Jan 09, 2009 at 09:48:07PM -0500, Andy Walls wrote:
> > > On Thu, 2009-01-08 at 00:50 -0200, Mauro Carvalho Chehab wrote:
> > > > On Sun, 4 Jan 2009 22:57:42 +0100
> > > > Marcin Slusarz <marcin.slusarz@xxxxxxxxx> wrote:
> > > > 
> > > > > Hi
> > > > > There's a race between saa7134 device initialization and first interrupt
> > > > > handler, which manifests as an oops [1].
> > > > > 
> > > > > saa7134_initdev -> request_irq -> saa7134_irq ->
> > > > > saa7134_irq_video_signalchange -> saa7134_tvaudio_setmute -> mute_input_7133
> > > > > 
> > > > > In mute_input_7133 dev->input == NULL and accessing dev->input->amux will oops,
> > > > > because dev->input would be initialized later:
> > > > > 
> > > > > saa7134_initdev -> saa7134_hwinit2 -> saa7134_video_init2 -> video_mux ->
> > > > > saa7134_tvaudio_setinput
> > > > > 
> > > > > I'm not sure how it should be fixed correctly, but one of attached patches
> > > > > should fix the symptom.
> > > > > 
> > > > > Marcin
>  
> > > Marcin,
> > > 
> > > What devices share the interrupt with the SAA7133?
> > > 
> > > $ cat /proc/interrupts
> > > 
> > > And could on of those those devices possibly generate an interrupt while
> > > the saa7134 driver is being modporbe'd?
> > 
> > I don't have this hardware so I can't tell. I've picked random oops from
> > kerneloops.org and tried to fix it.
> 
> Ah.  Good man.
> 
> 
> > > 
> > > Mauro & Marcin,
> > > 
> > > This looks like to me what is going on:
> > > 
> > > saa7134_hwinit1() does properly turn off IRQs for which it wants
> > > reports:
> > > 
> > > 	saa_writel(SAA7134_IRQ1, 0);
> > >         saa_writel(SAA7134_IRQ2, 0);
> > > 
> > > but not clearing the state of SAA7134_IRQ_REPORT, maybe something like
> > > this (I don't have a SAA7134 datasheet):
> > > 
> > > 	saa_writel(SAA7134_IRQ_REPORT, 0xffffffff);
> > > 
> > > So when saa7134_initdev() calls request_irq(..., saa7134_irq,
> > > IRQF_SHARED | IRQF_DISABLED, ...), it gets an IRQ that is shared with
> > > another device.
> > > 
> > > Before saa7134_hwinit2() is called by saa7134_initdev() to set
> > > "dev->input", some other device fires an interrupt and saa7134_irq() is
> > > called that then operates on the unknown state of the SAA7134_IRQ_REPORT
> > > register that was never cleared.
> > 
> > Sounds good. But I think this register should be set to 0, because in 
> > saa7134_irq, we do:
> > 
> > report = saa_readl(SAA7134_IRQ_REPORT);
> > (...)
> > if ((report & SAA7134_IRQ_REPORT_RDCAP) || (report & SAA7134_IRQ_REPORT_INTL))
> > 	saa7134_irq_video_signalchange(dev);
> > 
> > But I'm not v4l expert, so...
> 
> With most chip interrupt status registers, you write the flags you want
> to clear.
> 
> That way you can always do something like:
> 
> 	a = read(ISR_REG);
> 	write(ISR_REG,a);
> 
> to clear the Interrupt status register.
> 
> Note what saa7134_irq() does:
> 
>                 report = saa_readl(SAA7134_IRQ_REPORT);
> 		[...]
>                 saa_writel(SAA7134_IRQ_REPORT,report);
> 		[...]
>                 if ((report & SAA7134_IRQ_REPORT_RDCAP) ||
>                         (report & SAA7134_IRQ_REPORT_INTL))
>                                 saa7134_irq_video_signalchange(dev);
> 
> 
> So I'm thinking writing 0 to the register won't have the desired effect.
> 
> Again, thanks for taking the initiative.
> 
> Regards,
> Andy
> 
> > > Marcin has mapped out the oops from there.
> > > 
> > > So the solution, I'm guessing, is likely to clear the SAA7134_IRQ_REPORT
> > > register in saa7134_hwinit1().
> > > 
> > > If only I had a datasheet, hardware, spare time... ;)
> > > 

Hartmut has the register programming instructions under NDA and some
others obviously too.

Must have been very lucky all that time ...

Cheers,
Hermann




--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux