| Message ID | 4017204.UeppN1BopX@aspire.rjw.lan (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
On 16 October 2017 at 03:29, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Modify i2c-designware-platdrv to set DPM_FLAG_SMART_PREPARE for its > devices and return 0 from the system suspend ->prepare callback > if the device has an ACPI companion object in order to tell the PM > core and middle layers to avoid skipping system suspend/resume > callbacks for the device in that case (which may be problematic, > because the device may be accessed during suspend and resume of > other devices via I2C operation regions then). > > Also the pm_runtime_suspended() check in dw_i2c_plat_prepare() > is not necessary any more, because the core does it when setting > power.direct_complete for the device, so drop it. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/i2c/busses/i2c-designware-platdrv.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c > =================================================================== > --- linux-pm.orig/drivers/i2c/busses/i2c-designware-platdrv.c > +++ linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c > @@ -370,6 +370,8 @@ static int dw_i2c_plat_probe(struct plat > ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); > adap->dev.of_node = pdev->dev.of_node; > > + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); > + > /* The code below assumes runtime PM to be disabled. */ > WARN_ON(pm_runtime_enabled(&pdev->dev)); > > @@ -433,7 +435,13 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match) > #ifdef CONFIG_PM_SLEEP > static int dw_i2c_plat_prepare(struct device *dev) > { > - return pm_runtime_suspended(dev); > + /* > + * If the ACPI companion device object is present for this device, it > + * may be accessed during suspend and resume of other devices via I2C > + * operation regions, so tell the PM core and middle layers to avoid > + * skipping system suspend/resume callbacks for it in that case. > + */ The above scenario can also happens for non-acpi companion devices. That makes this comment a bit confusing to me. > + return !has_acpi_companion(dev); I understand it still works by always returning 1 for the non-acpi case, because the PM core deals with it for the direct_complete path. However it looks rather odd, especially due to the comment above. Perhaps returning pm_runtime_suspended() in the other case make this more clear? Or perhaps clarifying the comment somehow? :-) > } > > static void dw_i2c_plat_complete(struct device *dev) > > Kind regards Uffe
Index: linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c =================================================================== --- linux-pm.orig/drivers/i2c/busses/i2c-designware-platdrv.c +++ linux-pm/drivers/i2c/busses/i2c-designware-platdrv.c @@ -370,6 +370,8 @@ static int dw_i2c_plat_probe(struct plat ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev)); adap->dev.of_node = pdev->dev.of_node; + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_SMART_PREPARE); + /* The code below assumes runtime PM to be disabled. */ WARN_ON(pm_runtime_enabled(&pdev->dev)); @@ -433,7 +435,13 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match) #ifdef CONFIG_PM_SLEEP static int dw_i2c_plat_prepare(struct device *dev) { - return pm_runtime_suspended(dev); + /* + * If the ACPI companion device object is present for this device, it + * may be accessed during suspend and resume of other devices via I2C + * operation regions, so tell the PM core and middle layers to avoid + * skipping system suspend/resume callbacks for it in that case. + */ + return !has_acpi_companion(dev); } static void dw_i2c_plat_complete(struct device *dev)