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.
