Skip to contentSkip to conversation

i3c: mipi-i3c-hci-pci: Add System Suspend support

PATCH Superseded Adrian Hunter 2 participants 7 messages Jan 19 I3C

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover i3c: mipi-i3c-hci-pci: Add System Suspend support

Hi

Changes in V2:

i3c: master: Add i3c_master_do_daa_ext() for post-hibernation address recovery
Helper function is now called i3c_master_do_daa_ext() and takes 'restore' as an argument.
Fix new helper function so that RSTDAA is conditional on the 'restore' parameter.
Subject and commit message amended accordingly.

i3c: mipi-i3c-hci: Add optional System Suspend support
Helper function is now called i3c_master_do_daa_ext() and takes 'restore' as an argument.
Commit message amended accordingly.

Here are 3 small patches to add System Suspend support to mipi-i3c-hci and mipi-i3c-hci-pci.

Adrian Hunter (3):
i3c: master: Add i3c_master_do_daa_ext() for post-hibernation address recovery
i3c: mipi-i3c-hci: Add optional System Suspend support
i3c: mipi-i3c-hci-pci: Add System Suspend support

 drivers/i3c/master.c                               | 49 +++++++++++++++++-----
 drivers/i3c/master/mipi-i3c-hci/core.c             | 49 ++++++++++++++++++++++
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c |  7 ++++
 include/linux/i3c/master.h                         |  1 +
 4 files changed, 95 insertions(+), 11 deletions(-)

Regards
Adrian

Signature · 3 lines
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
#1 lore raw

PATCH v2 1/3 i3c: master: Add i3c_master_do_daa_ext() for post-hibernation address recovery

After system hibernation, I3C Dynamic Addresses may be reassigned at boot and no longer match the values recorded before suspend. Introduce i3c_master_do_daa_ext() to handle this situation.

The restore procedure is straightforward: issue a Reset Dynamic Address Assignment (RSTDAA), then run the standard DAA sequence. The existing DAA logic already supports detecting and updating devices whose dynamic addresses differ from previously known values.

Refactor the DAA path by introducing a shared helper used by both the normal i3c_master_do_daa() path and the new extended restore function, and correct the kernel-doc in the process.

Export i3c_master_do_daa_ext() so that master drivers can invoke it from their PM restore callbacks.

---
 drivers/i3c/master.c       | 49 +++++++++++++++++++++++++++++---------
 include/linux/i3c/master.h |  1 +
 2 files changed, 39 insertions(+), 11 deletions(-)
drivers/i3c/master.c +38 -11
index 594d61edcef4..ca1587cddb9b 100644
@@ -1735,22 +1735,24 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
 }
 
 /**
- * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
- * @master: master doing the DAA
+ * i3c_master_do_daa_ext() - Dynamic Address Assignment (extended version)
+ * @master: controller
+ * @restore: hibernation restore
  *
- * This function is instantiating an I3C device object and adding it to the
- * I3C device list. All device information are automatically retrieved using
- * standard CCC commands.
- *
- * The I3C device object is returned in case the master wants to attach
- * private data to it using i3c_dev_set_master_data().
+ * Perform Dynamic Address Assignment with optional support for System
+ * Hibernation (@restore is true).
  *
- * This function must be called with the bus lock held in write mode.
+ * After System Hibernation, Dynamic Addresses can have been reassigned at boot
+ * time to different values. A simple strategy is followed to handle that.
+ * Perform a Reset of Dynamic Address (RSTDAA) followed by the normal DAA
+ * procedure which has provision for reassigning addresses that differ from the
+ * previously recorded addresses.
  *
  * Return: a 0 in case of success, an negative error code otherwise.
  */
-int i3c_master_do_daa(struct i3c_master_controller *master)
+int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool restore)
 {
+	int rstret = 0;
 	int ret;
 
 	ret = i3c_master_rpm_get(master);
@@ -1758,7 +1760,15 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
 		return ret;
 
 	i3c_bus_maintenance_lock(&master->bus);
+
+	if (restore) {
+		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
+		if (rstret == I3C_ERROR_M2)
+			rstret = 0;
+	}
+
 	ret = master->ops->do_daa(master);
+
 	i3c_bus_maintenance_unlock(&master->bus);
 
 	if (ret)
@@ -1769,7 +1779,24 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
 	i3c_bus_normaluse_unlock(&master->bus);
 out:
 	i3c_master_rpm_put(master);
-	return ret;
+
+	return rstret ?: ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_do_daa_ext);
+
+/**
+ * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
+ * @master: master doing the DAA
+ *
+ * This function instantiates I3C device objects and adds them to the
+ * I3C device list. All device information is automatically retrieved using
+ * standard CCC commands.
+ *
+ * Return: a 0 in case of success, an negative error code otherwise.
+ */
+int i3c_master_do_daa(struct i3c_master_controller *master)
+{
+	return i3c_master_do_daa_ext(master, false);
 }
 EXPORT_SYMBOL_GPL(i3c_master_do_daa);
 
