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)?
---
drivers/acpi/acpi_lpss.c | 103 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 90 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,20 @@ 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 current_proxy_count;
+static LIST_HEAD(proxy_list);
+static DEFINE_MUTEX(proxy_list_lock);
+
struct lpss_private_data {
void __iomem *mmio_base;
resource_size_t mmio_size;
@@ -85,6 +93,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 +382,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 +597,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_list_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) {
+ current_proxy_count--;
+ ret = pm_runtime_put_sync_suspend(current_proxy->dev);
+ }
- return 0;
+ out:
+ if (use_proxy)
+ mutex_unlock(&proxy_list_lock);
+
+ return ret;
}
static int acpi_lpss_runtime_resume(struct device *dev)
@@ -615,8 +639,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_list_lock);
+ if (current_proxy) {
+ ret = pm_runtime_get_sync(current_proxy->dev);
+ if (!ret)
+ current_proxy_count++;
+ } else {
+ ret = -EBUSY;
+ }
+ mutex_unlock(&proxy_list_lock);
if (ret)
return ret;
}
@@ -652,6 +684,28 @@ static struct dev_pm_domain acpi_lpss_pm
},
};
+static void acpi_lpss_switch_proxy(void)
+{
+ struct lpss_private_data *lpd;
+ unsigned int count;
+ struct device *dev;
+
+ if (list_empty(&proxy_list)) {
+ current_proxy = NULL;
+ WARN_ON(current_proxy_count);
+ return;
+ }
+ count = current_proxy_count;
+ dev = current_proxy->dev;
+ lpd = list_first_entry(&proxy_list, struct lpss_private_data,
+ proxy_entry.node);
+ current_proxy = &lpd->proxy_entry;
+ while (count--) {
+ pm_runtime_put_noidle(dev);
+ pm_runtime_get_sync(current_proxy->dev);
+ }
+}
+
static int acpi_lpss_platform_notify(struct notifier_block *nb,
unsigned long action, void *data)
{
@@ -689,6 +743,29 @@ static int acpi_lpss_platform_notify(str
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_list_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)
+ current_proxy = &pdata->proxy_entry;
+
+ mutex_unlock(&proxy_list_lock);
+ }
+ break;
+ case BUS_NOTIFY_UNBIND_DRIVER:
+ if (pdata->dev_desc->flags & LPSS_DEV_PROXY) {
+ mutex_lock(&proxy_list_lock);
+
+ list_del(&pdata->proxy_entry.node);
+ acpi_lpss_switch_proxy();
+
+ mutex_unlock(&proxy_list_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
