This gives userspace the possibility to provide the firehose bootloader
via the sysfs-firmware-API instead of having to modify the global
firmware loadpath.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
Please note that this is not tested yet, as I don't have access to a matching
firmware file.
This submission is to gather general feedback from the maintainers and then
Richard will do the actual testing, while I'll do the development.
This patch is should not have any impact beyond moving from request_firmware()
to request_firmware_nowait() and the involved code reshuffle.
---
drivers/bus/mhi/core/boot.c | 159 +++++++++++++++++++++++-------------
1 file changed, 101 insertions(+), 58 deletions(-)
diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 74295d3cc662..b0576ffc69c6 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -18,6 +18,17 @@
#include <linux/wait.h>
#include "internal.h"
+struct mhi_fw_load_callback_ctx {
+ const char *fw_name;
+ struct mhi_controller *mhi_cntrl;
+};
+
+enum mhi_fw_load_status {
+ MHI_FW_LOAD_READY_STATE,
+ MHI_FW_ERROR_READY_STATE,
+ MHI_FW_ERROR_FW_LOAD,
+};
+
/* Setup RDDM vector table for RDDM transfer and program RXVEC */
void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
struct image_info *img_info)
@@ -386,55 +397,53 @@ static void mhi_firmware_copy(struct mhi_controller *mhi_cntrl,
}
}
-void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
+static void mhi_fw_load_finish(struct mhi_controller *mhi_cntrl, enum mhi_fw_load_status status)
{
- const struct firmware *firmware = NULL;
struct device *dev = &mhi_cntrl->mhi_dev->dev;
- const char *fw_name;
- void *buf;
- dma_addr_t dma_addr;
- size_t size;
- int i, ret;
+ int ret;
- if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
- dev_err(dev, "Device MHI is not in valid state\n");
- return;
+ switch (status) {
+ case MHI_FW_LOAD_READY_STATE:
+ break;
+ case MHI_FW_ERROR_READY_STATE:
+ goto error_ready_state;
+ case MHI_FW_ERROR_FW_LOAD:
+ goto error_fw_load;
}
- /* save hardware info from BHI */
- ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_SERIALNU,
- &mhi_cntrl->serial_number);
- if (ret)
- dev_err(dev, "Could not capture serial number via BHI\n");
-
- for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++) {
- ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i),
- &mhi_cntrl->oem_pk_hash[i]);
- if (ret) {
- dev_err(dev, "Could not capture OEM PK HASH via BHI\n");
- break;
- }
+ /* Transitioning into MHI RESET->READY state */
+ ret = mhi_ready_state_transition(mhi_cntrl);
+ if (ret) {
+ dev_err(dev, "MHI did not enter READY state\n");
+ goto error_ready_state;
}
- /* wait for ready on pass through or any other execution environment */
- if (!MHI_FW_LOAD_CAPABLE(mhi_cntrl->ee))
- goto fw_load_ready_state;
-
- fw_name = (mhi_cntrl->ee == MHI_EE_EDL) ?
- mhi_cntrl->edl_image : mhi_cntrl->fw_image;
+ dev_info(dev, "Wait for device to enter SBL or Mission mode\n");
+ return;
- if (!fw_name || (mhi_cntrl->fbc_download && (!mhi_cntrl->sbl_size ||
- !mhi_cntrl->seg_len))) {
- dev_err(dev,
- "No firmware image defined or !sbl_size || !seg_len\n");
- goto error_fw_load;
+error_ready_state:
+ if (mhi_cntrl->fbc_download) {
+ mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
+ mhi_cntrl->fbc_image = NULL;
}
- ret = request_firmware(&firmware, fw_name, dev);
- if (ret) {
- dev_err(dev, "Error loading firmware: %d\n", ret);
- goto error_fw_load;
- }
+error_fw_load:
+ mhi_cntrl->pm_state = MHI_PM_FW_DL_ERR;
+ wake_up_all(&mhi_cntrl->state_event);
+}
+
+static void mhi_fw_load_callback(const struct firmware *firmware, void *context)
+{
+ struct mhi_fw_load_callback_ctx *ctx = context;
+ const char *fw_name = ctx->fw_name;
+ struct mhi_controller *mhi_cntrl = ctx->mhi_cntrl;
+ struct device *dev = &mhi_cntrl->mhi_dev->dev;
+ dma_addr_t dma_addr;
+ size_t size;
+ void *buf;
+ int ret;
+
+ kfree(context);
size = (mhi_cntrl->fbc_download) ? mhi_cntrl->sbl_size : firmware->size;
@@ -446,7 +455,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
GFP_KERNEL);
if (!buf) {
release_firmware(firmware);
- goto error_fw_load;
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_ERROR_FW_LOAD);
}
/* Download image using BHI */
@@ -458,13 +467,13 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
if (ret) {
dev_err(dev, "MHI did not load image over BHI, ret: %d\n", ret);
release_firmware(firmware);
- goto error_fw_load;
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_ERROR_FW_LOAD);
}
/* Wait for ready since EDL image was loaded */
if (fw_name == mhi_cntrl->edl_image) {
release_firmware(firmware);
- goto fw_load_ready_state;
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_LOAD_READY_STATE);
}
write_lock_irq(&mhi_cntrl->pm_lock);
@@ -480,7 +489,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
firmware->size);
if (ret) {
release_firmware(firmware);
- goto error_fw_load;
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_ERROR_FW_LOAD);
}
/* Load the firmware into BHIE vec table */
@@ -489,26 +498,60 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
release_firmware(firmware);
-fw_load_ready_state:
- /* Transitioning into MHI RESET->READY state */
- ret = mhi_ready_state_transition(mhi_cntrl);
- if (ret) {
- dev_err(dev, "MHI did not enter READY state\n");
- goto error_ready_state;
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_LOAD_READY_STATE);
+}
+
+void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl)
+{
+ struct device *dev = &mhi_cntrl->mhi_dev->dev;
+ struct mhi_fw_load_callback_ctx *cb_ctx;
+ const char *fw_name;
+ int i, ret;
+
+ if (MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state)) {
+ dev_err(dev, "Device MHI is not in valid state\n");
+ return;
}
- dev_info(dev, "Wait for device to enter SBL or Mission mode\n");
- return;
+ /* save hardware info from BHI */
+ ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_SERIALNU,
+ &mhi_cntrl->serial_number);
+ if (ret)
+ dev_err(dev, "Could not capture serial number via BHI\n");
-error_ready_state:
- if (mhi_cntrl->fbc_download) {
- mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image);
- mhi_cntrl->fbc_image = NULL;
+ for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++) {
+ ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_OEMPKHASH(i),
+ &mhi_cntrl->oem_pk_hash[i]);
+ if (ret) {
+ dev_err(dev, "Could not capture OEM PK HASH via BHI\n");
+ break;
+ }
}
-error_fw_load:
- mhi_cntrl->pm_state = MHI_PM_FW_DL_ERR;
- wake_up_all(&mhi_cntrl->state_event);
+ /* wait for ready on pass through or any other execution environment */
+ if (!MHI_FW_LOAD_CAPABLE(mhi_cntrl->ee))
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_LOAD_READY_STATE);
+
+ 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->sbl_size ||
+ !mhi_cntrl->seg_len))) {
+ dev_err(dev,
+ "No firmware image defined or !sbl_size || !seg_len\n");
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_ERROR_FW_LOAD);
+ }
+
+ cb_ctx = kzalloc(sizeof(*cb_ctx), GFP_KERNEL);
+ if (!cb_ctx)
+ return;
+
+ ret = request_firmware_nowait(THIS_MODULE, true, fw_name, dev, GFP_KERNEL, cb_ctx,
+ mhi_fw_load_callback);
+ if (ret) {
+ dev_err(dev, "Error loading firmware: %d\n", ret);
+ mhi_fw_load_finish(mhi_cntrl, MHI_FW_ERROR_FW_LOAD);
+ }
}
int mhi_download_amss_image(struct mhi_controller *mhi_cntrl)
base-commit: 622e96fb58d63985b028abc2cb9a873124bdac1e
--
2.34.1
On 12/10/2021 8:16 AM, Thomas Weißschuh wrote:
> This gives userspace the possibility to provide the firehose bootloader
> via the sysfs-firmware-API instead of having to modify the global
> firmware loadpath.
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
>
> ---
>
> Please note that this is not tested yet, as I don't have access to a matching
> firmware file.
> This submission is to gather general feedback from the maintainers and then
> Richard will do the actual testing, while I'll do the development.
>
> This patch is should not have any impact beyond moving from request_firmware()
> to request_firmware_nowait() and the involved code reshuffle.
what are we achieving by moving to async ver of the firmware load ? MHI
boot flow can not do anything until BHI load is over. Is the intention
eventually to enable firmware fallback mechanism and manually load the
firmware ?
[..]
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project
On 2021-12-13 16:07-0800, Hemant Kumar wrote:
> On 12/10/2021 8:16 AM, Thomas Weißschuh wrote:
> > This gives userspace the possibility to provide the firehose bootloader
> > via the sysfs-firmware-API instead of having to modify the global
> > firmware loadpath.
> >
> > Signed-off-by: Thomas Weißschuh <[email protected]>
> >
> > ---
> >
> > Please note that this is not tested yet, as I don't have access to a matching
> > firmware file.
> > This submission is to gather general feedback from the maintainers and then
> > Richard will do the actual testing, while I'll do the development.
> >
> > This patch is should not have any impact beyond moving from request_firmware()
> > to request_firmware_nowait() and the involved code reshuffle.
> what are we achieving by moving to async ver of the firmware load ? MHI boot
> flow can not do anything until BHI load is over. Is the intention eventually
> to enable firmware fallback mechanism and manually load the firmware ?
The goal is to provide the firehose bootloader (qcom/prog_firehose_sdx24.mbn)
via the firmware fallback mechanism when upgrading the firmware on the device
via the firehose protocol.
This bootloader firmware is not part of linux-firmware but provided as part of
each firmware update package, so it is not installed statically on the system.
I will extend the commit message with this information.
PS: The current patch is missing 'return' after calls to
'mhi_fw_load_finish()', this will be corrected in v2.
On 12/13/2021 10:32 PM, Thomas Weißschuh wrote:
> On 2021-12-13 16:07-0800, Hemant Kumar wrote:
>> On 12/10/2021 8:16 AM, Thomas Weißschuh wrote:
>>> This gives userspace the possibility to provide the firehose bootloader
>>> via the sysfs-firmware-API instead of having to modify the global
>>> firmware loadpath.
>>>
>>> Signed-off-by: Thomas Weißschuh <[email protected]>
>>>
>>> ---
>>>
>>> Please note that this is not tested yet, as I don't have access to a matching
>>> firmware file.
>>> This submission is to gather general feedback from the maintainers and then
>>> Richard will do the actual testing, while I'll do the development.
>>>
>>> This patch is should not have any impact beyond moving from request_firmware()
>>> to request_firmware_nowait() and the involved code reshuffle.
>> what are we achieving by moving to async ver of the firmware load ? MHI boot
>> flow can not do anything until BHI load is over. Is the intention eventually
>> to enable firmware fallback mechanism and manually load the firmware ?
>
> The goal is to provide the firehose bootloader (qcom/prog_firehose_sdx24.mbn)
> via the firmware fallback mechanism when upgrading the firmware on the device
> via the firehose protocol.
>
> This bootloader firmware is not part of linux-firmware but provided as part of
> each firmware update package, so it is not installed statically on the system.
>
> I will extend the commit message with this information.
For my understanding i have follow up question. As per the kernel doc
https://www.kernel.org/doc/html/latest/driver-api/firmware/fallback-mechanisms.html
If CONFIG_FW_LOADER_USER_HELPER enabled but
CONFIG_FW_LOADER_USER_HELPER_FALLBACK is disabled, only the custom
fallback mechanism is available and for the request_firmware_nowait() call.
Custom fall back mechanism says
Users of the request_firmware_nowait() call have yet another option
available at their disposal: rely on the sysfs fallback mechanism but
request that no kobject uevents be issued to userspace. Original logic
behind this was that utilities other than udev might be required to
lookup firmware in non-traditional paths
Your patch is passing uevent flag as true which means you are relying on
uevent to be issued to userspace. How do you plan to update the firmware
path ? Alternatively firmware class provides a module param to specify
the firmware path /sys/module/firmware_class/parameters/path.
>
> PS: The current patch is missing 'return' after calls to
> 'mhi_fw_load_finish()', this will be corrected in v2.
>
Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project
On 2021-12-14 14:55-0800, Hemant Kumar wrote:
> On 12/13/2021 10:32 PM, Thomas Weißschuh wrote:
> > On 2021-12-13 16:07-0800, Hemant Kumar wrote:
> > > On 12/10/2021 8:16 AM, Thomas Weißschuh wrote:
> > > > This gives userspace the possibility to provide the firehose bootloader
> > > > via the sysfs-firmware-API instead of having to modify the global
> > > > firmware loadpath.
> > > >
> > > > Signed-off-by: Thomas Weißschuh <[email protected]>
> > > >
> > > > ---
> > > >
> > > > Please note that this is not tested yet, as I don't have access to a matching
> > > > firmware file.
> > > > This submission is to gather general feedback from the maintainers and then
> > > > Richard will do the actual testing, while I'll do the development.
> > > >
> > > > This patch is should not have any impact beyond moving from request_firmware()
> > > > to request_firmware_nowait() and the involved code reshuffle.
> > > what are we achieving by moving to async ver of the firmware load ? MHI boot
> > > flow can not do anything until BHI load is over. Is the intention eventually
> > > to enable firmware fallback mechanism and manually load the firmware ?
> >
> > The goal is to provide the firehose bootloader (qcom/prog_firehose_sdx24.mbn)
> > via the firmware fallback mechanism when upgrading the firmware on the device
> > via the firehose protocol.
> >
> > This bootloader firmware is not part of linux-firmware but provided as part of
> > each firmware update package, so it is not installed statically on the system.
> >
> > I will extend the commit message with this information.
>
> For my understanding i have follow up question. As per the kernel doc
> https://www.kernel.org/doc/html/latest/driver-api/firmware/fallback-mechanisms.html
I'll try to answer, but please be aware that I have no previous experience with
the firmware fallback mechanism and can not test this patch.
At this point in time I'm also hoping more for a general confirmation about
using *some* fallback mechanism for MHI, so Richard and Ivan can test the patch
(before it is committed) and then we can figure out exactly which of the
fallback mechanisms fits best with feedback from them.
> If CONFIG_FW_LOADER_USER_HELPER enabled but
> CONFIG_FW_LOADER_USER_HELPER_FALLBACK is disabled, only the custom fallback
> mechanism is available and for the request_firmware_nowait() call.
>
> Custom fall back mechanism says
> Users of the request_firmware_nowait() call have yet another option
> available at their disposal: rely on the sysfs fallback mechanism but
> request that no kobject uevents be issued to userspace. Original logic
> behind this was that utilities other than udev might be required to lookup
> firmware in non-traditional paths
>
> Your patch is passing uevent flag as true which means you are relying on
> uevent to be issued to userspace. How do you plan to update the firmware
> path ? Alternatively firmware class provides a module param to specify the
> firmware path /sys/module/firmware_class/parameters/path.
Emitting the uevent would allow the firmware update tool
(fwupd, https://github.com/fwupd/fwupd) to monitor uevents and recognize when
the device is ready to receive the firmware and then trigger the upload.
If I see it correctly uevent=0 and uevent=1 do the same things except uevent=1
also publishes the uevent.
Modifying /sys/module/firmware_class/parameters/path is what currently is being
done.
But modifying the global firmware load path has the potential for the following
issues:
* While the global firmware load path is modified to a custom location any
load_firmware() call from other devices will fail because the new path does
not have the normal linux-firmware.
* If the tool modifying crashes while before restoring the original load path
all further load_firmware()-calls will also fail.
> >
> > PS: The current patch is missing 'return' after calls to
> > 'mhi_fw_load_finish()', this will be corrected in v2.
Another point:
For sdx55 and sdx65 there is not only an edl firmware specified but also a
firmware for normal operation.
This firmware is not part of linux-firmware.
Currently the firmware load would fail and put the modem into an error
condition (I think?).
With this patch there would be a timeout before that error state is reached.
I know that some people have the SDX55 running with Linux but I'm now wondering
where they are getting the firmware "qcom/sdx55m/sbl1.mbn" from.
Thomas
Thomas Weißschuh <[email protected]> writes:
> This gives userspace the possibility to provide the firehose bootloader
> via the sysfs-firmware-API instead of having to modify the global
> firmware loadpath.
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
>
> ---
>
> Please note that this is not tested yet, as I don't have access to a matching
> firmware file.
> This submission is to gather general feedback from the maintainers and then
> Richard will do the actual testing, while I'll do the development.
>
> This patch is should not have any impact beyond moving from request_firmware()
> to request_firmware_nowait() and the involved code reshuffle.
Related to firmware loading, for ath11k I have a different kind of need
for firmware handling in MHI. Instead of providing the firmware name to
MHI and MHI subystem loading the firmware from user space, I would like
to ath11k load the firmware from user space and just provide a pointer
to the firmware data.
The long story here is that currently ath11k pci devices have two
different firmware files, amss.bin and m3.bin. amss.bin is loaded via
MHI and m3.bin via QMI. What I would like do is to create one container
file firmware-2.bin and it would contain amss.bin, m3.bin and various
meta data needed by ath11k. ath11k would then parse firmware-2.bin and
provide pointer to amss.bin data for MHI.
We already use similar firmware-2.bin file in ath10k, but of course
ath10k doesn't need MHI so this hasn't been an issue before. We need
firmware-2.bin files because of two reasons: seamless backwards
compatibility and a reliable way to provide meta data to the driver.
Thoughts about this?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches