mhi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ath11k: support firmware-2.bin
@ 2023-01-11  9:25 Kalle Valo
  2023-01-11  9:25 ` [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer Kalle Valo
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kalle Valo @ 2023-01-11  9:25 UTC (permalink / raw)
  To: mhi; +Cc: ath11k, linux-wireless

From: Kalle Valo <quic_kvalo@quicinc.com>

We need firmware-2.bin support in ath11k so that we can add ath11k specific meta
data to firmware releases, for example feature flags so that ath11k can
automatically detect what features the firmware release supports.  Also makes
it easier and more reliable to update the firmware for PCI devices as it's not
possible to mix firmware files, everything will be in one file.

Please review and comment. It would be easier if I could take the MHI patch
(patch 1) to my ath.git tree along with the rest of the patches but let's
discuss that separately.

Anilkumar Kolli (1):
  ath11k: add firmware-2.bin support

Kalle Valo (2):
  mhi: allow MHI client drivers to provide the firmware via a pointer
  ath11k: qmi: refactor ath11k_qmi_m3_load()

 drivers/bus/mhi/host/boot.c              |  27 ++--
 drivers/net/wireless/ath/ath11k/Makefile |   3 +-
 drivers/net/wireless/ath/ath11k/core.c   |   8 ++
 drivers/net/wireless/ath/ath11k/core.h   |  15 +++
 drivers/net/wireless/ath/ath11k/fw.c     | 157 +++++++++++++++++++++++
 drivers/net/wireless/ath/ath11k/fw.h     |  27 ++++
 drivers/net/wireless/ath/ath11k/mhi.c    |  18 ++-
 drivers/net/wireless/ath/ath11k/qmi.c    |  54 +++++---
 include/linux/mhi.h                      |   6 +
 9 files changed, 283 insertions(+), 32 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath11k/fw.c
 create mode 100644 drivers/net/wireless/ath/ath11k/fw.h


base-commit: e51980bf88c65f6ad4916baf720ad1234de01791
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer
  2023-01-11  9:25 [PATCH 0/3] ath11k: support firmware-2.bin Kalle Valo
@ 2023-01-11  9:25 ` Kalle Valo
  2023-01-11 15:20   ` Jeffrey Hugo
  2023-01-11  9:25 ` [PATCH 2/3] ath11k: qmi: refactor ath11k_qmi_m3_load() Kalle Valo
  2023-01-11  9:25 ` [PATCH 3/3] ath11k: add firmware-2.bin support Kalle Valo
  2 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2023-01-11  9:25 UTC (permalink / raw)
  To: mhi; +Cc: ath11k, linux-wireless

From: Kalle Valo <quic_kvalo@quicinc.com>

Currently MHI loads the firmware image from the path provided by client
devices. ath11k needs to support firmware image embedded along with meta data
(named as firmware-2.bin). So allow the client driver to request the firmware
file from user space on it's own and provide the firmware image data and size
to MHI via a pointer struct mhi_controller::fw_data.

This is an optional feature, if fw_data is NULL MHI load the firmware using the
name from struct mhi_controller::fw_image string as before.

Tested with ath11k and WCN6855 hw2.0.

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/bus/mhi/host/boot.c | 27 +++++++++++++++++++--------
 include/linux/mhi.h         |  6 ++++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
index 1c69feee1703..d6d5f424491e 100644
--- a/drivers/bus/mhi/host/boot.c
+++ b/drivers/bus/mhi/host/boot.c
@@ -365,12 +365,10 @@ int mhi_alloc_bhie_table(struct mhi_controller *mhi_cntrl,
 }
 
 static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
-			      const struct firmware *firmware,
+			      const u8 *buf, size_t remainder,
 			      struct image_info *img_info)
 {
-	size_t remainder = firmware->size;
 	size_t to_cpy;
-	const u8 *buf = firmware->data;
 	struct mhi_buf *mhi_buf = img_info->mhi_buf;
 	struct bhi_vec_entry *bhi_vec = img_info->bhi_vec;
 
@@ -392,9 +390,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	const struct firmware *firmware = NULL;
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
 	const char *fw_name;
+	const u8 *fw_data;
 	void *buf;
 	dma_addr_t dma_addr;
-	size_t size;
+	size_t size, fw_sz;
 	int i, ret;
 
 	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
@@ -424,6 +423,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	fw_name = (mhi_cntrl->ee == MHI_EE_EDL) ?
 		mhi_cntrl->edl_image : mhi_cntrl->fw_image;
 
+	if (!fw_name && mhi_cntrl->fbc_download &&
+	    mhi_cntrl->fw_data && mhi_cntrl->fw_sz) {
+		size = mhi_cntrl->sbl_size;
+		fw_data = mhi_cntrl->fw_data;
+		fw_sz = mhi_cntrl->fw_sz;
+		goto skip_req_fw;
+	}
+
 	if (!fw_name || (mhi_cntrl->fbc_download && (!mhi_cntrl->sbl_size ||
 						     !mhi_cntrl->seg_len))) {
 		dev_err(dev,
@@ -443,6 +450,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	if (size > firmware->size)
 		size = firmware->size;
 
+	fw_data = firmware->data;
+	fw_sz = firmware->size;
+
+skip_req_fw:
 	buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, size, &dma_addr,
 				 GFP_KERNEL);
 	if (!buf) {
@@ -451,7 +462,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	}
 
 	/* Download image using BHI */
-	memcpy(buf, firmware->data, size);
+	memcpy(buf, fw_data, size);
 	ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
 	dma_free_coherent(mhi_cntrl->cntrl_dev, size, buf, dma_addr);
 
@@ -463,7 +474,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	}
 
 	/* Wait for ready since EDL image was loaded */
-	if (fw_name == mhi_cntrl->edl_image) {
+	if (fw_name && fw_name == mhi_cntrl->edl_image) {
 		release_firmware(firmware);
 		goto fw_load_ready_state;
 	}
@@ -478,14 +489,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	 */
 	if (mhi_cntrl->fbc_download) {
 		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
-					   firmware->size);
+					   fw_sz);
 		if (ret) {
 			release_firmware(firmware);
 			goto error_fw_load;
 		}
 
 		/* Load the firmware into BHIE vec table */
-		mhi_firmware_copy(mhi_cntrl, firmware, mhi_cntrl->fbc_image);
+		mhi_firmware_copy(mhi_cntrl, fw_data, fw_sz, mhi_cntrl->fbc_image);
 	}
 
 	release_firmware(firmware);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index a5441ad33c74..0d11fe22633e 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -299,6 +299,10 @@ struct mhi_controller_config {
  * @iova_start: IOMMU starting address for data (required)
  * @iova_stop: IOMMU stop address for data (required)
  * @fw_image: Firmware image name for normal booting (optional)
+ * @fw_data: Firmware image data content for normal booting, used only
+ *           if fw_image is NULL (optional)
+ * @fw_sz: Firmware image data size for normal booting, used only if fw_image
+ *         is NULL (optional)
  * @edl_image: Firmware image name for emergency download mode (optional)
  * @rddm_size: RAM dump size that host should allocate for debugging purpose
  * @sbl_size: SBL image size downloaded through BHIe (optional)
@@ -384,6 +388,8 @@ struct mhi_controller {
 	dma_addr_t iova_start;
 	dma_addr_t iova_stop;
 	const char *fw_image;
+	const u8 *fw_data;
+	size_t fw_sz;
 	const char *edl_image;
 	size_t rddm_size;
 	size_t sbl_size;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] ath11k: qmi: refactor ath11k_qmi_m3_load()
  2023-01-11  9:25 [PATCH 0/3] ath11k: support firmware-2.bin Kalle Valo
  2023-01-11  9:25 ` [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer Kalle Valo
@ 2023-01-11  9:25 ` Kalle Valo
  2023-01-11  9:25 ` [PATCH 3/3] ath11k: add firmware-2.bin support Kalle Valo
  2 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2023-01-11  9:25 UTC (permalink / raw)
  To: mhi; +Cc: ath11k, linux-wireless

From: Kalle Valo <quic_kvalo@quicinc.com>

Simple refactoring to make it easier to add firmware-2.bin support in the
following patch.

Earlier ath11k_qmi_m3_load() supported changing m3.bin contents while ath11k is
running. But that's not going to actually work, m3.bin is supposed to the be
same during the lifetime of ath11k, for example we don't support changing the
firmware capabilities on the fly. Due to this ath11k requests m3.bin firmware
file first and only then checks m3_mem->vaddr, so we are basically requesting
the firmware file even if it's not needed. Reverse the code so that m3_mem
buffer is checked first, and only if it doesn't exist, then m3.bin is requested
from user space.

Checking for m3_mem->size is redundant when m3_mem->vaddr is NULL, we would
not be able to use the buffer in that case. So remove the check for size.

Simplify the exit handling and use 'goto out'.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/qmi.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index ab923e24b0a9..5bc98180aed4 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2493,6 +2493,10 @@ static int ath11k_qmi_m3_load(struct ath11k_base *ab)
 	char path[100];
 	int ret;
 
+	if (m3_mem->vaddr)
+		/* m3 firmware buffer is already available in the DMA buffer */
+		return 0;
+
 	fw = ath11k_core_firmware_request(ab, ATH11K_M3_FILE);
 	if (IS_ERR(fw)) {
 		ret = PTR_ERR(fw);
@@ -2502,25 +2506,25 @@ static int ath11k_qmi_m3_load(struct ath11k_base *ab)
 		return ret;
 	}
 
-	if (m3_mem->vaddr || m3_mem->size)
-		goto skip_m3_alloc;
-
 	m3_mem->vaddr = dma_alloc_coherent(ab->dev,
 					   fw->size, &m3_mem->paddr,
 					   GFP_KERNEL);
 	if (!m3_mem->vaddr) {
 		ath11k_err(ab, "failed to allocate memory for M3 with size %zu\n",
 			   fw->size);
-		release_firmware(fw);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
-skip_m3_alloc:
 	memcpy(m3_mem->vaddr, fw->data, fw->size);
 	m3_mem->size = fw->size;
+
+	ret = 0;
+
+out:
 	release_firmware(fw);
 
-	return 0;
+	return ret;
 }
 
 static void ath11k_qmi_m3_free(struct ath11k_base *ab)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] ath11k: add firmware-2.bin support
  2023-01-11  9:25 [PATCH 0/3] ath11k: support firmware-2.bin Kalle Valo
  2023-01-11  9:25 ` [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer Kalle Valo
  2023-01-11  9:25 ` [PATCH 2/3] ath11k: qmi: refactor ath11k_qmi_m3_load() Kalle Valo
@ 2023-01-11  9:25 ` Kalle Valo
  2 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2023-01-11  9:25 UTC (permalink / raw)
  To: mhi; +Cc: ath11k, linux-wireless

From: Anilkumar Kolli <quic_akolli@quicinc.com>

Firmware IE containers can dynamically provide various information
what firmware supports. Also it can embed more than one image so
updating firmware is easy, user just needs to update one file in
/lib/firmware/.

The firmware API 2 or higher will use the IE container format, the
current API 1 will not use the new format but it still is supported
for some time. Firmware API 2 files are named as firmware-2.bin
(which contains both amss.bin and m3.bin images) and API 1 files are
amss.bin and m3.bin.

Currently ath11k PCI driver provides firmware binary (amss.bin) path to
MHI driver, MHI driver reads firmware from filesystem and boots it. Add
provision to read firmware files from ath11k driver and provide the amss.bin
firmware data and size to MHI using a pointer.

Currently enum ath11k_fw_features is empty, the patches adding features will
add the flags.

With AHB devices there's no amss.bin or m3.bin, so no changes in how AHB
firmware files are used. But AHB devices can use future additions to the meta
data, for example in enum ath11k_fw_features.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.9

Co-developed-by: P Praneesh <quic_ppranees@quicinc.com>
Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
Signed-off-by: Anilkumar Kolli <quic_akolli@quicinc.com>
Co-developed-by: Kalle Valo <quic_kvalo@quicinc.com>
Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/Makefile |   3 +-
 drivers/net/wireless/ath/ath11k/core.c   |   8 ++
 drivers/net/wireless/ath/ath11k/core.h   |  15 +++
 drivers/net/wireless/ath/ath11k/fw.c     | 157 +++++++++++++++++++++++
 drivers/net/wireless/ath/ath11k/fw.h     |  27 ++++
 drivers/net/wireless/ath/ath11k/mhi.c    |  18 ++-
 drivers/net/wireless/ath/ath11k/qmi.c    |  36 ++++--
 7 files changed, 247 insertions(+), 17 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath11k/fw.c
 create mode 100644 drivers/net/wireless/ath/ath11k/fw.h

diff --git a/drivers/net/wireless/ath/ath11k/Makefile b/drivers/net/wireless/ath/ath11k/Makefile
index cc47e0114595..2c94d50ae36f 100644
--- a/drivers/net/wireless/ath/ath11k/Makefile
+++ b/drivers/net/wireless/ath/ath11k/Makefile
@@ -17,7 +17,8 @@ ath11k-y += core.o \
 	    peer.o \
 	    dbring.o \
 	    hw.o \
-	    pcic.o
+	    pcic.o \
+	    fw.o
 
 ath11k-$(CONFIG_ATH11K_DEBUGFS) += debugfs.o debugfs_htt_stats.o debugfs_sta.o
 ath11k-$(CONFIG_NL80211_TESTMODE) += testmode.o
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 498310e38d44..61ed3adcfd70 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -16,6 +16,7 @@
 #include "debug.h"
 #include "hif.h"
 #include "wow.h"
+#include "fw.h"
 
 unsigned int ath11k_debug_mask;
 EXPORT_SYMBOL(ath11k_debug_mask);
@@ -1942,6 +1943,12 @@ int ath11k_core_pre_init(struct ath11k_base *ab)
 		return ret;
 	}
 
+	ret = ath11k_fw_pre_init(ab);
+	if (ret) {
+		ath11k_err(ab, "failed to pre init firmware: %d", ret);
+		return ret;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(ath11k_core_pre_init);
@@ -1972,6 +1979,7 @@ void ath11k_core_deinit(struct ath11k_base *ab)
 	ath11k_hif_power_down(ab);
 	ath11k_mac_destroy(ab);
 	ath11k_core_soc_destroy(ab);
+	ath11k_fw_destroy(ab);
 }
 EXPORT_SYMBOL(ath11k_core_deinit);
 
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index beb552108ac3..75cdedd268d4 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -15,6 +15,8 @@
 #include <linux/ctype.h>
 #include <linux/rhashtable.h>
 #include <linux/average.h>
+#include <linux/firmware.h>
+
 #include "qmi.h"
 #include "htc.h"
 #include "wmi.h"
@@ -29,6 +31,7 @@
 #include "dbring.h"
 #include "spectral.h"
 #include "wow.h"
+#include "fw.h"
 
 #define SM(_v, _f) (((_v) << _f##_LSB) & _f##_MASK)
 
@@ -977,6 +980,18 @@ struct ath11k_base {
 		const struct ath11k_pci_ops *ops;
 	} pci;
 
+	struct {
+		u32 api_version;
+
+		const struct firmware *fw;
+		const u8 *amss_data;
+		size_t amss_len;
+		const u8 *m3_data;
+		size_t m3_len;
+
+		DECLARE_BITMAP(fw_features, ATH11K_FW_FEATURE_COUNT);
+	} fw;
+
 	/* must be last */
 	u8 drv_priv[] __aligned(sizeof(void *));
 };
diff --git a/drivers/net/wireless/ath/ath11k/fw.c b/drivers/net/wireless/ath/ath11k/fw.c
new file mode 100644
index 000000000000..339b2b3bed84
--- /dev/null
+++ b/drivers/net/wireless/ath/ath11k/fw.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: BSD-3-Clause-Clear
+/*
+ * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include "core.h"
+
+#include "debug.h"
+
+static int ath11k_fw_request_firmware_api_n(struct ath11k_base *ab,
+					    const char *name)
+{
+	size_t magic_len, len, ie_len;
+	int ie_id, i, index, bit, ret;
+	struct ath11k_fw_ie *hdr;
+	const u8 *data;
+	__le32 *timestamp;
+
+	ab->fw.fw = ath11k_core_firmware_request(ab, name);
+	if (IS_ERR(ab->fw.fw)) {
+		ret = PTR_ERR(ab->fw.fw);
+		ath11k_err(ab, "failed to load %s: %d\n", name, ret);
+		ab->fw.fw = NULL;
+		return ret;
+	}
+
+	data = ab->fw.fw->data;
+	len = ab->fw.fw->size;
+
+	/* magic also includes the null byte, check that as well */
+	magic_len = strlen(ATH11K_FIRMWARE_MAGIC) + 1;
+
+	if (len < magic_len) {
+		ath11k_err(ab, "firmware image too small to contain magic: %zu\n",
+			   len);
+		ret = -EINVAL;
+		goto err;
+	}
+
+	if (memcmp(data, ATH11K_FIRMWARE_MAGIC, magic_len) != 0) {
+		ath11k_err(ab, "Invalid firmware magic\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/* jump over the padding */
+	magic_len = ALIGN(magic_len, 4);
+
+	len -= magic_len;
+	data += magic_len;
+
+	/* loop elements */
+	while (len > sizeof(struct ath11k_fw_ie)) {
+		hdr = (struct ath11k_fw_ie *)data;
+
+		ie_id = le32_to_cpu(hdr->id);
+		ie_len = le32_to_cpu(hdr->len);
+
+		len -= sizeof(*hdr);
+		data += sizeof(*hdr);
+
+		if (len < ie_len) {
+			ath11k_err(ab, "Invalid length for FW IE %d (%zu < %zu)\n",
+				   ie_id, len, ie_len);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		switch (ie_id) {
+		case ATH11K_FW_IE_TIMESTAMP:
+			if (ie_len != sizeof(u32))
+				break;
+
+			timestamp = (__le32 *)data;
+
+			ath11k_dbg(ab, ATH11K_DBG_BOOT, "found fw timestamp %d\n",
+				   le32_to_cpup(timestamp));
+			break;
+		case ATH11K_FW_IE_FEATURES:
+			ath11k_dbg(ab, ATH11K_DBG_BOOT,
+				   "found firmware features ie (%zd B)\n",
+				   ie_len);
+
+			for (i = 0; i < ATH11K_FW_FEATURE_COUNT; i++) {
+				index = i / 8;
+				bit = i % 8;
+
+				if (index == ie_len)
+					break;
+
+				if (data[index] & (1 << bit))
+					__set_bit(i, ab->fw.fw_features);
+			}
+
+			ath11k_dbg_dump(ab, ATH11K_DBG_BOOT, "features", "",
+					ab->fw.fw_features,
+					sizeof(ab->fw.fw_features));
+			break;
+		case ATH11K_FW_IE_AMSS_IMAGE:
+			ath11k_dbg(ab, ATH11K_DBG_BOOT,
+				   "found fw image ie (%zd B)\n",
+				   ie_len);
+
+			ab->fw.amss_data = data;
+			ab->fw.amss_len = ie_len;
+			break;
+		case ATH11K_FW_IE_M3_IMAGE:
+			ath11k_dbg(ab, ATH11K_DBG_BOOT,
+				   "found m3 image ie (%zd B)\n",
+				   ie_len);
+
+			ab->fw.m3_data = data;
+			ab->fw.m3_len = ie_len;
+			break;
+		default:
+			ath11k_warn(ab, "Unknown FW IE: %u\n", ie_id);
+			break;
+		}
+
+		/* jump over the padding */
+		ie_len = ALIGN(ie_len, 4);
+
+		len -= ie_len;
+		data += ie_len;
+	};
+
+	return 0;
+
+err:
+	release_firmware(ab->fw.fw);
+	ab->fw.fw = NULL;
+	return ret;
+}
+
+int ath11k_fw_pre_init(struct ath11k_base *ab)
+{
+	int ret;
+
+	ret = ath11k_fw_request_firmware_api_n(ab, ATH11K_FW_API2_FILE);
+	if (ret == 0) {
+		ab->fw.api_version = 2;
+		goto out;
+	}
+
+	ab->fw.api_version = 1;
+
+out:
+	ath11k_dbg(ab, ATH11K_DBG_BOOT, "using fw api %d\n",
+		   ab->fw.api_version);
+
+	return 0;
+}
+
+void ath11k_fw_destroy(struct ath11k_base *ab)
+{
+	release_firmware(ab->fw.fw);
+}
diff --git a/drivers/net/wireless/ath/ath11k/fw.h b/drivers/net/wireless/ath/ath11k/fw.h
new file mode 100644
index 000000000000..e33b0f78b571
--- /dev/null
+++ b/drivers/net/wireless/ath/ath11k/fw.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: BSD-3-Clause-Clear */
+/*
+ * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef ATH11K_FW_H
+#define ATH11K_FW_H
+
+#define ATH11K_FW_API2_FILE		"firmware-2.bin"
+#define ATH11K_FIRMWARE_MAGIC		"QCOM-ATH11K-FW"
+
+enum ath11k_fw_ie_type {
+	ATH11K_FW_IE_TIMESTAMP = 0,
+	ATH11K_FW_IE_FEATURES = 1,
+	ATH11K_FW_IE_AMSS_IMAGE = 2,
+	ATH11K_FW_IE_M3_IMAGE = 3,
+};
+
+enum ath11k_fw_features {
+	/* keep last */
+	ATH11K_FW_FEATURE_COUNT,
+};
+
+int ath11k_fw_pre_init(struct ath11k_base *ab);
+void ath11k_fw_destroy(struct ath11k_base *ab);
+
+#endif /* ATH11K_FW_H */
diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 86995e8dc913..40c43f63ebdd 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -6,6 +6,7 @@
 
 #include <linux/msi.h>
 #include <linux/pci.h>
+#include <linux/firmware.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/ioport.h>
@@ -389,16 +390,23 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
 	if (!mhi_ctrl)
 		return -ENOMEM;
 
-	ath11k_core_create_firmware_path(ab, ATH11K_AMSS_FILE,
-					 ab_pci->amss_path,
-					 sizeof(ab_pci->amss_path));
-
 	ab_pci->mhi_ctrl = mhi_ctrl;
 	mhi_ctrl->cntrl_dev = ab->dev;
-	mhi_ctrl->fw_image = ab_pci->amss_path;
 	mhi_ctrl->regs = ab->mem;
 	mhi_ctrl->reg_len = ab->mem_len;
 
+	if (ab->fw.amss_data && ab->fw.amss_len > 0) {
+		/* use MHI firmware file from firmware-N.bin */
+		mhi_ctrl->fw_data = ab->fw.amss_data;
+		mhi_ctrl->fw_sz = ab->fw.amss_len;
+	} else {
+		/* use the old separate mhi.bin MHI firmware file */
+		ath11k_core_create_firmware_path(ab, ATH11K_AMSS_FILE,
+						 ab_pci->amss_path,
+						 sizeof(ab_pci->amss_path));
+		mhi_ctrl->fw_image = ab_pci->amss_path;
+	}
+
 	ret = ath11k_mhi_get_msi(ab_pci);
 	if (ret) {
 		ath11k_err(ab, "failed to get msi for mhi\n");
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 5bc98180aed4..426d047ff1d5 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -2489,25 +2489,39 @@ static int ath11k_qmi_load_bdf_qmi(struct ath11k_base *ab,
 static int ath11k_qmi_m3_load(struct ath11k_base *ab)
 {
 	struct m3_mem_region *m3_mem = &ab->qmi.m3_mem;
-	const struct firmware *fw;
+	const struct firmware *fw = NULL;
+	const void *m3_data;
 	char path[100];
+	size_t m3_len;
 	int ret;
 
 	if (m3_mem->vaddr)
 		/* m3 firmware buffer is already available in the DMA buffer */
 		return 0;
 
-	fw = ath11k_core_firmware_request(ab, ATH11K_M3_FILE);
-	if (IS_ERR(fw)) {
-		ret = PTR_ERR(fw);
-		ath11k_core_create_firmware_path(ab, ATH11K_M3_FILE,
-						 path, sizeof(path));
-		ath11k_err(ab, "failed to load %s: %d\n", path, ret);
-		return ret;
+	if (ab->fw.m3_data && ab->fw.m3_len > 0) {
+		/* firmware-N.bin had a m3 firmware file so use that */
+		m3_data = ab->fw.m3_data;
+		m3_len = ab->fw.m3_len;
+	} else {
+		/* No m3 file in firmware-N.bin so try to request old
+		 * separate m3.bin.
+		 */
+		fw = ath11k_core_firmware_request(ab, ATH11K_M3_FILE);
+		if (IS_ERR(fw)) {
+			ret = PTR_ERR(fw);
+			ath11k_core_create_firmware_path(ab, ATH11K_M3_FILE,
+							 path, sizeof(path));
+			ath11k_err(ab, "failed to load %s: %d\n", path, ret);
+			return ret;
+		}
+
+		m3_data = fw->data;
+		m3_len = fw->size;
 	}
 
 	m3_mem->vaddr = dma_alloc_coherent(ab->dev,
-					   fw->size, &m3_mem->paddr,
+					   m3_len, &m3_mem->paddr,
 					   GFP_KERNEL);
 	if (!m3_mem->vaddr) {
 		ath11k_err(ab, "failed to allocate memory for M3 with size %zu\n",
@@ -2516,8 +2530,8 @@ static int ath11k_qmi_m3_load(struct ath11k_base *ab)
 		goto out;
 	}
 
-	memcpy(m3_mem->vaddr, fw->data, fw->size);
-	m3_mem->size = fw->size;
+	memcpy(m3_mem->vaddr, m3_data, m3_len);
+	m3_mem->size = m3_len;
 
 	ret = 0;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer
  2023-01-11  9:25 ` [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer Kalle Valo
@ 2023-01-11 15:20   ` Jeffrey Hugo
  2023-01-12  9:19     ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Hugo @ 2023-01-11 15:20 UTC (permalink / raw)
  To: Kalle Valo, mhi; +Cc: ath11k, linux-wireless

On 1/11/2023 2:25 AM, Kalle Valo wrote:
> From: Kalle Valo <quic_kvalo@quicinc.com>
> 
> Currently MHI loads the firmware image from the path provided by client
> devices. ath11k needs to support firmware image embedded along with meta data
> (named as firmware-2.bin). So allow the client driver to request the firmware
> file from user space on it's own and provide the firmware image data and size
> to MHI via a pointer struct mhi_controller::fw_data.
> 
> This is an optional feature, if fw_data is NULL MHI load the firmware using the
> name from struct mhi_controller::fw_image string as before.
> 
> Tested with ath11k and WCN6855 hw2.0.
> 
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> ---
>   drivers/bus/mhi/host/boot.c | 27 +++++++++++++++++++--------
>   include/linux/mhi.h         |  6 ++++++
>   2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index 1c69feee1703..d6d5f424491e 100644
> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -365,12 +365,10 @@ int mhi_alloc_bhie_table(struct mhi_controller *mhi_cntrl,
>   }
>   
>   static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
> -			      const struct firmware *firmware,
> +			      const u8 *buf, size_t remainder,
>   			      struct image_info *img_info)
>   {
> -	size_t remainder = firmware->size;
>   	size_t to_cpy;
> -	const u8 *buf = firmware->data;
>   	struct mhi_buf *mhi_buf = img_info->mhi_buf;
>   	struct bhi_vec_entry *bhi_vec = img_info->bhi_vec;
>   
> @@ -392,9 +390,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	const struct firmware *firmware = NULL;
>   	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>   	const char *fw_name;
> +	const u8 *fw_data;
>   	void *buf;
>   	dma_addr_t dma_addr;
> -	size_t size;
> +	size_t size, fw_sz;
>   	int i, ret;
>   
>   	if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
> @@ -424,6 +423,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	fw_name = (mhi_cntrl->ee == MHI_EE_EDL) ?
>   		mhi_cntrl->edl_image : mhi_cntrl->fw_image;
>   
> +	if (!fw_name && mhi_cntrl->fbc_download &&
> +	    mhi_cntrl->fw_data && mhi_cntrl->fw_sz) {
> +		size = mhi_cntrl->sbl_size;
> +		fw_data = mhi_cntrl->fw_data;
> +		fw_sz = mhi_cntrl->fw_sz;
> +		goto skip_req_fw;
> +	}
> +
>   	if (!fw_name || (mhi_cntrl->fbc_download && (!mhi_cntrl->sbl_size ||
>   						     !mhi_cntrl->seg_len))) {
>   		dev_err(dev,
> @@ -443,6 +450,10 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	if (size > firmware->size)
>   		size = firmware->size;
>   
> +	fw_data = firmware->data;
> +	fw_sz = firmware->size;
> +
> +skip_req_fw:
>   	buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, size, &dma_addr,
>   				 GFP_KERNEL);
>   	if (!buf) {
> @@ -451,7 +462,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	}
>   
>   	/* Download image using BHI */
> -	memcpy(buf, firmware->data, size);
> +	memcpy(buf, fw_data, size);
>   	ret = mhi_fw_load_bhi(mhi_cntrl, dma_addr, size);
>   	dma_free_coherent(mhi_cntrl->cntrl_dev, size, buf, dma_addr);
>   
> @@ -463,7 +474,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	}
>   
>   	/* Wait for ready since EDL image was loaded */
> -	if (fw_name == mhi_cntrl->edl_image) {
> +	if (fw_name && fw_name == mhi_cntrl->edl_image) {
>   		release_firmware(firmware);
>   		goto fw_load_ready_state;
>   	}
> @@ -478,14 +489,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>   	 */
>   	if (mhi_cntrl->fbc_download) {
>   		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
> -					   firmware->size);
> +					   fw_sz);

Minor nit, but it seems like this could be all on one line.

>   		if (ret) {
>   			release_firmware(firmware);
>   			goto error_fw_load;
>   		}
>   
>   		/* Load the firmware into BHIE vec table */
> -		mhi_firmware_copy(mhi_cntrl, firmware, mhi_cntrl->fbc_image);
> +		mhi_firmware_copy(mhi_cntrl, fw_data, fw_sz, mhi_cntrl->fbc_image);
>   	}
>   
>   	release_firmware(firmware);
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index a5441ad33c74..0d11fe22633e 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -299,6 +299,10 @@ struct mhi_controller_config {
>    * @iova_start: IOMMU starting address for data (required)
>    * @iova_stop: IOMMU stop address for data (required)
>    * @fw_image: Firmware image name for normal booting (optional)
> + * @fw_data: Firmware image data content for normal booting, used only
> + *           if fw_image is NULL (optional)

The implementation requires fbc_download to be set, which is not a 
requirement for fw_image.  That is not apparent here.

> + * @fw_sz: Firmware image data size for normal booting, used only if fw_image
> + *         is NULL (optional)
>    * @edl_image: Firmware image name for emergency download mode (optional)
>    * @rddm_size: RAM dump size that host should allocate for debugging purpose
>    * @sbl_size: SBL image size downloaded through BHIe (optional)
> @@ -384,6 +388,8 @@ struct mhi_controller {
>   	dma_addr_t iova_start;
>   	dma_addr_t iova_stop;
>   	const char *fw_image;
> +	const u8 *fw_data;
> +	size_t fw_sz;

Did you run pahole?  I remember this struct being well packed, and I 
think this will add a compiler hole but I have not actually verified.

>   	const char *edl_image;
>   	size_t rddm_size;
>   	size_t sbl_size;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer
  2023-01-11 15:20   ` Jeffrey Hugo
@ 2023-01-12  9:19     ` Kalle Valo
  2023-03-08 13:20       ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2023-01-12  9:19 UTC (permalink / raw)
  To: Jeffrey Hugo; +Cc: mhi, ath11k, linux-wireless

Jeffrey Hugo <quic_jhugo@quicinc.com> writes:

> On 1/11/2023 2:25 AM, Kalle Valo wrote:
>> From: Kalle Valo <quic_kvalo@quicinc.com>
>>
>> Currently MHI loads the firmware image from the path provided by client
>> devices. ath11k needs to support firmware image embedded along with meta data
>> (named as firmware-2.bin). So allow the client driver to request the firmware
>> file from user space on it's own and provide the firmware image data and size
>> to MHI via a pointer struct mhi_controller::fw_data.
>>
>> This is an optional feature, if fw_data is NULL MHI load the firmware using the
>> name from struct mhi_controller::fw_image string as before.
>>
>> Tested with ath11k and WCN6855 hw2.0.
>>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

[...]

>> @@ -478,14 +489,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
>>   	 */
>>   	if (mhi_cntrl->fbc_download) {
>>   		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
>> -					   firmware->size);
>> +					   fw_sz);
>
> Minor nit, but it seems like this could be all on one line.

Will fix in v2.

>> --- a/include/linux/mhi.h
>> +++ b/include/linux/mhi.h
>> @@ -299,6 +299,10 @@ struct mhi_controller_config {
>>    * @iova_start: IOMMU starting address for data (required)
>>    * @iova_stop: IOMMU stop address for data (required)
>>    * @fw_image: Firmware image name for normal booting (optional)
>> + * @fw_data: Firmware image data content for normal booting, used only
>> + *           if fw_image is NULL (optional)
>
> The implementation requires fbc_download to be set, which is not a
> requirement for fw_image.  That is not apparent here.

Ah, I had missed that. Will mention that in v2.

>> @@ -384,6 +388,8 @@ struct mhi_controller {
>>   	dma_addr_t iova_start;
>>   	dma_addr_t iova_stop;
>>   	const char *fw_image;
>> +	const u8 *fw_data;
>> +	size_t fw_sz;
>
> Did you run pahole?  I remember this struct being well packed, and I
> think this will add a compiler hole but I have not actually verified.

I actually haven't used pahole for ages. I will run it and check how
this structure is packed.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer
  2023-01-12  9:19     ` Kalle Valo
@ 2023-03-08 13:20       ` Kalle Valo
  2023-03-13 14:04         ` Jeffrey Hugo
  0 siblings, 1 reply; 8+ messages in thread
From: Kalle Valo @ 2023-03-08 13:20 UTC (permalink / raw)
  To: Jeffrey Hugo; +Cc: mhi, ath11k, linux-wireless

Kalle Valo <kvalo@kernel.org> writes:

> Jeffrey Hugo <quic_jhugo@quicinc.com> writes:
>
>> On 1/11/2023 2:25 AM, Kalle Valo wrote:
>>> From: Kalle Valo <quic_kvalo@quicinc.com>
>>>
>>> Currently MHI loads the firmware image from the path provided by client
>>> devices. ath11k needs to support firmware image embedded along with meta data
>>> (named as firmware-2.bin). So allow the client driver to request the firmware
>>> file from user space on it's own and provide the firmware image data and size
>>> to MHI via a pointer struct mhi_controller::fw_data.
>>>
>>> This is an optional feature, if fw_data is NULL MHI load the firmware using the
>>> name from struct mhi_controller::fw_image string as before.
>>>
>>> Tested with ath11k and WCN6855 hw2.0.
>>>
>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

[...]

>> Did you run pahole?  I remember this struct being well packed, and I
>> think this will add a compiler hole but I have not actually verified.
>
> I actually haven't used pahole for ages. I will run it and check how
> this structure is packed.

Below is what pahole shows with GCC 8.3 on x86_64. Look pretty well
packed, right?

struct mhi_controller {
	struct device *            cntrl_dev;            /*     0     8 */
	struct mhi_device *        mhi_dev;              /*     8     8 */
	struct dentry *            debugfs_dentry;       /*    16     8 */
	void *                     regs;                 /*    24     8 */
	void *                     bhi;                  /*    32     8 */
	void *                     bhie;                 /*    40     8 */
	void *                     wake_db;              /*    48     8 */
	dma_addr_t                 iova_start;           /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	dma_addr_t                 iova_stop;            /*    64     8 */
	const char  *              fw_image;             /*    72     8 */
	const u8  *                fw_data;              /*    80     8 */
	size_t                     fw_sz;                /*    88     8 */
	const char  *              edl_image;            /*    96     8 */
	size_t                     rddm_size;            /*   104     8 */
	size_t                     sbl_size;             /*   112     8 */
	size_t                     seg_len;              /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	size_t                     reg_len;              /*   128     8 */
	struct image_info *        fbc_image;            /*   136     8 */
	struct image_info *        rddm_image;           /*   144     8 */
	struct mhi_chan *          mhi_chan;             /*   152     8 */
	struct list_head           lpm_chans;            /*   160    16 */
	int *                      irq;                  /*   176     8 */
	u32                        max_chan;             /*   184     4 */
	u32                        total_ev_rings;       /*   188     4 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	u32                        hw_ev_rings;          /*   192     4 */
	u32                        sw_ev_rings;          /*   196     4 */
	u32                        nr_irqs;              /*   200     4 */
	u32                        family_number;        /*   204     4 */
	u32                        device_number;        /*   208     4 */
	u32                        major_version;        /*   212     4 */
	u32                        minor_version;        /*   216     4 */
	u32                        serial_number;        /*   220     4 */
	u32                        oem_pk_hash[16];      /*   224    64 */
	/* --- cacheline 4 boundary (256 bytes) was 32 bytes ago --- */
	struct mhi_event *         mhi_event;            /*   288     8 */
	struct mhi_cmd *           mhi_cmd;              /*   296     8 */
	struct mhi_ctxt *          mhi_ctxt;             /*   304     8 */
	struct mutex               pm_mutex;             /*   312   160 */
	/* --- cacheline 7 boundary (448 bytes) was 24 bytes ago --- */
	rwlock_t                   pm_lock;              /*   472    72 */
	/* --- cacheline 8 boundary (512 bytes) was 32 bytes ago --- */
	u32                        timeout_ms;           /*   544     4 */
	u32                        pm_state;             /*   548     4 */
	u32                        db_access;            /*   552     4 */
	enum mhi_ee_type           ee;                   /*   556     4 */
	enum mhi_state             dev_state;            /*   560     4 */
	atomic_t                   dev_wake;             /*   564     4 */
	atomic_t                   pending_pkts;         /*   568     4 */
	u32                        M0;                   /*   572     4 */
	/* --- cacheline 9 boundary (576 bytes) --- */
	u32                        M2;                   /*   576     4 */
	u32                        M3;                   /*   580     4 */
	struct list_head           transition_list;      /*   584    16 */
	spinlock_t                 transition_lock;      /*   600    72 */
	/* --- cacheline 10 boundary (640 bytes) was 32 bytes ago --- */
	spinlock_t                 wlock;                /*   672    72 */
	/* --- cacheline 11 boundary (704 bytes) was 40 bytes ago --- */
	struct mhi_link_info       mhi_link_info;        /*   744     8 */
	struct work_struct         st_worker;            /*   752    80 */
	/* --- cacheline 13 boundary (832 bytes) --- */
	struct workqueue_struct *  hiprio_wq;            /*   832     8 */
	wait_queue_head_t          state_event;          /*   840    88 */
	/* --- cacheline 14 boundary (896 bytes) was 32 bytes ago --- */
	void                       (*status_cb)(struct mhi_controller *, enum mhi_callback); /*   928     8 */
	void                       (*wake_get)(struct mhi_controller *, bool); /*   936     8 */
	void                       (*wake_put)(struct mhi_controller *, bool); /*   944     8 */
	void                       (*wake_toggle)(struct mhi_controller *); /*   952     8 */
	/* --- cacheline 15 boundary (960 bytes) --- */
	int                        (*runtime_get)(struct mhi_controller *); /*   960     8 */
	void                       (*runtime_put)(struct mhi_controller *); /*   968     8 */
	int                        (*map_single)(struct mhi_controller *, struct mhi_buf_info *); /*   976     8 */
	void                       (*unmap_single)(struct mhi_controller *, struct mhi_buf_info *); /*   984     8 */
	int                        (*read_reg)(struct mhi_controller *, void *, u32 *); /*   992     8 */
	void                       (*write_reg)(struct mhi_controller *, void *, u32); /*  1000     8 */
	void                       (*reset)(struct mhi_controller *); /*  1008     8 */
	size_t                     buffer_len;           /*  1016     8 */
	/* --- cacheline 16 boundary (1024 bytes) --- */
	int                        index;                /*  1024     4 */
	bool                       bounce_buf;           /*  1028     1 */
	bool                       fbc_download;         /*  1029     1 */
	bool                       wake_set;             /*  1030     1 */

	/* XXX 1 byte hole, try to pack */

	long unsigned int          irq_flags;            /*  1032     8 */
	u32                        mru;                  /*  1040     4 */

	/* size: 1048, cachelines: 17, members: 73 */
	/* sum members: 1043, holes: 1, sum holes: 1 */
	/* padding: 4 */
	/* last cacheline: 24 bytes */
};

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer
  2023-03-08 13:20       ` Kalle Valo
@ 2023-03-13 14:04         ` Jeffrey Hugo
  0 siblings, 0 replies; 8+ messages in thread
From: Jeffrey Hugo @ 2023-03-13 14:04 UTC (permalink / raw)
  To: Kalle Valo; +Cc: mhi, ath11k, linux-wireless

On 3/8/2023 6:20 AM, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
> 
>> Jeffrey Hugo <quic_jhugo@quicinc.com> writes:
>>
>>> On 1/11/2023 2:25 AM, Kalle Valo wrote:
>>>> From: Kalle Valo <quic_kvalo@quicinc.com>
>>>>
>>>> Currently MHI loads the firmware image from the path provided by client
>>>> devices. ath11k needs to support firmware image embedded along with meta data
>>>> (named as firmware-2.bin). So allow the client driver to request the firmware
>>>> file from user space on it's own and provide the firmware image data and size
>>>> to MHI via a pointer struct mhi_controller::fw_data.
>>>>
>>>> This is an optional feature, if fw_data is NULL MHI load the firmware using the
>>>> name from struct mhi_controller::fw_image string as before.
>>>>
>>>> Tested with ath11k and WCN6855 hw2.0.
>>>>
>>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> 
> [...]
> 
>>> Did you run pahole?  I remember this struct being well packed, and I
>>> think this will add a compiler hole but I have not actually verified.
>>
>> I actually haven't used pahole for ages. I will run it and check how
>> this structure is packed.
> 
> Below is what pahole shows with GCC 8.3 on x86_64. Look pretty well
> packed, right?

Yes, looks almost perfect.

> 
> struct mhi_controller {
> 	struct device *            cntrl_dev;            /*     0     8 */
> 	struct mhi_device *        mhi_dev;              /*     8     8 */
> 	struct dentry *            debugfs_dentry;       /*    16     8 */
> 	void *                     regs;                 /*    24     8 */
> 	void *                     bhi;                  /*    32     8 */
> 	void *                     bhie;                 /*    40     8 */
> 	void *                     wake_db;              /*    48     8 */
> 	dma_addr_t                 iova_start;           /*    56     8 */
> 	/* --- cacheline 1 boundary (64 bytes) --- */
> 	dma_addr_t                 iova_stop;            /*    64     8 */
> 	const char  *              fw_image;             /*    72     8 */
> 	const u8  *                fw_data;              /*    80     8 */
> 	size_t                     fw_sz;                /*    88     8 */
> 	const char  *              edl_image;            /*    96     8 */
> 	size_t                     rddm_size;            /*   104     8 */
> 	size_t                     sbl_size;             /*   112     8 */
> 	size_t                     seg_len;              /*   120     8 */
> 	/* --- cacheline 2 boundary (128 bytes) --- */
> 	size_t                     reg_len;              /*   128     8 */
> 	struct image_info *        fbc_image;            /*   136     8 */
> 	struct image_info *        rddm_image;           /*   144     8 */
> 	struct mhi_chan *          mhi_chan;             /*   152     8 */
> 	struct list_head           lpm_chans;            /*   160    16 */
> 	int *                      irq;                  /*   176     8 */
> 	u32                        max_chan;             /*   184     4 */
> 	u32                        total_ev_rings;       /*   188     4 */
> 	/* --- cacheline 3 boundary (192 bytes) --- */
> 	u32                        hw_ev_rings;          /*   192     4 */
> 	u32                        sw_ev_rings;          /*   196     4 */
> 	u32                        nr_irqs;              /*   200     4 */
> 	u32                        family_number;        /*   204     4 */
> 	u32                        device_number;        /*   208     4 */
> 	u32                        major_version;        /*   212     4 */
> 	u32                        minor_version;        /*   216     4 */
> 	u32                        serial_number;        /*   220     4 */
> 	u32                        oem_pk_hash[16];      /*   224    64 */
> 	/* --- cacheline 4 boundary (256 bytes) was 32 bytes ago --- */
> 	struct mhi_event *         mhi_event;            /*   288     8 */
> 	struct mhi_cmd *           mhi_cmd;              /*   296     8 */
> 	struct mhi_ctxt *          mhi_ctxt;             /*   304     8 */
> 	struct mutex               pm_mutex;             /*   312   160 */
> 	/* --- cacheline 7 boundary (448 bytes) was 24 bytes ago --- */
> 	rwlock_t                   pm_lock;              /*   472    72 */
> 	/* --- cacheline 8 boundary (512 bytes) was 32 bytes ago --- */
> 	u32                        timeout_ms;           /*   544     4 */
> 	u32                        pm_state;             /*   548     4 */
> 	u32                        db_access;            /*   552     4 */
> 	enum mhi_ee_type           ee;                   /*   556     4 */
> 	enum mhi_state             dev_state;            /*   560     4 */
> 	atomic_t                   dev_wake;             /*   564     4 */
> 	atomic_t                   pending_pkts;         /*   568     4 */
> 	u32                        M0;                   /*   572     4 */
> 	/* --- cacheline 9 boundary (576 bytes) --- */
> 	u32                        M2;                   /*   576     4 */
> 	u32                        M3;                   /*   580     4 */
> 	struct list_head           transition_list;      /*   584    16 */
> 	spinlock_t                 transition_lock;      /*   600    72 */
> 	/* --- cacheline 10 boundary (640 bytes) was 32 bytes ago --- */
> 	spinlock_t                 wlock;                /*   672    72 */
> 	/* --- cacheline 11 boundary (704 bytes) was 40 bytes ago --- */
> 	struct mhi_link_info       mhi_link_info;        /*   744     8 */
> 	struct work_struct         st_worker;            /*   752    80 */
> 	/* --- cacheline 13 boundary (832 bytes) --- */
> 	struct workqueue_struct *  hiprio_wq;            /*   832     8 */
> 	wait_queue_head_t          state_event;          /*   840    88 */
> 	/* --- cacheline 14 boundary (896 bytes) was 32 bytes ago --- */
> 	void                       (*status_cb)(struct mhi_controller *, enum mhi_callback); /*   928     8 */
> 	void                       (*wake_get)(struct mhi_controller *, bool); /*   936     8 */
> 	void                       (*wake_put)(struct mhi_controller *, bool); /*   944     8 */
> 	void                       (*wake_toggle)(struct mhi_controller *); /*   952     8 */
> 	/* --- cacheline 15 boundary (960 bytes) --- */
> 	int                        (*runtime_get)(struct mhi_controller *); /*   960     8 */
> 	void                       (*runtime_put)(struct mhi_controller *); /*   968     8 */
> 	int                        (*map_single)(struct mhi_controller *, struct mhi_buf_info *); /*   976     8 */
> 	void                       (*unmap_single)(struct mhi_controller *, struct mhi_buf_info *); /*   984     8 */
> 	int                        (*read_reg)(struct mhi_controller *, void *, u32 *); /*   992     8 */
> 	void                       (*write_reg)(struct mhi_controller *, void *, u32); /*  1000     8 */
> 	void                       (*reset)(struct mhi_controller *); /*  1008     8 */
> 	size_t                     buffer_len;           /*  1016     8 */
> 	/* --- cacheline 16 boundary (1024 bytes) --- */
> 	int                        index;                /*  1024     4 */
> 	bool                       bounce_buf;           /*  1028     1 */
> 	bool                       fbc_download;         /*  1029     1 */
> 	bool                       wake_set;             /*  1030     1 */
> 
> 	/* XXX 1 byte hole, try to pack */
> 
> 	long unsigned int          irq_flags;            /*  1032     8 */
> 	u32                        mru;                  /*  1040     4 */
> 
> 	/* size: 1048, cachelines: 17, members: 73 */
> 	/* sum members: 1043, holes: 1, sum holes: 1 */
> 	/* padding: 4 */
> 	/* last cacheline: 24 bytes */
> };
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-03-13 14:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11  9:25 [PATCH 0/3] ath11k: support firmware-2.bin Kalle Valo
2023-01-11  9:25 ` [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer Kalle Valo
2023-01-11 15:20   ` Jeffrey Hugo
2023-01-12  9:19     ` Kalle Valo
2023-03-08 13:20       ` Kalle Valo
2023-03-13 14:04         ` Jeffrey Hugo
2023-01-11  9:25 ` [PATCH 2/3] ath11k: qmi: refactor ath11k_qmi_m3_load() Kalle Valo
2023-01-11  9:25 ` [PATCH 3/3] ath11k: add firmware-2.bin support Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).