wifi: ath11k: support firmware-2.bin

From: Kalle Valo <[email protected]>

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 <[email protected]>

---
 drivers/bus/mhi/host/boot.c | 28 +++++++++++++++++++---------
 include/linux/mhi.h         |  6 ++++++
 2 files changed, 25 insertions(+), 9 deletions(-)

drivers/bus/mhi/host/boot.c +3 -4

index 1c69feee1703..5e6e1e340057 100644
@@ -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)) {

drivers/bus/mhi/host/boot.c +13 -5

+		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;
 	}

@@ -477,15 +488,14 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
 	 * device transitioning into MHI READY state
 	 */
 	if (mhi_cntrl->fbc_download) {
-		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image,
-					   firmware->size);
+		ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image, 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);

include/linux/mhi.h +4 -0

index a5441ad33c74..72eef7309736 100644
@@ -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 and fbc_download is true (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)

include/linux/mhi.h +0 -0

 	const char *edl_image;
 	size_t rddm_size;
 	size_t sbl_size;

Signature · 1 line

2.30.2

Quoted 2 lines · Kalle Valo

From: Kalle Valo <[email protected]>

Subject prefix should be: "bus: mhi: host: ..."

Quoted 11 lines

drivers/bus/mhi/host/boot.c +2 -1

@@ -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;
 

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 425

Quoted 5 lines

drivers/bus/mhi/host/boot.c +0 -0

 	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;
 

Can you please add a comment here?

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 428

Quoted 4 lines

drivers/bus/mhi/host/boot.c +3 -0

+	if (!fw_name && mhi_cntrl->fbc_download &&
+	    mhi_cntrl->fw_data && mhi_cntrl->fw_sz) {
+		size = mhi_cntrl->sbl_size;

Don't you need to validate sbl_size?

Quoted 14 lines

include/linux/mhi.h +4 -0

@@ -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 and fbc_download is true (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)

include/linux/mhi.h struct mhi_controller { · line 392

Quoted 5 lines

include/linux/mhi.h +2 -0

@@ -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;

Even though these members are not creating holes now, shuffling the datatypes will create holes in the future. So I always prefer to keep the struct members sorted in the below order:

pointer
struct/union
u64
u32
u16
u8
bool

Quoted 5 lines · Manivannan Sadhasivam

Quoted 2 lines · Kalle Valo

From: Kalle Valo <[email protected]>

Subject prefix should be: "bus: mhi: host: ..."

Will do.

Quoted 11 lines

Quoted 11 lines

drivers/bus/mhi/host/boot.c +2 -1

@@ -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;
 

Quoted 7 lines

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 425

Quoted 5 lines

drivers/bus/mhi/host/boot.c +0 -0

 	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;
 

Can you please add a comment here?

Ok.

Quoted 6 lines

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 428

Quoted 4 lines

drivers/bus/mhi/host/boot.c +3 -0

+	if (!fw_name && mhi_cntrl->fbc_download &&
+	    mhi_cntrl->fw_data && mhi_cntrl->fw_sz) {
+		size = mhi_cntrl->sbl_size;

Don't you need to validate sbl_size?

Good point, I'll add that.

Quoted 24 lines

Quoted 14 lines

include/linux/mhi.h +4 -0

@@ -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 and fbc_download is true (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)

include/linux/mhi.h struct mhi_controller { · line 392

Quoted 5 lines

include/linux/mhi.h +2 -0

@@ -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;

Even though these members are not creating holes now, shuffling the datatypes will create holes in the future. So I always prefer to keep the struct members sorted in the below order:

Quoted 7 lines

pointer
struct/union
u64
u32
u16
u8
bool

I'm not sure what are suggesting here as struct mhi_controller is not using that style. Are you saying that fw_sz should be after reg_len? So something like this:

	const u8 *fw_data;
	const char *edl_image;
	size_t rddm_size;
	size_t sbl_size;
	size_t seg_len;
	size_t reg_len;
	size_t fw_sz;

Signature · 3 lines

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

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

Quoted 76 lines · Kalle Valo

Quoted 5 lines · Manivannan Sadhasivam

Quoted 2 lines · Kalle Valo

From: Kalle Valo <[email protected]>

Subject prefix should be: "bus: mhi: host: ..."

Will do.

Quoted 11 lines

Quoted 11 lines

drivers/bus/mhi/host/boot.c +2 -1

@@ -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;
 

Quoted 7 lines

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 425

Quoted 5 lines

drivers/bus/mhi/host/boot.c +0 -0

 	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;
 

Can you please add a comment here?

Ok.

Quoted 6 lines

drivers/bus/mhi/host/boot.c void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) · line 428

Quoted 4 lines

drivers/bus/mhi/host/boot.c +3 -0

+	if (!fw_name && mhi_cntrl->fbc_download &&
+	    mhi_cntrl->fw_data && mhi_cntrl->fw_sz) {
+		size = mhi_cntrl->sbl_size;

Don't you need to validate sbl_size?

Good point, I'll add that.

Quoted 24 lines

Quoted 14 lines

include/linux/mhi.h +4 -0

@@ -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 and fbc_download is true (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)

include/linux/mhi.h struct mhi_controller { · line 392

Quoted 5 lines

include/linux/mhi.h +2 -0

@@ -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;

Even though these members are not creating holes now, shuffling the datatypes will create holes in the future. So I always prefer to keep the struct members sorted in the below order:

Quoted 7 lines

pointer
struct/union
u64
u32
u16
u8
bool

I'm not sure what are suggesting here as struct mhi_controller is not using that style. Are you saying that fw_sz should be after reg_len? So something like this:

Quoted 8 lines

	const u8 *fw_data;
	const char *edl_image;
	size_t rddm_size;
	size_t sbl_size;
	size_t seg_len;
	size_t reg_len;
	size_t fw_sz;

Ah, I thought I already sorted them up but apparently not. Keep it as it is, I'll sort this and other structs later.