Search Linux Wireless

Re: [PATCH 1/3] mhi: allow MHI client drivers to provide the firmware via a pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/11/2023 2:25 AM, Kalle Valo wrote:
From: Kalle Valo <quic_kvalo@xxxxxxxxxxx>

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




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux