[PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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.

Signed-off-by: Bhaumik Bhatt <quic_bbhatt@quicinc.com>
Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/bus/mhi/host/init.c     | 37 +++++++++++++++++++++++++++++++++++++
 drivers/bus/mhi/host/internal.h |  1 +
 include/linux/mhi.h             |  2 ++
 3 files changed, 40 insertions(+)

diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index 65ceac1..4654bc5 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -120,10 +120,46 @@ static ssize_t soc_reset_store(struct device *dev,
 }
 static DEVICE_ATTR_WO(soc_reset);
 
+static ssize_t force_edl_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct mhi_device *mhi_dev = to_mhi_device(dev);
+	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 10, &val);
+	if (ret < 0) {
+		dev_err(dev, "Could not parse string: %d\n", ret);
+		return ret;
+	}
+
+	if (!val)
+		return count;
+
+	ret = mhi_device_get_sync(mhi_cntrl->mhi_dev);
+	if (ret)
+		return ret;
+
+	pm_wakeup_event(&mhi_cntrl->mhi_dev->dev, 0);
+	mhi_cntrl->runtime_get(mhi_cntrl);
+
+	mhi_write_db(mhi_cntrl, mhi_cntrl->edl_db, 0xEDEDEDED);
+	mhi_soc_reset(mhi_cntrl);
+
+	mhi_cntrl->runtime_put(mhi_cntrl);
+	mhi_device_put(mhi_cntrl->mhi_dev);
+
+	return count;
+}
+static DEVICE_ATTR_WO(force_edl);
+
 static struct attribute *mhi_dev_attrs[] = {
 	&dev_attr_serial_number.attr,
 	&dev_attr_oem_pk_hash.attr,
 	&dev_attr_soc_reset.attr,
+	&dev_attr_force_edl.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(mhi_dev);
@@ -524,6 +560,7 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
 
 	/* Setup wake db */
 	mhi_cntrl->wake_db = base + val + (8 * MHI_DEV_WAKE_DB);
+	mhi_cntrl->edl_db = base + val + (8 * MHI_EDL_DB);
 	mhi_cntrl->wake_set = false;
 
 	/* Setup channel db address for each channel in tre_ring */
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 30ac415..c414ebf 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -128,6 +128,7 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING					128
 #define PRIMARY_CMD_RING				0
 #define MHI_DEV_WAKE_DB					127
+#define MHI_EDL_DB					91
 #define MHI_MAX_MTU					0xffff
 #define MHI_RANDOM_U32_NONZERO(bmsk)			(get_random_u32_inclusive(1, bmsk))
 
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d0f9b522..c754d32 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -298,6 +298,7 @@ struct mhi_controller_config {
  * @bhi: Points to base of MHI BHI register space
  * @bhie: Points to base of MHI BHIe register space
  * @wake_db: MHI WAKE doorbell register address
+ * @edl_db: MHI EDL channel 91 doorbell register address
  * @iova_start: IOMMU starting address for data (required)
  * @iova_stop: IOMMU stop address for data (required)
  * @fw_image: Firmware image name for normal booting (optional)
@@ -387,6 +388,7 @@ struct mhi_controller {
 	void __iomem *bhi;
 	void __iomem *bhie;
 	void __iomem *wake_db;
+	void __iomem *edl_db;
 
 	dma_addr_t iova_start;
 	dma_addr_t iova_stop;
-- 
2.7.4

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Jeffrey Hugo]

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?

-Jeff

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Manivannan Sadhasivam]

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.

- Mani

> -Jeff

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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?

>
> - Mani
>
>> -Jeff

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Jeffrey Hugo]

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

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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.

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Jeffrey Hugo]

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

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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?

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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.

3. Get EDL doorbell (channel doorbell 91) address is also moved to EDL 
callback instead of mhi_init_mmio,
    so that we don't have to add another flag to tell if the device 
reserves channel 91 for purpose to enter EDL.

May I have your further comments?

Thanks,
Qiang

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Manivannan Sadhasivam]

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

மணிவண்ணன் சதாசிவம்

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

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.

Re: [PATCH] bus: mhi: host: Add sysfs entry to force device to enter EDL

[Qiang Yu]

On 1/2/2024 11:31 PM, 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?

This is documented in MHI spec v1.2, 13.2 Emergency download (EDL) mode 
cookie. So I think

it is standard behavior. At least, SDX65 and SDX75 support it.

>
> What about devices that don't support EDL mode?
>
> How should the host avoid using this special cookie when EDL mode is 
> not desired?

Can I include another flag in mhi_pci_dev_info and mhi_controller and 
check this flag

before writing EDL cookie?

>
> -Jeff