include/linux/i3c/master.h +1 -0
index c1ec597f655c..5d0d5359a03e 100644
@@ -598,6 +598,7 @@ int i3c_master_get_free_addr(struct i3c_master_controller *master,
 int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 				  u8 addr);
 int i3c_master_do_daa(struct i3c_master_controller *master);
+int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool restore);
 struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *ptr,
 					  size_t len, bool force_bounce,
 					  enum dma_data_direction dir);
Signature · 7 lines
2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
reviewer
#2 lore raw

Re: [PATCH V2 1/3] i3c: master: Add i3c_master_do_daa_ext() for post-hibernation address recovery

Quoted 18 lines · Adrian Hunter

After system hibernation, I3C Dynamic Addresses may be reassigned at boot and no longer match the values recorded before suspend. Introduce i3c_master_do_daa_ext() to handle this situation.

The restore procedure is straightforward: issue a Reset Dynamic Address Assignment (RSTDAA), then run the standard DAA sequence. The existing DAA logic already supports detecting and updating devices whose dynamic addresses differ from previously known values.

Refactor the DAA path by introducing a shared helper used by both the normal i3c_master_do_daa() path and the new extended restore function, and correct the kernel-doc in the process.

Export i3c_master_do_daa_ext() so that master drivers can invoke it from their PM restore callbacks.

---

...

drivers/i3c/master.c i3c_master_register_new_i3c_devs(struct i3c_master_controller *master) · line 1753

Quoted 2 lines
drivers/i3c/master.c +1 -1
-int i3c_master_do_daa(struct i3c_master_controller *master)
+int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool restore)

is 'rstdaa' better than 'restore'? 'rstdaa' indicate what this API did actually.

Frank

Quoted 67 lines

{

drivers/i3c/master.c +27 -1
+	int rstret = 0;
 	int ret;
 
 	ret = i3c_master_rpm_get(master);
@@ -1758,7 +1760,15 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
 		return ret;
 
 	i3c_bus_maintenance_lock(&master->bus);
+
+	if (restore) {
+		rstret = i3c_master_rstdaa_locked(master, I3C_BROADCAST_ADDR);
+		if (rstret == I3C_ERROR_M2)
+			rstret = 0;
+	}
+
 	ret = master->ops->do_daa(master);
+
 	i3c_bus_maintenance_unlock(&master->bus);
 
 	if (ret)
@@ -1769,7 +1779,24 @@ int i3c_master_do_daa(struct i3c_master_controller *master)
 	i3c_bus_normaluse_unlock(&master->bus);
 out:
 	i3c_master_rpm_put(master);
-	return ret;
+
+	return rstret ?: ret;
+}
+EXPORT_SYMBOL_GPL(i3c_master_do_daa_ext);
+
+/**
+ * i3c_master_do_daa() - do a DAA (Dynamic Address Assignment)
+ * @master: master doing the DAA
+ *
+ * This function instantiates I3C device objects and adds them to the
+ * I3C device list. All device information is automatically retrieved using
+ * standard CCC commands.
+ *
+ * Return: a 0 in case of success, an negative error code otherwise.
+ */
+int i3c_master_do_daa(struct i3c_master_controller *master)
+{
+	return i3c_master_do_daa_ext(master, false);
 }
 EXPORT_SYMBOL_GPL(i3c_master_do_daa);
 
