Re: [PATCH] OMAPDSS: HDMI: Discard phy_tx_enabled member

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

 



On Mon, 2012-06-18 at 17:16 +0530, Jassi Brar wrote:
> On 18 June 2012 16:24, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
> > On Mon, 2012-06-18 at 15:42 +0530, Jassi Brar wrote:
> 
> >> BTW, coming to think about it, I am not sure what we need the
> >> spin_lock_irqsave() protection for in hdmi_check_hpd_state() ?  It
> >> can't control HPD gpio state change and hdmi_set_phy_pwr() seems too
> >> expensive and is already unprotected elsewhere.
> >
> > It's needed when enabling the hdmi output. In phy_enable() the irq is
> > requested first, and then the phy_enable() runs hdmi_check_hpd_state().
> > So there's a chance to run hdmi_check_hpd_state() from both
> > hpd-interrupt and phy_enable() at the same time.
> >
> > The hdmi_set_phy_pwr() is not called in many places, but I think there's
> > indeed a problem there. It is called after free_irq(), but I think
> > (guess) the irq handler could still be running after free_irq. So those
> > should be protected by the same spinlock too.
> >
> You know TI HDMI better than I do, so I assume your concerns are valid.
> So preferably I would move request_threaded_irq() to after
> hdmi_check_hpd_state() in ti_hdmi_4xxx_phy_enable()  and convert the

No, you can't move the check. If you move it, the HPD state could change
between the check and the request_irq, and we'd miss it.

> spin_lock_irqsave() in hdmi_check_hpd_state() to some mutex (we don't
> want irqs disabled so long as it takes for phy to power on/off).

Yes, I guess a mutex is better.

 Tomi

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux