Thread (9 messages) 9 messages, 2 authors, 2013-08-05

Re: [PATCH v2] ath10k: fix device teardown

From: Kalle Valo <hidden>
Date: 2013-08-02 08:00:37

Michal Kazior [off-list ref] writes:
On 2 August 2013 09:41, Kalle Valo [off-list ref] wrote:
quoted
Michal Kazior [off-list ref] writes:
quoted
@@ -1742,6 +1761,12 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
 {
      int ret;

+     ret = ath10k_pci_start_intr(ar);
+     if (ret) {
+             ath10k_err("could not start interrupt handling (%d)\n", ret);
+             goto err;
+     }
So now we call start_intr() during power_up(), which means that we do
the request_irq() calls during every interface up event. Does that cause
any meaningful overhead?
I don't think so.
Good.
quoted
For me it looks better to do all resource allocation in
ath10k_pci_probe(), like request_irq(), and free the resources in
ath10k_pci_remove(). But then we would need to immeadiately call
disable_irq() and then enable_irq() from power_up() so I'm not sure if
that's any better.
Not only that. Since disable/enable_irq must be balanced we'd need
some way to track whether we have irqs enabled/disabled - either with
an extra bool variable, additional ath10k_states or new pci-specific
states.

The patch assumes disable_irq is followed by free_irq (which it is)
and possibly request_irq later on.
Yeah, your v2 sounds much better. And if there's overhead or something
else we can always change this later.

I'll wait for comments from others and if I don't get any, I'll apply
this.

-- 
Kalle Valo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help