Re: [PATCH 4/4] ath11k: Register mhi controller device for qca6390

From: Kalle Valo <kvalo@codeaurora.org>
To: Govind Singh <govinds@codeaurora.org>
Cc: linux-wireless@vger.kernel.org, ath11k@lists.infradead.org
Subject: Re: [PATCH 4/4] ath11k: Register mhi controller device for qca6390
Date: Mon, 11 May 2020 21:08:24 +0300	[thread overview]
Message-ID: <878shy4a5j.fsf@kamboji.qca.qualcomm.com> (raw)
In-Reply-To: <20200508085850.23363-5-govinds@codeaurora.org> (Govind Singh's message of "Fri, 8 May 2020 14:28:50 +0530")

Govind Singh <govinds@codeaurora.org> writes:

> MHI is a communication protocol to communicate with external
> Qualcomm modems and Wi-Fi chipsets over high speed peripheral buses. Even
> though MHI doesn’t dictate underlying physical layer, protocol and mhi stack
> is structured for PCIe based devices.
>
> Register directly with mhi core layer as a mhi device driver for
> firmware download.
>
> Tested QCA6390 on X86 platform.
> Tested firmware WLAN.HST.1.0.1.c1-00440-QCAHSTSWPLZ_V2_TO_X86-1.
>
> Signed-off-by: Govind Singh <govinds@codeaurora.org>

[...]

> +int ath11k_pci_start_mhi(struct ath11k_pci *ar_pci);
> +void ath11k_pci_stop_mhi(struct ath11k_pci *ar_pci);
> +int ath11k_pci_register_mhi(struct ath11k_pci *ar_pci);
> +void ath11k_pci_unregister_mhi(struct ath11k_pci *ar_pci);

The schema for filenames is this:

ath11k_<filename>_<functionname>

So I changed all functions in mhi.c to follow that.

> +static int ath11k_pci_qca6x90_powerup(struct ath11k_pci *ab_pci)
> +{
> +	return ath11k_pci_start_mhi(ab_pci);
> +}
> +
> +static void ath11k_pci_qca6x90_powerdown(struct ath11k_pci *ab_pci)
> +{
> +	ath11k_pci_stop_mhi(ab_pci);
> +}

These looked like useless functions so I removed them.

> +static const struct ath11k_hif_ops ath11k_pci_hif_ops = {
> +	.power_down = ath11k_pci_power_down,
> +	.power_up = ath11k_pci_power_up,
> +};

I added __always_unused as a temporary solution to avoid a compiler
warning, but we should remove that once we really use this.

> @@ -229,14 +313,26 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
>  		goto err_free_core;
>  	}
>  
> +	ab->mem = ab_pci->mem;
> +	ab->mem_len = ab_pci->mem_len;
> +
>  	ret = ath11k_pci_enable_msi(ab_pci);
>  	if (ret) {
>  		ath11k_err(ab, "failed to enable  msi: %d\n", ret);
>  		goto err_pci_free_region;
>  	}
>  
> +	ret = ath11k_pci_register_mhi(ab_pci);
> +	if (ret) {
> +		ath11k_err(ab, "failed to register  mhi: %d\n", ret);
> +		goto err_pci_disable_msi;
> +	}
> +
>  	return 0;

I added a warning here that PCI is work-in-progress. Oh, I need to add
that also to Kconfig.

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

_______________________________________________
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k