Sender: kvalo=codeaurora.org@mg.codeaurora.org
DMARC-Filter: OpenDMARC Filter v1.4.1 smtp.codeaurora.org 881D6C4338F
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Baochen Qiang <bqiang@codeaurora.org>
Cc:     ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH 4/7] ath11k: refactor multiple MSI vector implementation
References: <20211026041705.5167-1-bqiang@codeaurora.org>
Date:   Fri, 19 Nov 2021 14:43:25 +0200
In-Reply-To: <20211026041705.5167-1-bqiang@codeaurora.org> (Baochen Qiang's
        message of "Tue, 26 Oct 2021 12:17:05 +0800")
Message-ID: <87lf1kco42.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Baochen Qiang <bqiang@codeaurora.org> writes:

> From: Carl Huang <cjhuang@codeaurora.org>
>
> This is to prepare for one MSI vector support. IRQ enable and disable
> of CE and DP are done only in case of multiple MSI vectors.
>
> Tested-on: QCA6390 hw2.0 PCI WLAN.HST.1.0.1-01740-QCAHSTSWPLZ_V2_TO_X86-1
>
> Signed-off-by: Carl Huang <cjhuang@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Baochen Qiang <bqiang@codeaurora.org>

[...]

> @@ -558,6 +558,13 @@ static void ath11k_pci_free_irq(struct ath11k_base *ab)
>  static void ath11k_pci_ce_irq_enable(struct ath11k_base *ab, u16 ce_id)
>  {
>  	u32 irq_idx;
> +	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);

ab_pci should be before irq_idx to follow the reverse xmas tree style, I
fixed it in the pending branch.

> +
> +	/*In case of one MSI vector, we handle irq enable/disable
> +	 *in a uniform way since we only have one irq
> +	 */

There should be a space after '*', fixed now.

> +	if (!test_bit(ATH11K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
> +		return;
>  
>  	irq_idx = ATH11K_PCI_IRQ_CE0_OFFSET + ce_id;
>  	enable_irq(ab->irq_num[irq_idx]);
> @@ -566,6 +573,13 @@ static void ath11k_pci_ce_irq_enable(struct ath11k_base *ab, u16 ce_id)
>  static void ath11k_pci_ce_irq_disable(struct ath11k_base *ab, u16 ce_id)
>  {
>  	u32 irq_idx;
> +	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);
> +
> +	/*In case of one MSI vector, we handle irq enable/disable
> +	 *in a uniform way since we only have one irq
> +	 */
> +	if (!test_bit(ATH11K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
> +		return;

Both style issues fixed here as well.

>  static void ath11k_pci_ext_grp_disable(struct ath11k_ext_irq_grp *irq_grp)
>  {
>  	int i;
> +	struct ath11k_pci *ab_pci = ath11k_pci_priv(irq_grp->ab);
> +
> +	/*In case of one MSI vector, we handle irq enable/disable
> +	 *in a uniform way since we only have one irq
> +	 */
> +	if (!test_bit(ATH11K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
> +		return;

And here.

>  
>  	for (i = 0; i < irq_grp->num_irq; i++)
>  		disable_irq_nosync(irq_grp->ab->irq_num[irq_grp->irqs[i]]);
> @@ -651,6 +672,13 @@ static void __ath11k_pci_ext_irq_disable(struct ath11k_base *sc)
>  static void ath11k_pci_ext_grp_enable(struct ath11k_ext_irq_grp *irq_grp)
>  {
>  	int i;
> +	struct ath11k_pci *ab_pci = ath11k_pci_priv(irq_grp->ab);
> +
> +	/*In case of one MSI vector, we handle irq enable/disable
> +	 *in a uniform way since we only have one irq
> +	 */
> +	if (!test_bit(ATH11K_PCI_FLAG_MULTI_MSI_VECTORS, &ab_pci->flags))
> +		return;

And here.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches