| Message ID | 1691395192-16090-1-git-send-email-quic_qianyu@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mhi: host: Add standard ELF header image download functionality | expand |
On Mon, Aug 07, 2023 at 03:59:52PM +0800, Qiang Yu wrote: > From: Mayank Rana <quic_mrana@quicinc.com> > 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 <quic_mrana@quicinc.com> > [quic_qianyu@quicinc.com: Update commit message, minor updates] > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> > --- > 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 > >
Qiang Yu <quic_qianyu@quicinc.com> writes: > From: Mayank Rana <quic_mrana@quicinc.com> > > 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 <quic_mrana@quicinc.com> > [quic_qianyu@quicinc.com: Update commit message, minor updates] > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com> 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
On 8/22/2023 7:51 PM, Kalle Valo wrote: > Qiang Yu<quic_qianyu@quicinc.com> writes: > >> From: Mayank Rana<quic_mrana@quicinc.com> >> >> 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<quic_mrana@quicinc.com> >> [quic_qianyu@quicinc.com: Update commit message, minor updates] >> Signed-off-by: Qiang Yu<quic_qianyu@quicinc.com> > I guess this is for an out-of-tree driver? I haven't heard any such > requirements for ath11k or ath12k. Yes, this is for next generation wlan chip. After confirming with WLAN team, we will hold this patch until WLAN team upstream the driver for next generation wlan chip. >> --- 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? Thanks for your suggestions. As per Mayank, with current method, we are able to accommodate new image request better way with less code change, and satisfy processing of RDDM without any modification. Maybe we can revisit it when the driver for next gen wlan driver comes in picture. > [1]https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/commit/?h=mhi-next&id=efe47a18e43f59f063a82ccaa464a3b4844bb8a8 >
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; };