Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH v2 7/7] Revert "dmaengine: dw: platform: provide platform data for Intel"

50 views
Skip to first unread message

Andy Shevchenko

unread,
Nov 26, 2015, 10:30:10 AM11/26/15
to
Since we have a work around to prevent a system hangup we don't need to provide
a platform data explicitly anymore.

This reverts commit 175267b389f781748e2bbb6c737e76b5c9bc4c88.

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
drivers/dma/dw/platform.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index d0734e9..127093a 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -155,7 +155,6 @@ static int dw_probe(struct platform_device *pdev)
struct dw_dma_chip *chip;
struct device *dev = &pdev->dev;
struct resource *mem;
- const struct acpi_device_id *id;
struct dw_dma_platform_data *pdata;
int err;

@@ -179,11 +178,6 @@ static int dw_probe(struct platform_device *pdev)
pdata = dev_get_platdata(dev);
if (!pdata)
pdata = dw_dma_parse_dt(pdev);
- if (!pdata && has_acpi_companion(dev)) {
- id = acpi_match_device(dev->driver->acpi_match_table, dev);
- if (id)
- pdata = (struct dw_dma_platform_data *)id->driver_data;
- }

chip->dev = dev;

@@ -264,17 +258,8 @@ MODULE_DEVICE_TABLE(of, dw_dma_of_id_table);
#endif

#ifdef CONFIG_ACPI
-static struct dw_dma_platform_data dw_dma_acpi_pdata = {
- .nr_channels = 8,
- .is_private = true,
- .chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
- .chan_priority = CHAN_PRIORITY_ASCENDING,
- .block_size = 4095,
- .nr_masters = 2,
-};
-
static const struct acpi_device_id dw_dma_acpi_id_table[] = {
- { "INTL9C60", (kernel_ulong_t)&dw_dma_acpi_pdata },
+ { "INTL9C60", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, dw_dma_acpi_id_table);
--
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Andy Shevchenko

unread,
Nov 26, 2015, 10:30:10 AM11/26/15
to
Here is a next generation (previous one is [1]) of the long standing power
issue fix regarding to LPSS on Intel Baytrail and Braswell SoCs, in
particularly ASuS T100TA. There are few bugs already opened on kernel.org's and
RedHat's bugzilla sites.

The series depends on the patch submitted earlier [2].

The patch 1 brings a new notification to handle the case when ->probe() of the
driver fails. It allows to avoid a potential resource leak. I've noticed couple
of drivers that are using that in assumption that ->probe() never fails.

The patch 2 is needed to fix an I2C issue which Jarkko is currently
investigating.

It seems the best way to push it through linux-pm tree. Thus, it would be good
to get ACKs from the rest of maintainers.

Rafael, it would be nice to have an immutable branch or tag for this sice I
have more patches coming for dw_dmac driver which are based on top of this
series.

The patches have been tested on ASuS T100TA, Intel Cherrytrail, and Intel
Braswell SoCs.

[1] http://www.spinics.net/lists/linux-acpi/msg53963.html
[2] http://www.spinics.net/lists/kernel/msg2119229.html

Andy Shevchenko (7):
device core: add BUS_NOTIFY_BIND_DRIVER_ERROR notification
ACPI / LPSS: allow to use specific PM domain during ->probe()
ACPI / LPSS: do delay for all LPSS devices when D3->D0
ACPI / LPSS: override power state for LPSS DMA device
dmaengine: dw: platform: power on device on shutdown
dmaengine: dw: return immediately from IRQ when DMA isn't in use
Revert "dmaengine: dw: platform: provide platform data for Intel"

arch/x86/Kconfig | 3 +-
arch/x86/include/asm/iosf_mbi.h | 2 +
drivers/acpi/acpi_lpss.c | 184 ++++++++++++++++++++++++++++++++++++----
drivers/base/dd.c | 8 +-
drivers/dma/dw/core.c | 9 +-
drivers/dma/dw/platform.c | 29 +++----
include/linux/device.h | 1 +
7 files changed, 198 insertions(+), 38 deletions(-)

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:07 PM12/4/15
to
The specific power domain can't be used in a way provided by the commit
01ac170ba29a, i.e. pointer to platform device is a subject to change during
unbound / bind cycle.

This reverts commit 01ac170ba29a9903ee590e1ef2d8e6b27b49a16c.

Fixes: 3df2da968744 (Revert "ACPI / LPSS: introduce a 'proxy' device to power on LPSS for DMA")
Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
drivers/acpi/acpi_lpss.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index f9e0d09..da0e276 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -705,8 +705,13 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
}

