Re: [PATCH v6 3/3] ath11k: add IPQ6018 support

From: Kalle Valo <kvalo@codeaurora.org>
To: Anilkumar Kolli <akolli@codeaurora.org>
Cc: ath11k@lists.infradead.org, devicetree@vger.kernel.org,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH v6 3/3] ath11k: add IPQ6018 support
Date: Mon, 7 Sep 2020 10:01:30 +0000	[thread overview]
Message-ID: <0101017468037386-257bc211-f06b-4c45-87ea-aba74c7c68e7-000000@us-west-2.amazonses.com> (raw)
In-Reply-To: <1598287470-1871-4-git-send-email-akolli@codeaurora.org> (Anilkumar Kolli's message of "Mon, 24 Aug 2020 22:14:30 +0530")

Anilkumar Kolli <akolli@codeaurora.org> writes:

> IPQ6018 has one 5G and one 2G radio with 2x2,
> shares ipq8074 configurations.
>
> Signed-off-by: Anilkumar Kolli <akolli@codeaurora.org>

[...]

> @@ -890,9 +995,15 @@ static int ath11k_ahb_probe(struct platform_device *pdev)
>  		goto err_hal_srng_deinit;
>  	}
>  
> -	ab->hw_params.svc_to_ce_map_len =
> +	if (ab->hw_rev == ATH11K_HW_IPQ8074) {
> +		ab->hw_params.svc_to_ce_map_len =
>  			ARRAY_SIZE(target_service_to_ce_map_wlan_ipq8074);
> -	ab->hw_params.svc_to_ce_map = target_service_to_ce_map_wlan_ipq8074;
> +		ab->hw_params.svc_to_ce_map = target_service_to_ce_map_wlan_ipq8074;
> +	} else if (ab->hw_rev == ATH11K_HW_IPQ6018) {
> +		ab->hw_params.svc_to_ce_map_len =
> +			ARRAY_SIZE(target_service_to_ce_map_wlan_ipq6018);
> +		ab->hw_params.svc_to_ce_map = target_service_to_ce_map_wlan_ipq6018;
> +	}

The idea of having ab->hw_params is to avoid the need for hw_rev
sprinkled all over the code, this here defeats the purpose. I recommend
checking if my proposal from an earlier version is implementable:

"You could copy the map arrays to hw.c, as an example see how
 ath11k_hw_ring_mask_ipq8074 is implemented. That way we don't need any
 new hw_rev checks."

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