Re: [PATCH 1/6] ath11k: Add basic WoW functionalities

[Kalle Valo <kvalo@kernel.org>]

Carl Huang <cjhuang@codeaurora.org> writes:

> Implement basic WoW functionalities such as magic-packet, disconnect
> and pattern. The logic is very similar to ath10k.
>
> When WoW is configured, ath11k_core_suspend and ath11k_core_resume
> are skipped as WoW configuration and hif suspend/resume are done in
> ath11k_wow_op_suspend() and ath11k_wow_op_resume().
>
> 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: Baochen Qiang <bqiang@codeaurora.org>

[...]

>  int ath11k_core_suspend(struct ath11k_base *ab)
>  {
>  	int ret;
> +	struct ath11k *ar;
>  
>  	if (!ab->hw_params.supports_suspend)
>  		return -EOPNOTSUPP;
>  
> +	/* so far single_pdev_only chips have supports_suspend as true
> +	 * and only the first pdev is valid.
> +	 */
> +	ar = ab->pdevs[0].ar;

I'm a bit worried about this assumption, it might change in the future.
What about a helper like this to add a test for it:

static inline struct ath11k_pdev *ath11k_core_get_single_pdev()
{
        WARN_ON(!ab->hw_params.single_pdev_only);

        return ab->pdevs[0];
}

>  int ath11k_core_resume(struct ath11k_base *ab)
>  {
>  	int ret;
> +	struct ath11k *ar;
>  
>  	if (!ab->hw_params.supports_suspend)
>  		return -EOPNOTSUPP;
>  
> +	/* so far signle_pdev_only chips have supports_suspend as true
> +	 * and only the first pdev is valid.
> +	 */
> +	ar = ab->pdevs[0].ar;
> +	if (!ar || ar->state != ATH11K_STATE_OFF)
> +		return 0;

The same helper could be used here.

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

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