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

[Qiang Yu]

On 8/22/2023 7:51 PM, Kalle Valo wrote:
> Qiang Yu<quic_qianyu@xxxxxxxxxxx>  writes:
>
> > From: Mayank Rana<quic_mrana@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > [quic_qianyu@xxxxxxxxxxx: Update commit message, minor updates]
> > Signed-off-by: Qiang Yu<quic_qianyu@xxxxxxxxxxx>
> 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