switch (action) {
- case BUS_NOTIFY_ADD_DEVICE:
+ case BUS_NOTIFY_BOUND_DRIVER:
pdev->dev.pm_domain = &acpi_lpss_pm_domain;
+ break;
+ case BUS_NOTIFY_UNBOUND_DRIVER:
+ pdev->dev.pm_domain = NULL;
+ break;
+ case BUS_NOTIFY_ADD_DEVICE:
if (pdata->dev_desc->flags & LPSS_LTR)
return sysfs_create_group(&pdev->dev.kobj,
&lpss_attr_group);
@@ -714,7 +719,6 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
case BUS_NOTIFY_DEL_DEVICE:
if (pdata->dev_desc->flags & LPSS_LTR)
sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
- pdev->dev.pm_domain = NULL;
break;
default:
break;

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:07 PM12/4/15
to
Since we have a work around to prevent a system hangup we don't need to provide
a platform data explicitly anymore.

This reverts commit 175267b389f781748e2bbb6c737e76b5c9bc4c88.

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:07 PM12/4/15
to
Here is a v3 of the next generation (previous one is [1]) of the long standing
power issue fix regarding to LPSS on Intel Baytrail and Braswell SoCs, in
particularly ASuS T100TA. There are few bugs already opened on kernel.org's and
RedHat's bugzilla sites.

The series depends on the patch submitted earlier [2].

The patch 1 brings a new notification to handle the case when ->probe() of the
driver fails. It allows to avoid a potential problems. I've noticed couple of
drivers that are using that in assumption that ->probe() never fails.

The patches 2 & 4 are needed to fix an I2C issue which Jarkko is currently
investigating.

It seems the best way to push it through linux-pm tree. Thus, it would be good
to get ACKs from the rest of maintainers.

Rafael, it would be nice to have an immutable branch or tag for this sice I
have more patches coming for dw_dmac driver which are based on top of this
series.

The patches have been tested on ASuS T100TA, Intel Cherrytrail, and Intel
Braswell SoCs.

[1] http://www.spinics.net/lists/linux-acpi/msg53963.html
[2] http://www.spinics.net/lists/kernel/msg2119229.html

Changelog v3:
- patch 2 is split to pure revert with Fixes tag for stable and new change in
patch 3
- add patch 4 to resolve an issue when I2C can't be probed and SDHCI leaves
devices in D0
- address comments from Rafael
- quirk functions moved under CONFIG_PM

Andy Shevchenko (9):
device core: add BUS_NOTIFY_DRIVER_NOT_BOUND notification
Revert "ACPI / LPSS: allow to use specific PM domain during ->probe()"
ACPI / LPSS: allow to use specific PM domain during ->probe()
ACPI / LPSS: power on when probe() and otherwise when remove()
ACPI / LPSS: do delay for all LPSS devices when D3->D0
ACPI / LPSS: override power state for LPSS DMA device
dmaengine: dw: platform: power on device on shutdown
dmaengine: dw: return immediately from IRQ when DMA isn't in use
Revert "dmaengine: dw: platform: provide platform data for Intel"

arch/x86/Kconfig | 3 +-
arch/x86/include/asm/iosf_mbi.h | 2 +
drivers/acpi/acpi_lpss.c | 213 +++++++++++++++++++++++++++++++++++++---
drivers/base/dd.c | 10 +-
drivers/dma/dw/core.c | 9 +-
drivers/dma/dw/platform.c | 29 +++---
include/linux/device.h | 1 +
7 files changed, 231 insertions(+), 36 deletions(-)

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:07 PM12/4/15
to
This is a third approach to workaround long standing issue with LPSS on
BayTrail. First one [1] was reverted since it didn't resolve the issue
comprehensively. Second one [2] was rejected by internal review.

The LPSS DMA controller does not have neither _PS0 nor _PS3 method. Moreover it
can be powered off automatically whenever the last LPSS device goes down. In
case of no power any access to the DMA controller will hang the system. The
behaviour is reproduced on some HP laptops based on Intel BayTrail [3,4] as
well as on ASuS T100TA transformer.

Power on the LPSS island through the registers accessible in a specific way.

[1] http://www.spinics.net/lists/linux-acpi/msg53963.html
[2] https://bugzilla.redhat.com/attachment.cgi?id=1066779&action=diff
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1184273
[4] http://www.spinics.net/lists/dmaengine/msg01514.html

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
arch/x86/Kconfig | 3 +-
arch/x86/include/asm/iosf_mbi.h | 2 +
drivers/acpi/acpi_lpss.c | 153 ++++++++++++++++++++++++++++++++++++++--
3 files changed, 150 insertions(+), 8 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 324ac56..7fab0b9 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -535,9 +535,10 @@ config X86_INTEL_QUARK

config X86_INTEL_LPSS
bool "Intel Low Power Subsystem Support"
- depends on ACPI
+ depends on X86 && ACPI
select COMMON_CLK
select PINCTRL
+ select IOSF_MBI
---help---
Select to build support for Intel Low Power Subsystem such as
found on Intel Lynxpoint PCH. Selecting this option enables
diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
index cdc5f63..b41ee16 100644
--- a/arch/x86/include/asm/iosf_mbi.h
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -19,6 +19,8 @@
/* IOSF SB read/write opcodes */
#define MBI_MMIO_READ 0x00
#define MBI_MMIO_WRITE 0x01
+#define MBI_CFG_READ 0x04
+#define MBI_CFG_WRITE 0x05
#define MBI_CR_READ 0x06
#define MBI_CR_WRITE 0x07
#define MBI_REG_READ 0x10
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index a10c2d6..630fb62 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -15,11 +15,16 @@
#include <linux/clk-provider.h>
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/platform_data/clk-lpss.h>
#include <linux/pm_runtime.h>
#include <linux/delay.h>

+#include <asm/cpu_device_id.h>
+#include <asm/iosf_mbi.h>
+#include <asm/pmc_atom.h>
+
#include "internal.h"

ACPI_MODULE_NAME("acpi_lpss");
@@ -71,7 +76,7 @@ struct lpss_device_desc {
void (*setup)(struct lpss_private_data *pdata);
};

-static struct lpss_device_desc lpss_dma_desc = {
+static const struct lpss_device_desc lpss_dma_desc = {
.flags = LPSS_CLK,
};

@@ -84,6 +89,23 @@ struct lpss_private_data {
u32 prv_reg_ctx[LPSS_PRV_REG_COUNT];
};

+/* LPSS run time quirks */
+static unsigned int lpss_quirks;
+
+/*
+ * LPSS_QUIRK_ALWAYS_POWER_ON: override power state for LPSS DMA device.
+ *
+ * The LPSS DMA controller does not have neither _PS0 nor _PS3 method. Moreover
+ * it can be powered off automatically whenever the last LPSS device goes down.
+ * In case of no power any access to the DMA controller will hang the system.
+ * The behaviour is reproduced on some HP laptops based on Intel BayTrail as
+ * well as on ASuS T100TA transformer.
+ *
+ * This quirk overrides power state of entire LPSS island to keep DMA powered
+ * on whenever we have at least one other device in use.
+ */
+#define LPSS_QUIRK_ALWAYS_POWER_ON BIT(0)
+
/* UART Component Parameter Register */
#define LPSS_UART_CPR 0xF4
#define LPSS_UART_CPR_AFCE BIT(4)
@@ -196,13 +218,21 @@ static const struct lpss_device_desc bsw_i2c_dev_desc = {
.setup = byt_i2c_setup,
};

-static struct lpss_device_desc bsw_spi_dev_desc = {
+static const struct lpss_device_desc bsw_spi_dev_desc = {
.flags = LPSS_CLK | LPSS_CLK_GATE | LPSS_CLK_DIVIDER | LPSS_SAVE_CTX
| LPSS_NO_D3_DELAY,
.prv_offset = 0x400,
.setup = lpss_deassert_reset,
};

+#define ICPU(model) { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id lpss_cpu_ids[] = {
+ ICPU(0x37), /* Valleyview, Bay Trail */
+ ICPU(0x4c), /* Braswell, Cherry Trail */
+ {}
+};
+
#else

#define LPSS_ADDR(desc) (0UL)
@@ -661,6 +691,89 @@ static int acpi_lpss_resume_early(struct device *dev)
}
#endif /* CONFIG_PM_SLEEP */

+/* IOSF SB for LPSS island */
+#define LPSS_IOSF_UNIT_LPIOEP 0xA0
+#define LPSS_IOSF_UNIT_LPIO1 0xAB
+#define LPSS_IOSF_UNIT_LPIO2 0xAC
+
+#define LPSS_IOSF_PMCSR 0x84
+#define LPSS_PMCSR_D0 0
+#define LPSS_PMCSR_D3hot 3
+#define LPSS_PMCSR_Dx_MASK GENMASK(1, 0)
+
+#define LPSS_IOSF_GPIODEF0 0x154
+#define LPSS_GPIODEF0_DMA1_D3 BIT(2)
+#define LPSS_GPIODEF0_DMA2_D3 BIT(3)
+#define LPSS_GPIODEF0_DMA_D3_MASK GENMASK(3, 2)
+
+static DEFINE_MUTEX(lpss_iosf_mutex);
+
+static void lpss_iosf_enter_d3_state(void)
+{
+ u32 value1 = 0;
+ u32 mask1 = LPSS_GPIODEF0_DMA_D3_MASK;
+ u32 value2 = LPSS_PMCSR_D3hot;
+ u32 mask2 = LPSS_PMCSR_Dx_MASK;
+ /*
+ * PMC provides an information about actual status of the LPSS devices.
+ * Here we read the values related to LPSS power island, i.e. LPSS
+ * devices, excluding both LPSS DMA controllers, along with SCC domain.
+ */
+ u32 func_dis, d3_sts_0, pmc_status, pmc_mask = 0xfe000ffe;
+ int ret;
+
+ ret = pmc_atom_read(PMC_FUNC_DIS, &func_dis);
+ if (ret)
+ return;
+
+ mutex_lock(&lpss_iosf_mutex);
+
+ ret = pmc_atom_read(PMC_D3_STS_0, &d3_sts_0);
+ if (ret)
+ goto exit;
+
+ /*
+ * Get the status of entire LPSS power island per device basis.
+ * Shutdown both LPSS DMA controllers if and only if all other devices
+ * are already in D3hot.
+ */
+ pmc_status = (~(d3_sts_0 | func_dis)) & pmc_mask;
+ if (pmc_status)
+ goto exit;
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE,
+ LPSS_IOSF_PMCSR, value2, mask2);
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO2, MBI_CFG_WRITE,
+ LPSS_IOSF_PMCSR, value2, mask2);
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
+ LPSS_IOSF_GPIODEF0, value1, mask1);
+exit:
+ mutex_unlock(&lpss_iosf_mutex);
+}
+
+static void lpss_iosf_exit_d3_state(void)
+{
+ u32 value1 = LPSS_GPIODEF0_DMA1_D3 | LPSS_GPIODEF0_DMA2_D3;
+ u32 mask1 = LPSS_GPIODEF0_DMA_D3_MASK;
+ u32 value2 = LPSS_PMCSR_D0;
+ u32 mask2 = LPSS_PMCSR_Dx_MASK;
+
+ mutex_lock(&lpss_iosf_mutex);
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE,
+ LPSS_IOSF_GPIODEF0, value1, mask1);
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO2, MBI_CFG_WRITE,
+ LPSS_IOSF_PMCSR, value2, mask2);
+
+ iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE,
+ LPSS_IOSF_PMCSR, value2, mask2);
+
+ mutex_unlock(&lpss_iosf_mutex);
+}
+
static int acpi_lpss_runtime_suspend(struct device *dev)
{
struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
@@ -673,7 +786,17 @@ static int acpi_lpss_runtime_suspend(struct device *dev)
if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
acpi_lpss_save_ctx(dev, pdata);

- return acpi_dev_runtime_suspend(dev);
+ ret = acpi_dev_runtime_suspend(dev);
+
+ /*
+ * This call must be last in the sequence, otherwise PMC will return
+ * wrong status for devices being about to be powered off. See
+ * lpss_iosf_enter_d3_state() for further information.
+ */
+ if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
+ lpss_iosf_enter_d3_state();
+
+ return ret;
}

