| Message ID | 20211220064829.17557-3-quic_wgong@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | ath11k: add handler for board-2.bin without variant and read SMBIOS | expand |
Wen Gong <quic_wgong@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@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. > + > + 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.
On 3/10/2022 4:12 PM, Kalle Valo wrote: > Wen Gong <quic_wgong@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@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.
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c index 641c133563ab..30ae68ac7515 100644 --- a/drivers/net/wireless/ath/ath11k/core.c +++ b/drivers/net/wireless/ath/ath11k/core.c @@ -8,6 +8,9 @@ #include <linux/remoteproc.h> #include <linux/firmware.h> #include <linux/of.h> +#include <linux/dmi.h> +#include <linux/ctype.h> + #include "core.h" #include "dp_tx.h" #include "dp_rx.h" @@ -460,6 +463,77 @@ int ath11k_core_resume(struct ath11k_base *ab) } EXPORT_SYMBOL(ath11k_core_resume); +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; + + 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); + for (i = 0; i < len; i++) { + if (!isascii(bdf_ext[i]) || !isprint(bdf_ext[i])) { + ath11k_dbg(ab, ATH11K_DBG_BOOT, + "bdf variant name contains non ascii chars.\n"); + return; + } + } + + /* Copy extension name without magic prefix */ + if (strscpy(ab->qmi.target.bdf_ext, bdf_ext + strlen(magic), + sizeof(ab->qmi.target.bdf_ext)) < 0) { + ath11k_dbg(ab, ATH11K_DBG_BOOT, + "bdf variant string is longer than the buffer can accommodate (variant: %s)\n", + bdf_ext); + return; + } + + ath11k_dbg(ab, ATH11K_DBG_BOOT, + "found and validated bdf variant smbios_type 0x%x bdf %s\n", + ATH11K_SMBIOS_BDF_EXT_TYPE, bdf_ext); +} + +int ath11k_core_check_smbios(struct ath11k_base *ab) +{ + ab->qmi.target.bdf_ext[0] = '\0'; + dmi_walk(ath11k_core_check_bdfext, ab); + + if (ab->qmi.target.bdf_ext[0] == '\0') + return -ENODATA; + + return 0; +} + int ath11k_core_check_dt(struct ath11k_base *ab) { size_t max_len = sizeof(ab->qmi.target.bdf_ext); diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h index bab3878a41a7..c0b8f5ab09c9 100644 --- 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); diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c index 3b9ba0e03a66..f84a83af145d 100644 --- a/drivers/net/wireless/ath/ath11k/qmi.c +++ b/drivers/net/wireless/ath/ath11k/qmi.c @@ -2037,6 +2037,10 @@ static int ath11k_qmi_request_target_cap(struct ath11k_base *ab) ab->qmi.target.fw_build_timestamp, ab->qmi.target.fw_build_id); + r = ath11k_core_check_smbios(ab); + if (r) + ath11k_dbg(ab, ATH11K_DBG_QMI, "SMBIOS bdf variant name not set.\n"); + r = ath11k_core_check_dt(ab); if (r) ath11k_dbg(ab, ATH11K_DBG_QMI, "DT bdf variant name not set.\n");
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@quicinc.com> --- drivers/net/wireless/ath/ath11k/core.c | 74 ++++++++++++++++++++++++++ drivers/net/wireless/ath/ath11k/core.h | 11 ++++ drivers/net/wireless/ath/ath11k/qmi.c | 4 ++ 3 files changed, 89 insertions(+)