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

[akolli@xxxxxxxxxxxxxx]

On 2020-09-07 15:30, Kalle Valo wrote:
> Anilkumar Kolli <akolli@xxxxxxxxxxxxxx> writes:
>
> > IPQ6018 has one 5G and one 2G radio with 2x2,
> > shares ipq8074 configurations.
> >
> > Signed-off-by: Anilkumar Kolli <akolli@xxxxxxxxxxxxxx>
>
> [...]
>
> > @@ -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."

Thanks for the details, will address and send in next patch set.

Thanks
Anil