Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>>> @@ -1905,22 +1915,10 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
>>> goto err;
>>> }
>>>
>>> - ret = ath10k_ce_disable_interrupts(ar);
>>> - if (ret) {
>>> - ath10k_err("failed to disable CE interrupts: %d\n", ret);
>>> - goto err_ce;
>>> - }
>>> -
>>> - ret = ath10k_pci_init_irq(ar);
>>> - if (ret) {
>>> - ath10k_err("failed to init irqs: %d\n", ret);
>>> - goto err_ce;
>>> - }
>>> -
>>> ret = ath10k_pci_request_early_irq(ar);
>>> if (ret) {
>>> ath10k_err("failed to request early irq: %d\n", ret);
>>> - goto err_deinit_irq;
>>> + goto err_ce;
>>> }
>>
>> You add ath10k_pci_ce_init() to probe() and respective
>> ath10k_pci_ce_deinit() to remove(), and you remove
>> ath10k_pci_ce_deinit() from hif_power_down(). But why do you leave
>> ath10k_pci_ce_init() to hif_power_up()? Isn't that unnecessary as we
>> already do that in probe()?
>
> Hmm.. I didn't check if copy engine register state (notable ring index
> values) is guaranteed to be usable in all cases yet and decided it's
> safer to just re-init it on each power up until it is proven it is not
> necessary.
So I have been trying to follow this naming scheme for initilisation:
_create() - called during device probe time
_start() - if up / power up
_stop() - if down / power down
_destroy() - called when device is removed
If you call ce_init() both in create() and start() time there has to be
some redundant code and I think the function should be split. This isn't
a concern now, but something for the future.
--
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