static int acpi_lpss_runtime_resume(struct device *dev)
@@ -681,6 +804,13 @@ static int acpi_lpss_runtime_resume(struct device *dev)
struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
int ret;

+ /*
+ * This call is kept first to be in symmetry with
+ * acpi_lpss_runtime_suspend() one.
+ */
+ if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available())
+ lpss_iosf_exit_d3_state();
+
ret = acpi_dev_runtime_resume(dev);
if (ret)
return ret;
@@ -798,10 +928,19 @@ static struct acpi_scan_handler lpss_handler = {

void __init acpi_lpss_init(void)
{
- if (!lpt_clk_init()) {
- bus_register_notifier(&platform_bus_type, &acpi_lpss_nb);
- acpi_scan_add_handler(&lpss_handler);
- }
+ const struct x86_cpu_id *id;
+ int ret;
+
+ ret = lpt_clk_init();
+ if (ret)
+ return;
+
+ id = x86_match_cpu(lpss_cpu_ids);
+ if (id)
+ lpss_quirks |= LPSS_QUIRK_ALWAYS_POWER_ON;
+
+ bus_register_notifier(&platform_bus_type, &acpi_lpss_nb);
+ acpi_scan_add_handler(&lpss_handler);
}

#else

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:34 PM12/4/15
to
When LPSS drivers are compiled as a module, which is usually the case, the
second probe of that driver may fail because the driver is written in an
assumption that device is powered on. That is not the case for all drivers.
Moreover we would like not drain power in vain.

Implement ->activate() and ->dismiss() callbacks in the ACPI LPSS custom power
domain.

-------- 8< -------- 8< -------- 8< -------- 8< -------- 8< --------

Case 1: The I2C probe() repeat.

/sys/bus/platform/devices/808622C1:00 \_SB_.PCI0.I2C1 [D3hot]
/sys/bus/platform/devices/808622C1:01 \_SB_.PCI0.I2C2 [D3hot]
/sys/bus/platform/devices/808622C1:02 \_SB_.PCI0.I2C3 [D3hot]
/sys/bus/platform/devices/808622C1:03 \_SB_.PCI0.I2C4 [D3hot]
/sys/bus/platform/devices/808622C1:05 \_SB_.PCI0.I2C6 [D3hot]
/sys/bus/platform/devices/808622C1:06 \_SB_.PCI0.I2C7 [D3hot]

% modprobe i2c-designware-platform
i2c_designware 808622C1:00: Unknown Synopsys component type: 0xffffffff
i2c_designware 808622C1:01: Unknown Synopsys component type: 0xffffffff
i2c_designware 808622C1:02: Unknown Synopsys component type: 0xffffffff
i2c_designware 808622C1:03: Unknown Synopsys component type: 0xffffffff
i2c_designware 808622C1:05: Unknown Synopsys component type: 0xffffffff
i2c_designware 808622C1:06: Unknown Synopsys component type: 0xffffffff

Case 2: The power drain in case of SDHCI.

/sys/bus/platform/devices/80860F14:00 \_SB_.PCI0.SDHA [D3hot]
/sys/bus/platform/devices/80860F14:01 \_SB_.PCI0.SDHC [D3hot]

% modprobe -r sdhci-acpi
mmc0: card 0001 removed

/sys/bus/platform/devices/80860F14:00 \_SB_.PCI0.SDHA [D0]
/sys/bus/platform/devices/80860F14:01 \_SB_.PCI0.SDHC [D0]

-------- 8< -------- 8< -------- 8< -------- 8< -------- 8< --------

Patch fixes above problems.

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
drivers/acpi/acpi_lpss.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 6263939..c5f12f3 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -596,6 +596,34 @@ static void acpi_lpss_restore_ctx(struct device *dev,
}
}

