Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL
From: Qiang Yu
Date: Mon Apr 08 2024 - 23:32:25 EST
On 4/8/2024 6:15 PM, Manivannan Sadhasivam wrote:
On Mon, Apr 08, 2024 at 04:10:40PM +0800, Qiang Yu wrote:
On 4/3/2024 1:44 PM, Qiang Yu wrote:
On 4/2/2024 11:33 PM, Jeffrey Hugo wrote:
On 4/2/2024 7:52 AM, Qiang Yu wrote:
On 4/2/2024 12:34 PM, Qiang Yu wrote:
On 1/12/2024 3:08 AM, Jeffrey Hugo wrote:
On 1/9/2024 2:20 AM, Qiang Yu wrote:
On 1/3/2024 12:52 AM, Manivannan Sadhasivam wrote:
On Tue, Jan 02, 2024 at 08:31:15AM -0700, Jeffrey Hugo wrote:
On 12/25/2023 12:47 AM, Qiang Yu wrote:
From: Bhaumik Bhatt <quic_bbhatt@xxxxxxxxxxx>
Forcing the device (eg. SDX75) to enter
Emergency Download Mode involves
writing the 0xEDEDEDED cookie to the
channel 91 doorbell register and
forcing an SOC reset afterwards. Allow
users of the MHI bus to exercise the
sequence using a sysfs entry.
I don't see this documented in the spec
anywhere. Is this standard behavior
for all MHI devices?
What about devices that don't support EDL mode?
How should the host avoid using this special
cookie when EDL mode is not
desired?
All points raised by Jeff are valid. I had
discussions with Hemant and Bhaumik
previously on allowing the devices to enter EDL
mode in a generic manner and we
didn't conclude on one final approach.
Whatever way we come up with, it should be
properly described in the MHI spec
and _should_ be backwards compatible.
Hi Mani, Jeff. The method of entering EDL mode is
documented in MHI spec v1.2, Chapter 13.2.
Could you please check once?
I do see it listed there. However that was a FR for
SDX55, so devices prior to that would not support this.
AIC100 predates this change and would not support the
functionality. I verified the AIC100 implementation is
not aware of this cookie.
Also, that functionality depends on channel 91 being
reserved per the table 9-2, however that table only
applies to modem class devices as it is under chapter 9
"Modem protocols over PCIe". Looking at the ath11k and
ath12k implementations in upstream, it looks like they
partially comply. Other devices have different MHI
channel definitions.
Chapter 9 doesn't appear to be in older versions of the
spec that I have, so it is unclear if this functionality
is backwards compatible (was channel 91 used for another
purpose in pre-SDX55 modems).
I'm not convinced this belongs in the MHI core. At a
minimum, the MHI controller(s) for the applicable
devices needs to opt-in to this.
-Jeff
Hi Jeff
Sorry for reply so late. In older versions of the spec,
there is no description about EDL doorbell. However, in MHI
spec v1.2, section 13.2,
It explicitly says "To set the EDL cookie, the host writes
0xEDEDEDED to channel doorbell 91." So I think every device
based on MHI spec v1.2
should reserve channel doorbell 91 for EDL mode.
So can we add another flag called mhi_ver in mhi controller
to indicate its mhi version and then we can add mhi_ver
checking to determine if this
device supports EDL sysfs operation?
Thanks,
Qiang
I discussed with internal team, look like devices that reserve
channel doorbell 91 for EDL, thier MHIVER register value can
still be 1.0 instead
of 1.2. So even if we add a flag called mhi_ver to store the
value read from the MHIVER register. We still can not do EDL
support check depend on it.
But I still think enter EDL mode by writing EDL cookie to
channel doorbell is a standard way. At least it's a standard way
from MHI spec V1.2.
In mhi_controller, we have a variable edl_image representing the
name and path of firmware. But We still can not determine if the
device reserve
channel doorbell 91 by checking this because some devices may
enter EDL mode in different way. Mayebe we have to add a flag in
mhi_controller
called edl_support to do the check.
So, not all devices support EDL mode (even v1.2 devices, which I
know of one in development). Of the devices that support EDL mode,
not all of them use the same mechanism to enter EDL mode.
It appears all of this needs to be shoved to the controller.
At best, I think the controller can provide an optional EDL
callback. If the callback is provided, then MHI creates a sysfs
entry (similar to soc_reset) for the purpose of entering EDL mode.
If the sysfs entry is called, all MHI does is call the controller's
callback.
-Jeff
Hi Jeff
This idea looks good. We can add edl call back in mhi_pci_dev_info and
assgin it to mhi controller during probe.
Meanwhile, we can get edl doorbell address in this callback instead of
mhi_init_mmio.
Mani, what do you think about it? Can I implement the EDL sysfs entry
like this?
Hi Mani, Jeff
I plan to implement EDL sysfs entry as Jeff suggested.
1. Add an optional EDL callback in mhi_pci_dev_info and assign it to mhi
controller during probe. All logic
to enter EDL mode will be moved in this EDL callback.
2. Create EDL sysfs entry anyway, and check if EDL callback exists, run EDL
callback, otherwise print log
and return.
You should not print anything on unsupported platforms while introducing a new
feature.
MHI stack should first check for the existence of the EDL callback and then only
it should try to create the sysfs entry. But the EDL callback varies from device
to device afaik, so I would've fancied to pass the callback from the
mhi_controller_config structure. But the config is meant to provide config
options as opposed to callbacks.
So I think a neat way would be to add a new flag,
mhi_controller_config::edl_trigger. Then enable that flag in the config of
supported devices and during mhi_pci_probe(), pass the
mhi_pci_generic_edl_trigger() function as the callback to
mhi_controller::edl_trigger.
In the future, if we happen to add more EDL triggering mechanisms (vendor
specific), then we can use bitfields to differentiate them.
- Mani
மணிவண்ணன் சதாசிவம்
Hi Mani, thanks for your comments, let me prepare next version patch as
you and Jeff suggested.