Received-SPF: pass (google.com: domain of linux-kernel+bounces-128249-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <024549ba-4522-d8d0-08ea-c42966f850af@quicinc.com>
Date: Tue, 2 Apr 2024 09:33:26 -0600
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.0
Subject: Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter
 EDL
Content-Language: en-US
To: Qiang Yu <quic_qianyu@quicinc.com>,
        Manivannan Sadhasivam
	<mani@kernel.org>
CC: <mhi@lists.linux.dev>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_cang@quicinc.com>,
        <quic_mrana@quicinc.com>, Bhaumik Bhatt <quic_bbhatt@quicinc.com>
References: <1703490474-84730-1-git-send-email-quic_qianyu@quicinc.com>
 <cff4b828-9566-a2bd-287a-138d74a76a59@quicinc.com>
 <20240102165229.GC4917@thinkpad>
 <90c0a654-a02f-46e2-96a9-34f6a30c95a0@quicinc.com>
 <a10439f1-0fcd-834c-12a3-677976529cf1@quicinc.com>
 <e78382b5-428e-4de8-be0d-b319534238f1@quicinc.com>
 <0cfac65c-8b71-4900-88a3-631c93aebc17@quicinc.com>
From: Jeffrey Hugo <quic_jhugo@quicinc.com>
In-Reply-To: <0cfac65c-8b71-4900-88a3-631c93aebc17@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

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