Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: wgong=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Tue, 09 Nov 2021 17:51:13 +0800
From:   Wen Gong <wgong@codeaurora.org>
To:     Kalle Valo <kvalo@codeaurora.org>
Cc:     Jouni Malinen <jouni@codeaurora.org>, ath11k@lists.infradead.org,
        linux-wireless@vger.kernel.org
Subject: Re: [PATCH] ath11k: add string type to search board data in
 board-2.bin for WCN6855
In-Reply-To: <87mtmepz3z.fsf@codeaurora.org>
References: <20210913181131.194024-1-jouni@codeaurora.org>
 <87mtmepz3z.fsf@codeaurora.org>
Message-ID: <802a09c8fb8f284a2961f229353b9590@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2021-11-08 21:15, Kalle Valo wrote:
> Jouni Malinen <jouni@codeaurora.org> writes:
...
>> +	switch (ab->id.bdf_search) {
>> +	case ATH11K_BDF_SEARCH_BUS_AND_BOARD:
>> +		scnprintf(name, name_len,
>> +			  
>> "bus=%s,vendor=%04x,device=%04x,subsystem-vendor=%04x,subsystem-device=%04x,qmi-chip-id=%d,qmi-board-id=%d%s",
>> +			  ath11k_bus_str(ab->hif.bus),
>> +			  ab->id.vendor, ab->id.device,
>> +			  ab->id.subsystem_vendor,
>> +			  ab->id.subsystem_device,
>> +			  FIELD_GET(ATH11K_CHIP_ID_MASK, ab->qmi.target.chip_id),
>> +			  ab->qmi.target.board_id & 0xFF,
> 
> Why are chip_id and board_id masked? Why cannot we use values directly
> provided by the firmware?
> 
below is the log for WCN6855 2.0, its chip_id is 0x2 and board_id is 
0x106, but actually we need to
find the board data with chip-id=0 and board-id=6, so we add mask here.

[ 3000.176621] ath11k_pci 0000:05:00.0: chip_id 0x2 chip_family 0xb 
board_id 0x106 soc_id 0x400c0200
[ 3000.182361] ath11k_pci 0000:05:00.0: boot using board name 
'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=3374,qmi-chip-id=0,qmi-board-id=6'

> And if we need to mask those, it's better to do them in qmi.c where 
> they
> are stored:
> 
Currenly logic of default: in ath11k_core_create_board_name() which is 
introduced in 1st commit
also use the chip_id and board_id, if mask them in qmi.c may effect the 
logic of ath11k_core_create_board_name().

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/drivers/net/wireless/ath/ath11k?h=ath-next&id=d5c65159f2895379e11ca13f62feabe93278985d
ath11k: driver for Qualcomm IEEE 802.11ax devices
+static int ath11k_core_create_board_name(struct ath11k_base *ab, char 
*name,
+					 size_t name_len)
+{
+	/* Note: bus is fixed to ahb. When other bus type supported,
+	 * make it to dynamic.
+	 */
+	scnprintf(name, name_len,
+		  "bus=ahb,qmi-chip-id=%d,qmi-board-id=%d",
+		  ab->qmi.target.chip_id,
+		  ab->qmi.target.board_id);
+
+	ath11k_dbg(ab, ATH11K_DBG_BOOT, "boot using board name '%s'\n", name);
+
+	return 0;
+}

another thing is that, other bits of chip_id/board_id maybe also need 
used by other place/feature, so we still need to
store the total original value of chip_id/board_id instead of mask it 
only for board data file.
Then user can easily get their own mask from chip_id/board_id.

> 	if (resp.chip_info_valid) {
> 		ab->qmi.target.chip_id = resp.chip_info.chip_id;
> 		ab->qmi.target.chip_family = resp.chip_info.chip_family;
> 	}
> 
> 	if (resp.board_info_valid)
> 		ab->qmi.target.board_id = resp.board_info.board_id;
> 	else
> 		ab->qmi.target.board_id = 0xFF;