Michal Kazior <michal.kazior@xxxxxxxxx> writes:
> On 2 August 2013 09:41, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
>> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>>
>>> @@ -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.
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
