2023-08-07 08:41:43

by Qiang Yu

[permalink] [raw]
Subject: [PATCH v2] mhi: host: Add standard ELF header image download functionality

From: Mayank Rana <[email protected]>

Some devices (e.g. WLAN chips) are unable to handle the non-standard ELF
format of the FBC image and thus need special handling of the FBC image.

Add standard_elf_image flag which makes decision in terms of how FBC image
based AMSS image is being downloaded with connected endpoint.
FBC image is having two image combine: SBL image + AMSS image.
1. FBC image download using legacy single ELF header image format:
- SBL image: 512KB of FBC image is downloaded using BHI.
- AMSS image: full FBC image is downloaded using BHIe.
2. FBC image download using separate ELF header image format:
- SBL image: 512 KB of FBC image is downloaded using BHI.
- AMSS image: 512 KB onward FBC image is downloaded using BHIe.
There is no change for SBL image download. Although AMSS image start
address is end address of SBL image while using separate ELF header format.

Signed-off-by: Mayank Rana <[email protected]>
[[email protected]: Update commit message, minor updates]
Signed-off-by: Qiang Yu <[email protected]>
---
v1->v2: modify commit message
correct author
rebase on latest mhi-next branch, resolve conflicts

drivers/bus/mhi/host/boot.c | 7 +++++++
include/linux/mhi.h | 2 ++
2 files changed, 9 insertions(+)

diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
index edc0ec5..586d551 100644
--- a/drivers/bus/mhi/host/boot.c
+++ b/drivers/bus/mhi/host/boot.c
@@ -495,6 +495,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
* device transitioning into MHI READY state
*/
if (mhi_cntrl->fbc_download) {
+ dev_dbg(dev, "standard_elf_image: %s\n",
+ (mhi_cntrl->standard_elf_image ? "True" : "False"));
+ if (mhi_cntrl->standard_elf_image) {
+ fw_data = firmware->data + mhi_cntrl->sbl_size;
+ fw_sz = fw_sz - mhi_cntrl->sbl_size;
+ }
+
ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image, fw_sz);
if (ret) {
release_firmware(firmware);
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 039943e..e065101 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -310,6 +310,7 @@ struct mhi_controller_config {
* @reg_len: Length of the MHI MMIO region (required)
* @fbc_image: Points to firmware image buffer
* @rddm_image: Points to RAM dump buffer
+ * @standard_elf_image: Flag to make decision about firmware download start address (optional)
* @mhi_chan: Points to the channel configuration table
* @lpm_chans: List of channels that require LPM notifications
* @irq: base irq # to request (required)
@@ -456,6 +457,7 @@ struct mhi_controller {
bool bounce_buf;
bool fbc_download;
bool wake_set;
+ bool standard_elf_image;
unsigned long irq_flags;
u32 mru;
};
--
2.7.4



2023-08-08 17:58:00

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2] mhi: host: Add standard ELF header image download functionality

On Mon, Aug 07, 2023 at 03:59:52PM +0800, Qiang Yu wrote:
> From: Mayank Rana <[email protected]>
>

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

> Some devices (e.g. WLAN chips) are unable to handle the non-standard ELF
> format of the FBC image and thus need special handling of the FBC image.
>

Which WLAN chip? Is the driver for the chip already upstreamed?

> Add standard_elf_image flag which makes decision in terms of how FBC image
> based AMSS image is being downloaded with connected endpoint.
> FBC image is having two image combine: SBL image + AMSS image.
> 1. FBC image download using legacy single ELF header image format:
> - SBL image: 512KB of FBC image is downloaded using BHI.
> - AMSS image: full FBC image is downloaded using BHIe.
> 2. FBC image download using separate ELF header image format:
> - SBL image: 512 KB of FBC image is downloaded using BHI.
> - AMSS image: 512 KB onward FBC image is downloaded using BHIe.
> There is no change for SBL image download. Although AMSS image start
> address is end address of SBL image while using separate ELF header format.
>
> Signed-off-by: Mayank Rana <[email protected]>
> [[email protected]: Update commit message, minor updates]
> Signed-off-by: Qiang Yu <[email protected]>
> ---
> v1->v2: modify commit message
> correct author
> rebase on latest mhi-next branch, resolve conflicts
>
> drivers/bus/mhi/host/boot.c | 7 +++++++
> include/linux/mhi.h | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c
> index edc0ec5..586d551 100644
> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -495,6 +495,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
> * device transitioning into MHI READY state
> */
> if (mhi_cntrl->fbc_download) {
> + dev_dbg(dev, "standard_elf_image: %s\n",
> + (mhi_cntrl->standard_elf_image ? "True" : "False"));
> + if (mhi_cntrl->standard_elf_image) {

Commit message is saying that the devices cannot handle non-standard ELF image.
But this check is for standard ELF image. I'm confused.

> + fw_data = firmware->data + mhi_cntrl->sbl_size;
> + fw_sz = fw_sz - mhi_cntrl->sbl_size;
> + }
> +
> ret = mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->fbc_image, fw_sz);
> if (ret) {
> release_firmware(firmware);
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 039943e..e065101 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -310,6 +310,7 @@ struct mhi_controller_config {
> * @reg_len: Length of the MHI MMIO region (required)
> * @fbc_image: Points to firmware image buffer
> * @rddm_image: Points to RAM dump buffer
> + * @standard_elf_image: Flag to make decision about firmware download start address (optional)
> * @mhi_chan: Points to the channel configuration table
> * @lpm_chans: List of channels that require LPM notifications
> * @irq: base irq # to request (required)
> @@ -456,6 +457,7 @@ struct mhi_controller {
> bool bounce_buf;
> bool fbc_download;
> bool wake_set;
> + bool standard_elf_image;

Which driver is making use of this flag?

- Mani

> unsigned long irq_flags;
> u32 mru;
> };
> --
> 2.7.4
>
>

--
மணிவண்ணன் சதாசிவம்

2023-08-22 16:20:47

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] mhi: host: Add standard ELF header image download functionality

Qiang Yu <[email protected]> writes:

> From: Mayank Rana <[email protected]>
>
> Some devices (e.g. WLAN chips) are unable to handle the non-standard ELF
> format of the FBC image and thus need special handling of the FBC image.
>
> Add standard_elf_image flag which makes decision in terms of how FBC image
> based AMSS image is being downloaded with connected endpoint.
> FBC image is having two image combine: SBL image + AMSS image.
> 1. FBC image download using legacy single ELF header image format:
> - SBL image: 512KB of FBC image is downloaded using BHI.
> - AMSS image: full FBC image is downloaded using BHIe.
> 2. FBC image download using separate ELF header image format:
> - SBL image: 512 KB of FBC image is downloaded using BHI.
> - AMSS image: 512 KB onward FBC image is downloaded using BHIe.
> There is no change for SBL image download. Although AMSS image start
> address is end address of SBL image while using separate ELF header format.
>
> Signed-off-by: Mayank Rana <[email protected]>
> [[email protected]: Update commit message, minor updates]
> Signed-off-by: Qiang Yu <[email protected]>

I guess this is for an out-of-tree driver? I haven't heard any such
requirements for ath11k or ath12k.

> --- a/drivers/bus/mhi/host/boot.c
> +++ b/drivers/bus/mhi/host/boot.c
> @@ -495,6 +495,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
> * device transitioning into MHI READY state
> */
> if (mhi_cntrl->fbc_download) {
> + dev_dbg(dev, "standard_elf_image: %s\n",
> + (mhi_cntrl->standard_elf_image ? "True" : "False"));
> + if (mhi_cntrl->standard_elf_image) {
> + fw_data = firmware->data + mhi_cntrl->sbl_size;
> + fw_sz = fw_sz - mhi_cntrl->sbl_size;
> + }

So you are basically skipping the first sbl_size bytes of the firmware
file? Why not just fix the firmware file in userspace? Or maybe you can
use the recently added[1] mhi_cntrl->fw_data pointer and handle this in
your driver instead?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/commit/?h=mhi-next&id=efe47a18e43f59f063a82ccaa464a3b4844bb8a8

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

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