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
