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

Message ID 1703490474-84730-1-git-send-email-quic_qianyu@quicinc.com
State New
Headers
Series bus: mhi: host: Add sysfs entry to force device to enter EDL |

Commit Message

Qiang Yu Dec. 25, 2023, 7:47 a.m. UTC
  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(+)
  

Comments

Jeffrey Hugo Jan. 2, 2024, 3:31 p.m. UTC | #1
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
  
Manivannan Sadhasivam Jan. 2, 2024, 4:52 p.m. UTC | #2
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
  
Qiang Yu Jan. 9, 2024, 8:43 a.m. UTC | #3
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
  
Qiang Yu Jan. 9, 2024, 9:20 a.m. UTC | #4
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
  
Jeffrey Hugo Jan. 11, 2024, 7:08 p.m. UTC | #5
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
  

Patch

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;