include/linux/i3c/master.h +1 -1
index c1ec597f655c..5d0d5359a03e 100644
@@ -598,6 +598,7 @@ int i3c_master_get_free_addr(struct i3c_master_controller *master,
 int i3c_master_add_i3c_dev_locked(struct i3c_master_controller *master,
 				  u8 addr);
 int i3c_master_do_daa(struct i3c_master_controller *master);
+int i3c_master_do_daa_ext(struct i3c_master_controller *master, bool restore);
 struct i3c_dma *i3c_master_dma_map_single(struct device *dev, void *ptr,
 					  size_t len, bool force_bounce,
 					  enum dma_data_direction dir);
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Signature · 3 lines
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
#3 lore raw

PATCH v2 2/3 i3c: mipi-i3c-hci: Add optional System Suspend support

In reply to Adrian Hunter

Add system suspend callbacks. Implement them by forcing runtime PM. Consequently bail out if Runtime PM is not allowed.

On resume from System Suspend (suspend to RAM), rerun Dynamic Address Assignment to restore addresses for devices that may have lost power.

On resume from System Hibernation (suspend to disk), use the new i3c_master_do_daa_ext() helper with 'restore' set to true, which additionally handles the case where devices are assigned different dynamic addresses after a hibernation boot.

---

Changes in V2:

Helper function is now called i3c_master_do_daa_ext() and takes 'restore' as an argument

 drivers/i3c/master/mipi-i3c-hci/core.c | 49 ++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
drivers/i3c/master/mipi-i3c-hci/core.c +49 -0
index 02c5a133e329..ac8cfec1ff9e 100644
@@ -769,6 +769,49 @@ static int i3c_hci_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int i3c_hci_suspend(struct device *dev)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+
+	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
+		return 0;
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int i3c_hci_resume_common(struct device *dev, bool restore)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+	int ret;
+
+	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
+		return 0;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	ret = i3c_master_do_daa_ext(&hci->master, restore);
+	if (ret)
+		dev_err(dev, "Dynamic Address Assignment failed on resume, error %d\n", ret);
+
+	/*
+	 * I3C devices may have retained their dynamic address anyway. Do not
+	 * fail the resume because of DAA error.
+	 */
+	return 0;
+}
+
+static int i3c_hci_resume(struct device *dev)
+{
+	return i3c_hci_resume_common(dev, false);
+}
+
+static int i3c_hci_restore(struct device *dev)
+{
+	return i3c_hci_resume_common(dev, true);
+}
+
 #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
 
 static void i3c_hci_rpm_enable(struct device *dev)
