On Wednesday, January 21, 2015 03:37:43 AM Rafael J. Wysocki wrote:
> On Tuesday, January 20, 2015 03:45:36 PM Rafael J. Wysocki wrote:
> > On Thursday, January 15, 2015 01:10:44 PM Andy Shevchenko wrote:
> > > We can't rely on the first enumerated device since it might be not probed yet
> > > when DMA driver wants to resume the device. This patch changes the place where
> > > we assign the 'proxy' device. From now on the first resumed device will be
> > > recognized as a 'proxy' because it is probed.
> >
> > I was about to apply this, but I realized that we couldn't do it this way. ->
> >
> > > Reported-by: Jerome Blin <jerome.blin@xxxxxxxxx>
> > > Fixes: 6c17ee44d524 (ACPI / LPSS: introduce a 'proxy' device to power on LPSS for DMA)
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/acpi/acpi_lpss.c | 17 +++++++++++------
> > > 1 file changed, 11 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> > > index 4f3febf..d851290 100644
> > > --- a/drivers/acpi/acpi_lpss.c
> > > +++ b/drivers/acpi/acpi_lpss.c
> > > @@ -374,8 +374,6 @@ static int acpi_lpss_create_device(struct acpi_device *adev,
> > > adev->driver_data = pdata;
> > > pdev = acpi_create_platform_device(adev);
> > > if (!IS_ERR_OR_NULL(pdev)) {
> > > - if (!proxy_device && dev_desc->flags & LPSS_DEV_PROXY)
> > > - proxy_device = &pdev->dev;
> > > return 1;
> > > }
> > >
> > > @@ -615,10 +613,17 @@ static int acpi_lpss_runtime_resume(struct device *dev)
> > > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
> > > int ret;
> > >
> > > - if (pdata->dev_desc->flags & LPSS_PROXY_REQ && proxy_device) {
> > > - ret = pm_runtime_get_sync(proxy_device);
> > > - if (ret)
> > > - return ret;
> > > + if (!proxy_device && pdata->dev_desc->flags & LPSS_DEV_PROXY)
> > > + proxy_device = dev;
> >
> > -> OK, so what if the driver of proxy_device is removed later?
> >
> > > +
> > > + if (pdata->dev_desc->flags & LPSS_PROXY_REQ) {
> > > + if (proxy_device) {
> > > + ret = pm_runtime_get_sync(proxy_device);
> > > + if (ret)
> > > + return ret;
> > > + } else {
> > > + return -EPROBE_DEFER;
> >
> > The error code returned here will disable runtime PM for dev forever,
> > even if proxy_device becomes available at one point. You could return
> > -EBUSY instead, but why don't we add BUS_NOTIFY_BOUND_DRIVER and
> > BUS_NOTIFY_UNBIND_DRIVER handling to acpi_lpss_platform_notify() and
> > set/unset proxy_device from there?
>
> Well, that's more complicated, because we need to be careful when we switch
> proxies while the old one has been reference counted.
>
> Maybe something like this (untested)?
That sill had a few problems.
The one below should be better (still untested, though). It assumes that
the devices needing a proxy will never start as runtime-suspended and will
always be resumed before removal.
---
drivers/acpi/acpi_lpss.c | 126 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 113 insertions(+), 13 deletions(-)
Index: linux-pm/drivers/acpi/acpi_lpss.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_lpss.c
+++ linux-pm/drivers/acpi/acpi_lpss.c
@@ -72,12 +72,21 @@ struct lpss_device_desc {
void (*setup)(struct lpss_private_data *pdata);
};
-static struct device *proxy_device;
-
static struct lpss_device_desc lpss_dma_desc = {
.flags = LPSS_CLK | LPSS_PROXY_REQ,
};
+struct lpss_proxy {
+ struct device *dev;
+ struct list_head node;
+};
+
+static struct lpss_proxy *current_proxy;
+static unsigned int proxy_count;
+static unsigned int proxy_suspend_count;
+static LIST_HEAD(proxy_list);
+static DEFINE_MUTEX(proxy_lock);
+
struct lpss_private_data {
void __iomem *mmio_base;
resource_size_t mmio_size;
@@ -85,6 +94,7 @@ struct lpss_private_data {
struct clk *clk;
const struct lpss_device_desc *dev_desc;
u32 prv_reg_ctx[LPSS_PRV_REG_COUNT];
+ struct lpss_proxy proxy_entry;
};
/* UART Component Parameter Register */
@@ -373,11 +383,8 @@ static int acpi_lpss_create_device(struc
adev->driver_data = pdata;
pdev = acpi_create_platform_device(adev);
- if (!IS_ERR_OR_NULL(pdev)) {
- if (!proxy_device && dev_desc->flags & LPSS_DEV_PROXY)
- proxy_device = &pdev->dev;
+ if (!IS_ERR_OR_NULL(pdev))
return 1;
- }
ret = PTR_ERR(pdev);
adev->driver_data = NULL;
@@ -591,23 +598,41 @@ static int acpi_lpss_resume_early(struct
static int acpi_lpss_runtime_suspend(struct device *dev)
{
struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+ bool use_proxy;
int ret;
+ if (pdata->dev_desc->flags & LPSS_PROXY_REQ) {
+ use_proxy = true;
+ mutex_lock(&proxy_lock);
+ if (!current_proxy) {
+ ret = -EBUSY;
+ goto out;
+ }
+ } else {
+ use_proxy = false;
+ }
+
ret = pm_generic_runtime_suspend(dev);
if (ret)
- return ret;
+ goto out;
if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
acpi_lpss_save_ctx(dev, pdata);
ret = acpi_dev_runtime_suspend(dev);
if (ret)
- return ret;
+ goto out;
- if (pdata->dev_desc->flags & LPSS_PROXY_REQ && proxy_device)
- return pm_runtime_put_sync_suspend(proxy_device);
+ if (use_proxy) {
+ proxy_suspend_count++;
+ ret = pm_runtime_put_sync_suspend(current_proxy->dev);
+ }
- return 0;
+ out:
+ if (use_proxy)
+ mutex_unlock(&proxy_lock);
+
+ return ret;
}
static int acpi_lpss_runtime_resume(struct device *dev)
@@ -615,8 +640,16 @@ static int acpi_lpss_runtime_resume(stru
struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
int ret;
- if (pdata->dev_desc->flags & LPSS_PROXY_REQ && proxy_device) {
- ret = pm_runtime_get_sync(proxy_device);
+ if (pdata->dev_desc->flags & LPSS_PROXY_REQ) {
+ mutex_lock(&proxy_lock);
+ if (current_proxy) {
+ ret = pm_runtime_get_sync(current_proxy->dev);
+ if (!ret)
+ proxy_suspend_count--;
+ } else {
+ ret = -EBUSY;
+ }
+ mutex_unlock(&proxy_lock);
if (ret)
return ret;
}
@@ -652,6 +685,29 @@ static struct dev_pm_domain acpi_lpss_pm
},
};
+static void acpi_lpss_switch_proxy(void)
+{
+ struct device *dev = current_proxy ? current_proxy->dev : NULL;
+ unsigned int count = proxy_count - proxy_suspend_count;
+
+ if (list_empty(&proxy_list)) {
+ current_proxy = NULL;
+ } else {
+ struct lpss_private_data *lpd;
+
+ lpd = list_first_entry(&proxy_list, struct lpss_private_data,
+ proxy_entry.node);
+ current_proxy = &lpd->proxy_entry;
+ }
+ while (count--) {
+ if (dev)
+ pm_runtime_put_noidle(dev);
+
+ if (current_proxy)
+ pm_runtime_get_sync(current_proxy->dev);
+ }
+}
+
static int acpi_lpss_platform_notify(struct notifier_block *nb,
unsigned long action, void *data)
{
@@ -680,15 +736,59 @@ static int acpi_lpss_platform_notify(str
switch (action) {
case BUS_NOTIFY_ADD_DEVICE:
pdev->dev.pm_domain = &acpi_lpss_pm_domain;
+ if (pdata->dev_desc->flags & LPSS_DEV_PROXY) {
+ mutex_lock(&proxy_lock);
+
+ /* This assimes the device to be powered up. */
+ proxy_count++;
+ if (current_proxy)
+ pm_runtime_get_sync(current_proxy->dev);
+
+ mutex_unlock(&proxy_lock);
+ }
if (pdata->dev_desc->flags & LPSS_LTR)
return sysfs_create_group(&pdev->dev.kobj,
&lpss_attr_group);
break;
case BUS_NOTIFY_DEL_DEVICE:
+ if (pdata->dev_desc->flags & LPSS_DEV_PROXY) {
+ mutex_lock(&proxy_lock);
+
+ /* This assumes that the device is not suspended. */
+ proxy_count--;
+ if (current_proxy)
+ pm_runtime_put(current_proxy->dev);
+
+ mutex_unlock(&proxy_lock);
+ }
if (pdata->dev_desc->flags & LPSS_LTR)
sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
pdev->dev.pm_domain = NULL;
break;
+ case BUS_NOTIFY_BOUND_DRIVER:
+ if (pdata->dev_desc->flags & LPSS_DEV_PROXY) {
+ mutex_lock(&proxy_lock);
+
+ INIT_LIST_HEAD(&pdata->proxy_entry.node);
+ pdata->proxy_entry.dev = &pdev->dev;
+ list_add_tail(&pdata->proxy_entry.node, &proxy_list);
+ if (!current_proxy)
+ acpi_lpss_switch_proxy();
+
+ mutex_unlock(&proxy_lock);
+ }
+ break;
+ case BUS_NOTIFY_UNBIND_DRIVER:
+ if (pdata->dev_desc->flags & LPSS_DEV_PROXY) {
+ mutex_lock(&proxy_lock);
+
+ list_del(&pdata->proxy_entry.node);
+ if (current_proxy == &pdata->proxy_entry)
+ acpi_lpss_switch_proxy();
+
+ mutex_unlock(&proxy_lock);
+ }
+ break;
default:
break;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
