[PATCH v5 2/2] ath11k: add read variant from SMBIOS for download board data
Wen Gong
quic_wgong at quicinc.com
Mon Mar 14 03:35:12 PDT 2022
On 3/10/2022 4:12 PM, Kalle Valo wrote:
> Wen Gong <quic_wgong at quicinc.com> writes:
>
>> This is to read variant from SMBIOS such as read from DT, the variant
>> string will be used to one part of string which used to search board
>> data from board-2.bin.
>>
>> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>>
>> Signed-off-by: Wen Gong <quic_wgong at quicinc.com>
> [...]
>
>> +static void ath11k_core_check_bdfext(const struct dmi_header *hdr, void *data)
>> +{
>> + struct ath11k_base *ab = data;
>> + const char *bdf_ext;
>> + const char *magic = ATH11K_SMBIOS_BDF_EXT_MAGIC;
>> + u8 bdf_enabled;
>> + int i;
>> + size_t len;
>> +
>> + if (ab->qmi.target.bdf_ext[0] != '\0')
>> + return;
>> +
>> + if (hdr->type != ATH11K_SMBIOS_BDF_EXT_TYPE)
>> + return;
>> +
>> + if (hdr->length != ATH11K_SMBIOS_BDF_EXT_LENGTH) {
>> + ath11k_dbg(ab, ATH11K_DBG_BOOT,
>> + "wrong smbios bdf ext type length (%d).\n",
>> + hdr->length);
>> + return;
>> + }
>> +
>> + bdf_enabled = *((u8 *)hdr + ATH11K_SMBIOS_BDF_EXT_OFFSET);
>> + if (!bdf_enabled) {
>> + ath11k_dbg(ab, ATH11K_DBG_BOOT, "bdf variant name not found.\n");
>> + return;
>> + }
>> +
>> + /* Only one string exists (per spec) */
>> + bdf_ext = (char *)hdr + hdr->length;
> A proper struct is preferred over pointer arithmetic. For example
> something like this:
>
> struct ath11k_smbios_bdf {
> struct dmi_header hdr;
> u32 padding;
> u8 bdf_enabled;
> u8 bdf_ext[ATH11K_SMBIOS_BDF_EXT_MAX_LEN];
> }
>
> I'm not sure if I got the offsets right, but I hope you get the idea
> anyway.
Will change it.
>> +
>> + if (memcmp(bdf_ext, magic, strlen(magic)) != 0) {
>> + ath11k_dbg(ab, ATH11K_DBG_BOOT,
>> + "bdf variant magic does not match.\n");
>> + return;
>> + }
>> +
>> + len = strlen(bdf_ext);
> What if bdf_ext is not null terminated? Wouldn't strnlen() with
> ATH11K_SMBIOS_BDF_EXT_MAX_LEN would be safer?
Yes, will change it.
>
>> --- a/drivers/net/wireless/ath/ath11k/core.h
>> +++ b/drivers/net/wireless/ath/ath11k/core.h
>> @@ -971,7 +971,18 @@ int ath11k_core_fetch_bdf(struct ath11k_base *ath11k,
>> struct ath11k_board_data *bd);
>> void ath11k_core_free_bdf(struct ath11k_base *ab, struct ath11k_board_data *bd);
>> int ath11k_core_check_dt(struct ath11k_base *ath11k);
>> +/* SMBIOS type containing Board Data File Name Extension */
>> +#define ATH11K_SMBIOS_BDF_EXT_TYPE 0xF8
>>
>> +/* SMBIOS type structure length (excluding strings-set) */
>> +#define ATH11K_SMBIOS_BDF_EXT_LENGTH 0x9
>> +
>> +/* Offset pointing to Board Data File Name Extension */
>> +#define ATH11K_SMBIOS_BDF_EXT_OFFSET 0x8
>> +
>> +/* The magic used by QCA spec */
>> +#define ATH11K_SMBIOS_BDF_EXT_MAGIC "BDF_"
>> +int ath11k_core_check_smbios(struct ath11k_base *ab);
>> void ath11k_core_halt(struct ath11k *ar);
>> int ath11k_core_resume(struct ath11k_base *ab);
>> int ath11k_core_suspend(struct ath11k_base *ab);
> Please don't mix defines and function declarations, so move defines up
> in the file.
Yes, will change it.
More information about the ath11k
mailing list