Re: [PATCH v2] PCI: rockchip: fix system hang up if activating CONFIG_DEBUG_SHIRQ

[jeffy <jeffy.chen@rock-chips.com>]

Hi Heiko,

On 08/10/2017 08:22 PM, Heiko Stuebner wrote:
>>
>> >+	devm_add_action_or_reset(dev,
>> >+				 rockchip_pcie_disable_clocks, rockchip);
>> >+
> err = devm_add_action_or_reset(...)
> if (err) {
> ...
> }
>
> devm_add_action_or_reset can fail. When it fails, it will call the
> action already, so your error handling does not need to disable
> clocks on its own.
>
>
> Also, as a more general comment, right now you do devm_request_irq
> from rockchip_pcie_parse_dt, which gets called_before_  clocks are
> enabled.
>
> This will likely bite you at some point as well, as the irq can fire at
> any point after it got requested ... including before you enable the
> clocks.
>
> So the order should probably be
>
> - enable clocks
> - register devm_action to shutdown clocks
> - parse_dt including requesting the irq.
>
>
> Heiko

it turns out this irq handler not only depends on clks, but also pm 
domain :(

so handling it with devm_* functions would be a little 
complicated(https://lkml.org/lkml/2017/8/15/146).

would it make sense to use request_irq/free_irq directly?

or maybe add a flag(for example probed), and check it in the irq handler 
before read registers? then we would just need to make sure the priv 
struct be freed later than the irq.

>
>