akolli@xxxxxxxxxxxxxx writes: > On 2020-06-19 04:40, Julian Calaby wrote: >> >> On Thu, Jun 18, 2020 at 3:31 PM Anilkumar Kolli >> <akolli@xxxxxxxxxxxxxx> wrote: >>> >>> @@ -950,6 +950,16 @@ static int ath11k_ahb_probe(struct >>> platform_device *pdev) >>> goto err_hal_srng_deinit; >>> } >>> >>> + ret = ath11k_init_hw_params(ab); >>> + if (ret) { >>> + ath11k_err(ab, "failed to get hw params %d\n", ret); >>> + return ret; >>> + } >>> + >>> + 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; >> >> I think you misunderstood my point about this, the point wasn't to >> copy the svc map to hw_params, but define it in hw_params: >> >> + { >> + .hw_rev = ATH11K_HW_IPQ6018, >> + .name = "ipq6018 hw1.0", >> + .fw = { >> + .dir = "IPQ6018/hw1.0", >> + .board_size = 256 * 1024, >> + .cal_size = 256 * 1024, >> + }, >> + .max_radios = 2, >> + .bdf_addr = 0x4ABC0000, >> + .hw_ops = &ipq6018_ops, >> + .svc_to_ce_map_len = >> ARRAY_SIZE(target_service_to_ce_map_wlan_ipq6018, >> + .svc_to_ce_map = target_service_to_ce_map_wlan_ipq6018, >> + }, >> >> That completely eliminates special case code based on the hardware ID >> in the driver. >> > The static array of structures target_service_to_ce_map_wlan_ipq6018[] > is defined in ahb.c and hw_params are initialised in core.c, this will > not work. no? You could move 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