lkml.org 
[lkml]   [2024]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL
From

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@quicinc.com>
>>>>>>>>>>>
>>>>>>>>>>> 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.


\
 
 \ /
  Last update: 2024-05-27 16:30    [W:0.134 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site