Wen Gong <quic_wgong@xxxxxxxxxxx> 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@xxxxxxxxxxx>
[...]
> +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.
> +
> + 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?
> --- 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.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
