On Wed, Mar 08, 2023 at 05:25:20PM +0200, Kalle Valo wrote:
> From: Kalle Valo <quic_kvalo@xxxxxxxxxxx>
>
Subject prefix should be: "bus: mhi: host: ..."
> 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 | 28 +++++++++++++++++++---------
> include/linux/mhi.h | 6 ++++++
> 2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index 1c69feee1703..5e6e1e340057 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;
>
Can you please add a comment here?
> + 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?
> + 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);
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index a5441ad33c74..72eef7309736 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 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)
> @@ -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
Thanks,
Mani
> const char *edl_image;
> size_t rddm_size;
> size_t sbl_size;
> --
> 2.30.2
>
>
--
மணிவண்ணன் சதாசிவம்
