From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michal Kazior <michal.kazior@tieto.com>
Date: Fri, 14 Jun 2013 14:02:00 +0200
Subject: [ath9k-devel] [PATCH v2 01/10] ath10k: decouple pci init/deinit
 logic
In-Reply-To: <87wqpxnarh.fsf@kamboji.qca.qualcomm.com>
References: <1371041642-20273-1-git-send-email-michal.kazior@tieto.com>
	<1371041642-20273-2-git-send-email-michal.kazior@tieto.com>
	<87wqpxnarh.fsf@kamboji.qca.qualcomm.com>
Message-ID: <51BB0638.9020802@tieto.com>
List-Id: <ath9k-devel.lists.ath9k.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: ath9k-devel@lists.ath9k.org

On 13/06/13 20:09, Kalle Valo wrote:
> So when booting the firwmware we call:
>
> hif_power_up()
> hif_start()
>
> and when we want to kill the firmware we do:
>
> hif_stop()
> hif_power_down()
>
> [...]

You're right. I'll fix that.


> I noticed ath10k_pci_ce_init() also allocs host memory etc. If possible,
> in the future we might want to refactor the function into two:
> ath10k_pci_ce_init() and ath10k_pci_ce_start(). And the former would be
> called only from ath10k_pci_probe(). That way we would not need to do
> any memory allocation during start time.
>
> But no need to refactor it right now, this is good enough for the first
> implementation.

There's also some an asymmetry here.

We should be enabling/disabling irqs when we start/stop, shouldn't we? 
But then again we need interrupts _before_ start() for BMI phase, while 
it's a also a good idea to disable them upon stop(). We do stop CE 
interrupts by poking up device registers, but we don't unregister irq 
handlers. There might be a race in there that we're not aware 
of/observed yet.

I'm thinking of having only 3 functions: start_bmi_phase, 
start_htc_phase, stop (that would handle either state transition). But 
that would require the aforementioned proper alloc/init/setup separation 
to be done first.


-- Pozdrawiam / Best regards, Michal Kazior.