+static int acpi_lpss_activate(struct device *dev)
+{
+ struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev));
+ int ret;
+
+ ret = acpi_dev_runtime_resume(dev);
+ if (ret)
+ return ret;
+
+ acpi_lpss_d3_to_d0_delay(pdata);
+
+ /*
+ * This is called only on ->probe() stage where a device is either in
+ * known state defined by BIOS or most likely powered off. Due to this
+ * we have to deassert reset line to be sure that ->probe() will
+ * recognize the device.
+ */
+ if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
+ lpss_deassert_reset(pdata);
+
+ return 0;
+}
+
+static void acpi_lpss_dismiss(struct device *dev)
+{
+ acpi_dev_runtime_suspend(dev);
+}
+
#ifdef CONFIG_PM_SLEEP
static int acpi_lpss_suspend_late(struct device *dev)
{
@@ -660,6 +688,10 @@ static int acpi_lpss_runtime_resume(struct device *dev)
#endif /* CONFIG_PM */

static struct dev_pm_domain acpi_lpss_pm_domain = {
+#ifdef CONFIG_PM
+ .activate = acpi_lpss_activate,
+ .dismiss = acpi_lpss_dismiss,
+#endif
.ops = {
#ifdef CONFIG_PM
#ifdef CONFIG_PM_SLEEP

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:37 PM12/4/15
to
We have to call dw_dma_disable() to stop any ongoing transfer. On some
platforms we can't do that since DMA device is powered off. Moreover we have no
possibility at that point to check if the platform is affected or not. That's
why we call pm_runtime_get_sync() / pm_runtime_put() unconditionally. On the
other hand we can't use pm_runtime_suspended() because runtime PM framework is
not fully used by the driver.

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
drivers/dma/dw/platform.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
index 68a4815..d0734e9 100644
--- a/drivers/dma/dw/platform.c
+++ b/drivers/dma/dw/platform.c
@@ -239,7 +239,19 @@ static void dw_shutdown(struct platform_device *pdev)
{
struct dw_dma_chip *chip = platform_get_drvdata(pdev);

+ /*
+ * We have to call dw_dma_disable() to stop any ongoing transfer. On
+ * some platforms we can't do that since DMA device is powered off.
+ * Moreover we have no possibility to check if the platform is affected
+ * or not. That's why we call pm_runtime_get_sync() / pm_runtime_put()
+ * unconditionally. On the other hand we can't use
+ * pm_runtime_suspended() because runtime PM framework is not fully
+ * used by the driver.
+ */
+ pm_runtime_get_sync(chip->dev);
dw_dma_disable(chip);
+ pm_runtime_put_sync_suspend(chip->dev);
+
clk_disable_unprepare(chip->clk);

Andy Shevchenko

unread,
Dec 4, 2015, 5:00:39 PM12/4/15
to
The LPSS DMA device has no context to save, though it requires the same delay
like the rest of LPSS devices when power state is changed from D3 to D0.

Do delay for the DMA device as well.

Signed-off-by: Andy Shevchenko <andriy.s...@linux.intel.com>
---
drivers/acpi/acpi_lpss.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index c5f12f3..a10c2d6 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -574,6 +574,17 @@ static void acpi_lpss_restore_ctx(struct device *dev,
{
unsigned int i;

+ for (i = 0; i < LPSS_PRV_REG_COUNT; i++) {
+ unsigned long offset = i * sizeof(u32);
+
+ __lpss_reg_write(pdata->prv_reg_ctx[i], pdata, offset);
+ dev_dbg(dev, "restoring 0x%08x to LPSS reg at offset 0x%02lx\n",
+ pdata->prv_reg_ctx[i], offset);
+ }
+}
+
+static void acpi_lpss_d3_to_d0_delay(struct lpss_private_data *pdata)
+{
/*
* The following delay is needed or the subsequent write operations may
* fail. The LPSS devices are actually PCI devices and the PCI spec
@@ -586,14 +597,6 @@ static void acpi_lpss_restore_ctx(struct device *dev,
delay = 0;

msleep(delay);
-
- for (i = 0; i < LPSS_PRV_REG_COUNT; i++) {
- unsigned long offset = i * sizeof(u32);
-
- __lpss_reg_write(pdata->prv_reg_ctx[i], pdata, offset);
- dev_dbg(dev, "restoring 0x%08x to LPSS reg at offset 0x%02lx\n",
- pdata->prv_reg_ctx[i], offset);
- }
}

static int acpi_lpss_activate(struct device *dev)
@@ -649,6 +652,8 @@ static int acpi_lpss_resume_early(struct device *dev)
if (ret)
return ret;

+ acpi_lpss_d3_to_d0_delay(pdata);
+
if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
acpi_lpss_restore_ctx(dev, pdata);

@@ -680,6 +685,8 @@ static int acpi_lpss_runtime_resume(struct device *dev)
if (ret)
return ret;

+ acpi_lpss_d3_to_d0_delay(pdata);
+
if (pdata->dev_desc->flags & LPSS_SAVE_CTX)
acpi_lpss_restore_ctx(dev, pdata);

Rafael J. Wysocki

unread,
Dec 4, 2015, 5:40:06 PM12/4/15
to
The series generally looks good to me, but patch [1/9] needs an ACK from Greg
and the dmaengine ones need ACKs from Vinod.

Thanks,
Rafael

Andy Shevchenko

unread,
Dec 4, 2015, 6:20:06 PM12/4/15
to
Thank you!
Greg, Vinod, what is your opinion?

Just noticed we have to exchange patches 4 and 5 (5 should go before 4
due to bisectability).

Also I would like to ask you to comment and maybe Ack (seems we have
no powercap maintainers) the mentioned patch for dependency [2].

>> [2] http://www.spinics.net/lists/kernel/msg2119229.html

--
With Best Regards,
Andy Shevchenko

Rafael J. Wysocki

unread,
Dec 4, 2015, 7:10:12 PM12/4/15
to
Hi,

On Sat, Dec 5, 2015 at 12:15 AM, Andy Shevchenko
<andy.sh...@gmail.com> wrote:
> On Sat, Dec 5, 2015 at 1:07 AM, Rafael J. Wysocki <r...@rjwysocki.net> wrote:
>> On Friday, December 04, 2015 11:49:16 PM Andy Shevchenko wrote:

[cut]

>>
>> The series generally looks good to me, but patch [1/9] needs an ACK from Greg
>> and the dmaengine ones need ACKs from Vinod.
>
> Thank you!
> Greg, Vinod, what is your opinion?
>
> Just noticed we have to exchange patches 4 and 5 (5 should go before 4
> due to bisectability).
>
> Also I would like to ask you to comment and maybe Ack (seems we have
> no powercap maintainers) the mentioned patch for dependency [2].
>
>>> [2] http://www.spinics.net/lists/kernel/msg2119229.html

I'm the top-level powercap maintainer, but the RAPL driver is
maintained by Jacob. Please ask him for an ACK.

Thanks,
Rafael

Jacob Pan

unread,
Dec 7, 2015, 11:40:07 PM12/7/15
to
On Sat, 5 Dec 2015 01:07:47 +0100
"Rafael J. Wysocki" <raf...@kernel.org> wrote:

> >>> [2] http://www.spinics.net/lists/kernel/msg2119229.html
>
> I'm the top-level powercap maintainer, but the RAPL driver is
> maintained by Jacob. Please ask him for an ACK.

looks good to me. no functional change to the driver.

Vinod Koul

unread,
Dec 8, 2015, 12:30:08 PM12/8/15
to
On Fri, Dec 04, 2015 at 11:49:23PM +0200, Andy Shevchenko wrote:
> We have to call dw_dma_disable() to stop any ongoing transfer. On some
> platforms we can't do that since DMA device is powered off. Moreover we have no
> possibility at that point to check if the platform is affected or not. That's
> why we call pm_runtime_get_sync() / pm_runtime_put() unconditionally. On the
> other hand we can't use pm_runtime_suspended() because runtime PM framework is
> not fully used by the driver.

Acked-by: Vinod Koul <vinod...@intel.com>

--
~Vinod

Vinod Koul

unread,
Dec 8, 2015, 12:30:08 PM12/8/15
to
On Fri, Dec 04, 2015 at 11:49:25PM +0200, Andy Shevchenko wrote:
> Since we have a work around to prevent a system hangup we don't need to provide
> a platform data explicitly anymore.

Acked-by: Vinod Koul <vinod...@intel.com>

--
~Vinod
0 new messages