@@ -945,6 +988,12 @@ static const struct platform_device_id i3c_hci_driver_ids[] = {
 MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
 
 static const struct dev_pm_ops i3c_hci_pm_ops = {
+	.suspend  = pm_sleep_ptr(i3c_hci_suspend),
+	.resume   = pm_sleep_ptr(i3c_hci_resume),
+	.freeze   = pm_sleep_ptr(i3c_hci_suspend),
+	.thaw     = pm_sleep_ptr(i3c_hci_resume),
+	.poweroff = pm_sleep_ptr(i3c_hci_suspend),
+	.restore  = pm_sleep_ptr(i3c_hci_restore),
 	RUNTIME_PM_OPS(i3c_hci_runtime_suspend, i3c_hci_runtime_resume, NULL)
 };
 
Signature · 7 lines
2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
reviewer Reviewed-by on v2 2/3
#4 lore raw

Re: [PATCH V2 2/3] i3c: mipi-i3c-hci: Add optional System Suspend support

Quoted 6 lines · Adrian Hunter

Add system suspend callbacks. Implement them by forcing runtime PM. Consequently bail out if Runtime PM is not allowed.

On resume from System Suspend (suspend to RAM), rerun Dynamic Address Assignment to restore addresses for devices that may have lost power.

Quoted 7 lines

On resume from System Hibernation (suspend to disk), use the new i3c_master_do_daa_ext() helper with 'restore' set to true, which additionally handles the case where devices are assigned different dynamic addresses after a hibernation boot.

---

Quoted 86 lines

Changes in V2:

Helper function is now called i3c_master_do_daa_ext() and takes 'restore' as an argument

 drivers/i3c/master/mipi-i3c-hci/core.c | 49 ++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
drivers/i3c/master/mipi-i3c-hci/core.c +49 -1
index 02c5a133e329..ac8cfec1ff9e 100644
@@ -769,6 +769,49 @@ static int i3c_hci_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int i3c_hci_suspend(struct device *dev)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+
+	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
+		return 0;
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int i3c_hci_resume_common(struct device *dev, bool restore)
+{
+	struct i3c_hci *hci = dev_get_drvdata(dev);
+	int ret;
+
+	if (!(hci->quirks & HCI_QUIRK_RPM_ALLOWED))
+		return 0;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	ret = i3c_master_do_daa_ext(&hci->master, restore);
+	if (ret)
+		dev_err(dev, "Dynamic Address Assignment failed on resume, error %d\n", ret);
+
+	/*
+	 * I3C devices may have retained their dynamic address anyway. Do not
+	 * fail the resume because of DAA error.
+	 */
+	return 0;
+}
+
+static int i3c_hci_resume(struct device *dev)
+{
+	return i3c_hci_resume_common(dev, false);
+}
+
+static int i3c_hci_restore(struct device *dev)
+{
+	return i3c_hci_resume_common(dev, true);
+}
+
 #define DEFAULT_AUTOSUSPEND_DELAY_MS 1000
 
 static void i3c_hci_rpm_enable(struct device *dev)
@@ -945,6 +988,12 @@ static const struct platform_device_id i3c_hci_driver_ids[] = {
 MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
 
 static const struct dev_pm_ops i3c_hci_pm_ops = {
+	.suspend  = pm_sleep_ptr(i3c_hci_suspend),
+	.resume   = pm_sleep_ptr(i3c_hci_resume),
+	.freeze   = pm_sleep_ptr(i3c_hci_suspend),
+	.thaw     = pm_sleep_ptr(i3c_hci_resume),
+	.poweroff = pm_sleep_ptr(i3c_hci_suspend),
+	.restore  = pm_sleep_ptr(i3c_hci_restore),
 	RUNTIME_PM_OPS(i3c_hci_runtime_suspend, i3c_hci_runtime_resume, NULL)
 };
 
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Signature · 3 lines
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
#5 lore raw

PATCH v2 3/3 i3c: mipi-i3c-hci-pci: Add System Suspend support

In reply to Adrian Hunter

Assign the driver PM operations pointer, which is necessary for the PCI subsystem to put the device into a low power state. Refer to pci_pm_suspend_noirq() which bails out if the pointer is NULL, before it has the opportunity to call pci_prepare_to_sleep().

No other actions are necessary as the mipi-i3c-hci driver takes care of controller state.

---

Chnages in V2:

None

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +7 -0
index 1b38771667e5..0f05a15c14c7 100644
@@ -320,6 +320,10 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	mfd_remove_devices(&pci->dev);
 }
 
+/* PM ops must exist for PCI to put a device to a low power state */
+static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
+};
+
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
 	/* Wildcat Lake-U */
 	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_mi_1_info},
@@ -342,6 +346,9 @@ static struct pci_driver mipi_i3c_hci_pci_driver = {
 	.id_table = mipi_i3c_hci_pci_devices,
 	.probe = mipi_i3c_hci_pci_probe,
 	.remove = mipi_i3c_hci_pci_remove,
+	.driver = {
+		.pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
+	},
 };
 
 module_pci_driver(mipi_i3c_hci_pci_driver);
Signature · 7 lines
2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c
reviewer Reviewed-by on v2 3/3
#6 lore raw
Quoted 10 lines · Adrian Hunter

Assign the driver PM operations pointer, which is necessary for the PCI subsystem to put the device into a low power state. Refer to pci_pm_suspend_noirq() which bails out if the pointer is NULL, before it has the opportunity to call pci_prepare_to_sleep().

No other actions are necessary as the mipi-i3c-hci driver takes care of controller state.

---

Quoted 43 lines

Chnages in V2:

None

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c +7 -1
index 1b38771667e5..0f05a15c14c7 100644
@@ -320,6 +320,10 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	mfd_remove_devices(&pci->dev);
 }
 
+/* PM ops must exist for PCI to put a device to a low power state */
+static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
+};
+
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
 	/* Wildcat Lake-U */
 	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_mi_1_info},
@@ -342,6 +346,9 @@ static struct pci_driver mipi_i3c_hci_pci_driver = {
 	.id_table = mipi_i3c_hci_pci_devices,
 	.probe = mipi_i3c_hci_pci_probe,
 	.remove = mipi_i3c_hci_pci_remove,
+	.driver = {
+		.pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
+	},
 };
 
 module_pci_driver(mipi_i3c_hci_pci_driver);
--

2.51.0

--
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Signature · 3 lines
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c