* [PATCH 0/4] ata: ahci_brcmstb: ALPM support
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, linux-arm-kernel, bcm-kernel-feedback-list,
computersforpeace, gregory.0xf0, Florian Fainelli
Hi Tejun,
This patch series contains a bunch of changes to the Broadcom AHCI driver
to support ALPM, disable DIPM (not supported) and updates the core libata
to account for driver-specific ALPM-resume procedure.
The last patch in this series allows some particular drivers, which are known
not to need the debounce delay while resuming SATA links to skip these, which
yields a faster resume time.
This is against your ata/for-next branch, thanks!
Danesh Petigara (4):
ata: ahci_brcmstb: enable support for ALPM
ata: ahci_brcmstb: disable DIPM support
drivers: ata: wake port before DMA stop for ALPM
libata: skip debounce delay on link resume
drivers/ata/ahci.h | 6 ++++++
drivers/ata/ahci_brcmstb.c | 40 ++++++++++++++++++++++++++++++++++-
drivers/ata/libahci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 4 +++-
include/linux/libata.h | 5 +++++
5 files changed, 105 insertions(+), 2 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/4] ata: ahci_brcmstb: ALPM support
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tejun,
This patch series contains a bunch of changes to the Broadcom AHCI driver
to support ALPM, disable DIPM (not supported) and updates the core libata
to account for driver-specific ALPM-resume procedure.
The last patch in this series allows some particular drivers, which are known
not to need the debounce delay while resuming SATA links to skip these, which
yields a faster resume time.
This is against your ata/for-next branch, thanks!
Danesh Petigara (4):
ata: ahci_brcmstb: enable support for ALPM
ata: ahci_brcmstb: disable DIPM support
drivers: ata: wake port before DMA stop for ALPM
libata: skip debounce delay on link resume
drivers/ata/ahci.h | 6 ++++++
drivers/ata/ahci_brcmstb.c | 40 ++++++++++++++++++++++++++++++++++-
drivers/ata/libahci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 4 +++-
include/linux/libata.h | 5 +++++
5 files changed, 105 insertions(+), 2 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM
2016-01-08 0:03 ` Florian Fainelli
(?)
@ 2016-01-08 0:03 ` Florian Fainelli
-1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: Florian Fainelli, linux-kernel, Danesh Petigara,
bcm-kernel-feedback-list, gregory.0xf0, computersforpeace,
linux-arm-kernel
From: Danesh Petigara <dpetigara@broadcom.com>
Enable support for ALPM in the host controller's capabilities
register. Also adjust the PLL timeout to give it enough time
to lock when the port exits slumber mode.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index a4a0940307bc..f38a07ea59f4 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -53,6 +53,10 @@
#define SATA_TOP_CTRL_PHY_OFFS 0x8
#define SATA_TOP_MAX_PHYS 2
+#define SATA_FIRST_PORT_CTRL 0x700
+#define SATA_NEXT_PORT_CTRL_OFFSET 0x80
+#define SATA_PORT_PCTRL6(reg_base) (reg_base + 0x18)
+
/* On big-endian MIPS, buses are reversed to big endian, so switch them back */
#if defined(CONFIG_MIPS) && defined(__BIG_ENDIAN)
#define DATA_ENDIAN 2 /* AHCI->DDR inbound accesses */
@@ -111,6 +115,35 @@ static inline void brcm_sata_writereg(u32 val, void __iomem *addr)
writel_relaxed(val, addr);
}
+static void brcm_sata_alpm_init(struct ahci_host_priv *hpriv)
+{
+ int i;
+ u32 bus_ctrl, port_ctrl, host_caps;
+ struct brcm_ahci_priv *priv = hpriv->plat_data;
+
+ /*Enable support for ALPM */
+ bus_ctrl = brcm_sata_readreg(priv->top_ctrl
+ + SATA_TOP_CTRL_BUS_CTRL);
+ brcm_sata_writereg(bus_ctrl | OVERRIDE_HWINIT,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+ host_caps = readl(hpriv->mmio + HOST_CAP);
+ writel(host_caps | HOST_CAP_ALPM, hpriv->mmio);
+ brcm_sata_writereg(bus_ctrl,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+
+ /*
+ * Adjust timeout to allow PLL sufficient time to lock while waking
+ * up from slumber mode.
+ */
+ for (i = 0, port_ctrl = SATA_FIRST_PORT_CTRL;
+ i < SATA_TOP_MAX_PHYS;
+ i++, port_ctrl += SATA_NEXT_PORT_CTRL_OFFSET) {
+ if (priv->port_mask & BIT(i))
+ writel(0xff1003fc,
+ hpriv->mmio + SATA_PORT_PCTRL6(port_ctrl));
+ }
+}
+
static void brcm_sata_phy_enable(struct brcm_ahci_priv *priv, int port)
{
void __iomem *phyctrl = priv->top_ctrl + SATA_TOP_CTRL_PHY_CTRL +
@@ -240,6 +273,7 @@ static int brcm_ahci_resume(struct device *dev)
brcm_sata_init(priv);
brcm_sata_phys_enable(priv);
+ brcm_sata_alpm_init(hpriv);
return ahci_platform_resume(dev);
}
#endif
@@ -284,6 +318,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
return PTR_ERR(hpriv);
hpriv->plat_data = priv;
+ brcm_sata_alpm_init(hpriv);
+
ret = ahci_platform_enable_resources(hpriv);
if (ret)
return ret;
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, linux-arm-kernel, bcm-kernel-feedback-list,
computersforpeace, gregory.0xf0, Danesh Petigara,
Florian Fainelli
From: Danesh Petigara <dpetigara@broadcom.com>
Enable support for ALPM in the host controller's capabilities
register. Also adjust the PLL timeout to give it enough time
to lock when the port exits slumber mode.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index a4a0940307bc..f38a07ea59f4 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -53,6 +53,10 @@
#define SATA_TOP_CTRL_PHY_OFFS 0x8
#define SATA_TOP_MAX_PHYS 2
+#define SATA_FIRST_PORT_CTRL 0x700
+#define SATA_NEXT_PORT_CTRL_OFFSET 0x80
+#define SATA_PORT_PCTRL6(reg_base) (reg_base + 0x18)
+
/* On big-endian MIPS, buses are reversed to big endian, so switch them back */
#if defined(CONFIG_MIPS) && defined(__BIG_ENDIAN)
#define DATA_ENDIAN 2 /* AHCI->DDR inbound accesses */
@@ -111,6 +115,35 @@ static inline void brcm_sata_writereg(u32 val, void __iomem *addr)
writel_relaxed(val, addr);
}
+static void brcm_sata_alpm_init(struct ahci_host_priv *hpriv)
+{
+ int i;
+ u32 bus_ctrl, port_ctrl, host_caps;
+ struct brcm_ahci_priv *priv = hpriv->plat_data;
+
+ /*Enable support for ALPM */
+ bus_ctrl = brcm_sata_readreg(priv->top_ctrl
+ + SATA_TOP_CTRL_BUS_CTRL);
+ brcm_sata_writereg(bus_ctrl | OVERRIDE_HWINIT,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+ host_caps = readl(hpriv->mmio + HOST_CAP);
+ writel(host_caps | HOST_CAP_ALPM, hpriv->mmio);
+ brcm_sata_writereg(bus_ctrl,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+
+ /*
+ * Adjust timeout to allow PLL sufficient time to lock while waking
+ * up from slumber mode.
+ */
+ for (i = 0, port_ctrl = SATA_FIRST_PORT_CTRL;
+ i < SATA_TOP_MAX_PHYS;
+ i++, port_ctrl += SATA_NEXT_PORT_CTRL_OFFSET) {
+ if (priv->port_mask & BIT(i))
+ writel(0xff1003fc,
+ hpriv->mmio + SATA_PORT_PCTRL6(port_ctrl));
+ }
+}
+
static void brcm_sata_phy_enable(struct brcm_ahci_priv *priv, int port)
{
void __iomem *phyctrl = priv->top_ctrl + SATA_TOP_CTRL_PHY_CTRL +
@@ -240,6 +273,7 @@ static int brcm_ahci_resume(struct device *dev)
brcm_sata_init(priv);
brcm_sata_phys_enable(priv);
+ brcm_sata_alpm_init(hpriv);
return ahci_platform_resume(dev);
}
#endif
@@ -284,6 +318,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
return PTR_ERR(hpriv);
hpriv->plat_data = priv;
+ brcm_sata_alpm_init(hpriv);
+
ret = ahci_platform_enable_resources(hpriv);
if (ret)
return ret;
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Danesh Petigara <dpetigara@broadcom.com>
Enable support for ALPM in the host controller's capabilities
register. Also adjust the PLL timeout to give it enough time
to lock when the port exits slumber mode.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index a4a0940307bc..f38a07ea59f4 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -53,6 +53,10 @@
#define SATA_TOP_CTRL_PHY_OFFS 0x8
#define SATA_TOP_MAX_PHYS 2
+#define SATA_FIRST_PORT_CTRL 0x700
+#define SATA_NEXT_PORT_CTRL_OFFSET 0x80
+#define SATA_PORT_PCTRL6(reg_base) (reg_base + 0x18)
+
/* On big-endian MIPS, buses are reversed to big endian, so switch them back */
#if defined(CONFIG_MIPS) && defined(__BIG_ENDIAN)
#define DATA_ENDIAN 2 /* AHCI->DDR inbound accesses */
@@ -111,6 +115,35 @@ static inline void brcm_sata_writereg(u32 val, void __iomem *addr)
writel_relaxed(val, addr);
}
+static void brcm_sata_alpm_init(struct ahci_host_priv *hpriv)
+{
+ int i;
+ u32 bus_ctrl, port_ctrl, host_caps;
+ struct brcm_ahci_priv *priv = hpriv->plat_data;
+
+ /*Enable support for ALPM */
+ bus_ctrl = brcm_sata_readreg(priv->top_ctrl
+ + SATA_TOP_CTRL_BUS_CTRL);
+ brcm_sata_writereg(bus_ctrl | OVERRIDE_HWINIT,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+ host_caps = readl(hpriv->mmio + HOST_CAP);
+ writel(host_caps | HOST_CAP_ALPM, hpriv->mmio);
+ brcm_sata_writereg(bus_ctrl,
+ priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL);
+
+ /*
+ * Adjust timeout to allow PLL sufficient time to lock while waking
+ * up from slumber mode.
+ */
+ for (i = 0, port_ctrl = SATA_FIRST_PORT_CTRL;
+ i < SATA_TOP_MAX_PHYS;
+ i++, port_ctrl += SATA_NEXT_PORT_CTRL_OFFSET) {
+ if (priv->port_mask & BIT(i))
+ writel(0xff1003fc,
+ hpriv->mmio + SATA_PORT_PCTRL6(port_ctrl));
+ }
+}
+
static void brcm_sata_phy_enable(struct brcm_ahci_priv *priv, int port)
{
void __iomem *phyctrl = priv->top_ctrl + SATA_TOP_CTRL_PHY_CTRL +
@@ -240,6 +273,7 @@ static int brcm_ahci_resume(struct device *dev)
brcm_sata_init(priv);
brcm_sata_phys_enable(priv);
+ brcm_sata_alpm_init(hpriv);
return ahci_platform_resume(dev);
}
#endif
@@ -284,6 +318,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
return PTR_ERR(hpriv);
hpriv->plat_data = priv;
+ brcm_sata_alpm_init(hpriv);
+
ret = ahci_platform_enable_resources(hpriv);
if (ret)
return ret;
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] ata: ahci_brcmstb: disable DIPM support
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 0:03 ` Florian Fainelli
-1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, linux-arm-kernel, bcm-kernel-feedback-list,
computersforpeace, gregory.0xf0, Danesh Petigara,
Florian Fainelli
From: Danesh Petigara <dpetigara@broadcom.com>
The Broadcom STB SATA host controller does not support device
initiated power management. Disable support for this feature
so the driver never sends SETFEATURES commands to the device
to enable/disable DIPM.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index f38a07ea59f4..f51e3d9ba86e 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -84,7 +84,7 @@ struct brcm_ahci_priv {
};
static const struct ata_port_info ahci_brcm_port_info = {
- .flags = AHCI_FLAG_COMMON,
+ .flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] ata: ahci_brcmstb: disable DIPM support
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Danesh Petigara <dpetigara@broadcom.com>
The Broadcom STB SATA host controller does not support device
initiated power management. Disable support for this feature
so the driver never sends SETFEATURES commands to the device
to enable/disable DIPM.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index f38a07ea59f4..f51e3d9ba86e 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -84,7 +84,7 @@ struct brcm_ahci_priv {
};
static const struct ata_port_info ahci_brcm_port_info = {
- .flags = AHCI_FLAG_COMMON,
+ .flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 0:03 ` Florian Fainelli
-1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, linux-arm-kernel, bcm-kernel-feedback-list,
computersforpeace, gregory.0xf0, Danesh Petigara,
Florian Fainelli
From: Danesh Petigara <dpetigara@broadcom.com>
The AHCI driver code stops and starts port DMA engines at will
without considering the power state of the particular port. The
AHCI specification isn't very clear on how to handle this scenario,
leaving implementation open to interpretation.
Broadcom's STB SATA host controller is unable to handle port DMA
controller restarts when the port in question is in low power mode.
When a port enters partial or slumber mode, it's PHY is powered down.
When a controller restart is requested, the controller's internal
state machine expects the PHY to be brought back up by software which
never happens in this case, resulting in failures.
To avoid this situation, logic is added to manually wake up the port
just before it's DMA engine is stopped, if the port happens to be in
a low power state. HBA initiated power management ensures that the port
eventually returns to it's configured low power state, when the link is
idle (as per the conditions listed in the spec). A new host flag is also
added to ensure this logic is only exercised for hosts with the above
limitation.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci.h | 6 ++++++
drivers/ata/ahci_brcmstb.c | 1 +
drivers/ata/libahci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 1 +
include/linux/libata.h | 4 ++++
5 files changed, 64 insertions(+)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index a4faa438889c..03453e6b7d9d 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -198,6 +198,12 @@ enum {
PORT_CMD_ICC_PARTIAL = (0x2 << 28), /* Put i/f in partial state */
PORT_CMD_ICC_SLUMBER = (0x6 << 28), /* Put i/f in slumber state */
+ /* PORT SCR STAT bits */
+ PORT_SCR_STAT_IPM_MASK = (0xf << 8), /* i/f IPM state mask */
+ PORT_SCR_STAT_IPM_ACTIVE = (0x1 << 8), /* i/f in active state */
+ PORT_SCR_STAT_IPM_PARTIAL = (0x2 << 8), /* i/f in partial state */
+ PORT_SCR_STAT_IPM_SLUMBER = (0x6 << 8), /* i/f in slumber state */
+
/* PORT_FBS bits */
PORT_FBS_DWE_OFFSET = 16, /* FBS device with error offset */
PORT_FBS_ADO_OFFSET = 12, /* FBS active dev optimization offset */
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index f51e3d9ba86e..e518a992e774 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -85,6 +85,7 @@ struct brcm_ahci_priv {
static const struct ata_port_info ahci_brcm_port_info = {
.flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
+ .flags2 = ATA_FLAG2_WAKE_BEFORE_STOP,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d61740e78d6d..b6664c579f9a 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
void __iomem *port_mmio = ahci_port_base(ap);
u32 tmp;
+ /*
+ * On some controllers, stopping a port's DMA engine
+ * while the port is in ALPM state (partial or slumber)
+ * results in failures on subsequent DMA engine starts.
+ * For those controllers, put the port back in active
+ * state before stopping it's DMA engine.
+ */
+ if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
+ u32 sts, retries = 10;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+
+ if (!(hpriv->cap & HOST_CAP_ALPM))
+ goto skip_wake_up;
+
+ tmp = readl(port_mmio + PORT_CMD);
+ if (!(tmp & PORT_CMD_ALPE))
+ goto skip_wake_up;
+
+ sts = readl(port_mmio + PORT_SCR_STAT) &
+ PORT_SCR_STAT_IPM_MASK;
+ if ((sts != PORT_SCR_STAT_IPM_PARTIAL) &&
+ (sts != PORT_SCR_STAT_IPM_SLUMBER))
+ goto skip_wake_up;
+
+ tmp |= PORT_CMD_ICC_ACTIVE;
+ writel(tmp, port_mmio + PORT_CMD);
+
+ do {
+ /*
+ * The exit latency for partial state is upto 10usec
+ * whereas it is upto 10msec for slumber state. Use
+ * this information to choose appropriate waiting
+ * calls so the time spent in the loop is minimized.
+ */
+ if (sts == PORT_SCR_STAT_IPM_PARTIAL)
+ udelay(1);
+ else
+ ata_msleep(ap, 1);
+ sts = readl(port_mmio + PORT_SCR_STAT) &
+ PORT_SCR_STAT_IPM_MASK;
+ if (sts == PORT_SCR_STAT_IPM_ACTIVE)
+ break;
+ } while (--retries);
+
+ if (!retries) {
+ dev_err(ap->host->dev,
+ "failed to wake up port\n");
+ return -EIO;
+ }
+ }
+
+skip_wake_up:
tmp = readl(port_mmio + PORT_CMD);
/* check if the HBA is idle */
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 60e368610c74..c6117e1ad373 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5803,6 +5803,7 @@ struct ata_host *ata_host_alloc_pinfo(struct device *dev,
ap->mwdma_mask = pi->mwdma_mask;
ap->udma_mask = pi->udma_mask;
ap->flags |= pi->flags;
+ ap->flags2 |= pi->flags2;
ap->link.flags |= pi->link_flags;
ap->ops = pi->port_ops;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 600c1e0626a5..f278ca897274 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -236,6 +236,8 @@ enum {
/* bits 24:31 of ap->flags are reserved for LLD specific flags */
+ /* struct ata_port flags2 */
+ ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
/* struct ata_port pflags */
ATA_PFLAG_EH_PENDING = (1 << 0), /* EH pending */
@@ -812,6 +814,7 @@ struct ata_port {
/* Flags owned by the EH context. Only EH should touch these once the
port is active */
unsigned long flags; /* ATA_FLAG_xxx */
+ unsigned long flags2; /* ATA_FLAG2_xxx */
/* Flags that change dynamically, protected by ap->lock */
unsigned int pflags; /* ATA_PFLAG_xxx */
unsigned int print_id; /* user visible unique port ID */
@@ -996,6 +999,7 @@ struct ata_port_operations {
struct ata_port_info {
unsigned long flags;
+ unsigned long flags2;
unsigned long link_flags;
unsigned long pio_mask;
unsigned long mwdma_mask;
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Danesh Petigara <dpetigara@broadcom.com>
The AHCI driver code stops and starts port DMA engines at will
without considering the power state of the particular port. The
AHCI specification isn't very clear on how to handle this scenario,
leaving implementation open to interpretation.
Broadcom's STB SATA host controller is unable to handle port DMA
controller restarts when the port in question is in low power mode.
When a port enters partial or slumber mode, it's PHY is powered down.
When a controller restart is requested, the controller's internal
state machine expects the PHY to be brought back up by software which
never happens in this case, resulting in failures.
To avoid this situation, logic is added to manually wake up the port
just before it's DMA engine is stopped, if the port happens to be in
a low power state. HBA initiated power management ensures that the port
eventually returns to it's configured low power state, when the link is
idle (as per the conditions listed in the spec). A new host flag is also
added to ensure this logic is only exercised for hosts with the above
limitation.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci.h | 6 ++++++
drivers/ata/ahci_brcmstb.c | 1 +
drivers/ata/libahci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 1 +
include/linux/libata.h | 4 ++++
5 files changed, 64 insertions(+)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index a4faa438889c..03453e6b7d9d 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -198,6 +198,12 @@ enum {
PORT_CMD_ICC_PARTIAL = (0x2 << 28), /* Put i/f in partial state */
PORT_CMD_ICC_SLUMBER = (0x6 << 28), /* Put i/f in slumber state */
+ /* PORT SCR STAT bits */
+ PORT_SCR_STAT_IPM_MASK = (0xf << 8), /* i/f IPM state mask */
+ PORT_SCR_STAT_IPM_ACTIVE = (0x1 << 8), /* i/f in active state */
+ PORT_SCR_STAT_IPM_PARTIAL = (0x2 << 8), /* i/f in partial state */
+ PORT_SCR_STAT_IPM_SLUMBER = (0x6 << 8), /* i/f in slumber state */
+
/* PORT_FBS bits */
PORT_FBS_DWE_OFFSET = 16, /* FBS device with error offset */
PORT_FBS_ADO_OFFSET = 12, /* FBS active dev optimization offset */
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index f51e3d9ba86e..e518a992e774 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -85,6 +85,7 @@ struct brcm_ahci_priv {
static const struct ata_port_info ahci_brcm_port_info = {
.flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
+ .flags2 = ATA_FLAG2_WAKE_BEFORE_STOP,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d61740e78d6d..b6664c579f9a 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
void __iomem *port_mmio = ahci_port_base(ap);
u32 tmp;
+ /*
+ * On some controllers, stopping a port's DMA engine
+ * while the port is in ALPM state (partial or slumber)
+ * results in failures on subsequent DMA engine starts.
+ * For those controllers, put the port back in active
+ * state before stopping it's DMA engine.
+ */
+ if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
+ u32 sts, retries = 10;
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+
+ if (!(hpriv->cap & HOST_CAP_ALPM))
+ goto skip_wake_up;
+
+ tmp = readl(port_mmio + PORT_CMD);
+ if (!(tmp & PORT_CMD_ALPE))
+ goto skip_wake_up;
+
+ sts = readl(port_mmio + PORT_SCR_STAT) &
+ PORT_SCR_STAT_IPM_MASK;
+ if ((sts != PORT_SCR_STAT_IPM_PARTIAL) &&
+ (sts != PORT_SCR_STAT_IPM_SLUMBER))
+ goto skip_wake_up;
+
+ tmp |= PORT_CMD_ICC_ACTIVE;
+ writel(tmp, port_mmio + PORT_CMD);
+
+ do {
+ /*
+ * The exit latency for partial state is upto 10usec
+ * whereas it is upto 10msec for slumber state. Use
+ * this information to choose appropriate waiting
+ * calls so the time spent in the loop is minimized.
+ */
+ if (sts == PORT_SCR_STAT_IPM_PARTIAL)
+ udelay(1);
+ else
+ ata_msleep(ap, 1);
+ sts = readl(port_mmio + PORT_SCR_STAT) &
+ PORT_SCR_STAT_IPM_MASK;
+ if (sts == PORT_SCR_STAT_IPM_ACTIVE)
+ break;
+ } while (--retries);
+
+ if (!retries) {
+ dev_err(ap->host->dev,
+ "failed to wake up port\n");
+ return -EIO;
+ }
+ }
+
+skip_wake_up:
tmp = readl(port_mmio + PORT_CMD);
/* check if the HBA is idle */
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 60e368610c74..c6117e1ad373 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5803,6 +5803,7 @@ struct ata_host *ata_host_alloc_pinfo(struct device *dev,
ap->mwdma_mask = pi->mwdma_mask;
ap->udma_mask = pi->udma_mask;
ap->flags |= pi->flags;
+ ap->flags2 |= pi->flags2;
ap->link.flags |= pi->link_flags;
ap->ops = pi->port_ops;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 600c1e0626a5..f278ca897274 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -236,6 +236,8 @@ enum {
/* bits 24:31 of ap->flags are reserved for LLD specific flags */
+ /* struct ata_port flags2 */
+ ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
/* struct ata_port pflags */
ATA_PFLAG_EH_PENDING = (1 << 0), /* EH pending */
@@ -812,6 +814,7 @@ struct ata_port {
/* Flags owned by the EH context. Only EH should touch these once the
port is active */
unsigned long flags; /* ATA_FLAG_xxx */
+ unsigned long flags2; /* ATA_FLAG2_xxx */
/* Flags that change dynamically, protected by ap->lock */
unsigned int pflags; /* ATA_PFLAG_xxx */
unsigned int print_id; /* user visible unique port ID */
@@ -996,6 +999,7 @@ struct ata_port_operations {
struct ata_port_info {
unsigned long flags;
+ unsigned long flags2;
unsigned long link_flags;
unsigned long pio_mask;
unsigned long mwdma_mask;
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] libata: skip debounce delay on link resume
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 0:03 ` Florian Fainelli
-1 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-ide
Cc: linux-kernel, linux-arm-kernel, bcm-kernel-feedback-list,
computersforpeace, gregory.0xf0, Danesh Petigara,
Florian Fainelli
From: Danesh Petigara <dpetigara@broadcom.com>
The link resume logic uses a 200msec delay while debouncing
the SControl register. The rationale behind that delay is
to accommodate some PHYs that behave badly if their SStatus/
SControl registers are pounded immediately on resume.
The Broadcom STB SATA PHY does not seem to have this issue.
This patch introduces a new link flag that allows platforms
to skip the debounce delay if it isn't needed.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 1 +
drivers/ata/libata-core.c | 3 ++-
include/linux/libata.h | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index e518a992e774..9d57a27944bc 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -86,6 +86,7 @@ struct brcm_ahci_priv {
static const struct ata_port_info ahci_brcm_port_info = {
.flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
.flags2 = ATA_FLAG2_WAKE_BEFORE_STOP,
+ .link_flags = ATA_LFLAG_NO_DB_DELAY,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c6117e1ad373..b7b578522a13 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3597,7 +3597,8 @@ int sata_link_resume(struct ata_link *link, const unsigned long *params,
* immediately after resuming. Delay 200ms before
* debouncing.
*/
- ata_msleep(link->ap, 200);
+ if (!(link->flags & ATA_LFLAG_NO_DB_DELAY))
+ ata_msleep(link->ap, 200);
/* is SControl restored correctly? */
if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index f278ca897274..e8b1759ff855 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -205,6 +205,7 @@ enum {
ATA_LFLAG_NO_LPM = (1 << 8), /* disable LPM on this link */
ATA_LFLAG_RST_ONCE = (1 << 9), /* limit recovery to one reset */
ATA_LFLAG_CHANGED = (1 << 10), /* LPM state changed on this link */
+ ATA_LFLAG_NO_DB_DELAY = (1 << 11), /* no debounce delay on link resume */
/* struct ata_port flags */
ATA_FLAG_SLAVE_POSS = (1 << 0), /* host supports slave dev */
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] libata: skip debounce delay on link resume
@ 2016-01-08 0:03 ` Florian Fainelli
0 siblings, 0 replies; 24+ messages in thread
From: Florian Fainelli @ 2016-01-08 0:03 UTC (permalink / raw)
To: linux-arm-kernel
From: Danesh Petigara <dpetigara@broadcom.com>
The link resume logic uses a 200msec delay while debouncing
the SControl register. The rationale behind that delay is
to accommodate some PHYs that behave badly if their SStatus/
SControl registers are pounded immediately on resume.
The Broadcom STB SATA PHY does not seem to have this issue.
This patch introduces a new link flag that allows platforms
to skip the debounce delay if it isn't needed.
Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/ata/ahci_brcmstb.c | 1 +
drivers/ata/libata-core.c | 3 ++-
include/linux/libata.h | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
index e518a992e774..9d57a27944bc 100644
--- a/drivers/ata/ahci_brcmstb.c
+++ b/drivers/ata/ahci_brcmstb.c
@@ -86,6 +86,7 @@ struct brcm_ahci_priv {
static const struct ata_port_info ahci_brcm_port_info = {
.flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
.flags2 = ATA_FLAG2_WAKE_BEFORE_STOP,
+ .link_flags = ATA_LFLAG_NO_DB_DELAY,
.pio_mask = ATA_PIO4,
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_platform_ops,
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c6117e1ad373..b7b578522a13 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3597,7 +3597,8 @@ int sata_link_resume(struct ata_link *link, const unsigned long *params,
* immediately after resuming. Delay 200ms before
* debouncing.
*/
- ata_msleep(link->ap, 200);
+ if (!(link->flags & ATA_LFLAG_NO_DB_DELAY))
+ ata_msleep(link->ap, 200);
/* is SControl restored correctly? */
if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index f278ca897274..e8b1759ff855 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -205,6 +205,7 @@ enum {
ATA_LFLAG_NO_LPM = (1 << 8), /* disable LPM on this link */
ATA_LFLAG_RST_ONCE = (1 << 9), /* limit recovery to one reset */
ATA_LFLAG_CHANGED = (1 << 10), /* LPM state changed on this link */
+ ATA_LFLAG_NO_DB_DELAY = (1 << 11), /* no debounce delay on link resume */
/* struct ata_port flags */
ATA_FLAG_SLAVE_POSS = (1 << 0), /* host supports slave dev */
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 16:29 ` Tejun Heo
-1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:29 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0,
Danesh Petigara
On Thu, Jan 07, 2016 at 04:03:30PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> Enable support for ALPM in the host controller's capabilities
> register. Also adjust the PLL timeout to give it enough time
> to lock when the port exits slumber mode.
Applied to libata/for-4.5 w/ minor style adjustments.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM
@ 2016-01-08 16:29 ` Tejun Heo
0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 07, 2016 at 04:03:30PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> Enable support for ALPM in the host controller's capabilities
> register. Also adjust the PLL timeout to give it enough time
> to lock when the port exits slumber mode.
Applied to libata/for-4.5 w/ minor style adjustments.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/4] ata: ahci_brcmstb: disable DIPM support
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 16:30 ` Tejun Heo
-1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:30 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0,
Danesh Petigara
On Thu, Jan 07, 2016 at 04:03:31PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> The Broadcom STB SATA host controller does not support device
> initiated power management. Disable support for this feature
> so the driver never sends SETFEATURES commands to the device
> to enable/disable DIPM.
>
> Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied to libata/for-4.5.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/4] ata: ahci_brcmstb: disable DIPM support
@ 2016-01-08 16:30 ` Tejun Heo
0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:30 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 07, 2016 at 04:03:31PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> The Broadcom STB SATA host controller does not support device
> initiated power management. Disable support for this feature
> so the driver never sends SETFEATURES commands to the device
> to enable/disable DIPM.
>
> Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied to libata/for-4.5.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 16:36 ` Tejun Heo
-1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0,
Danesh Petigara
Hello,
On Thu, Jan 07, 2016 at 04:03:32PM -0800, Florian Fainelli wrote:
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index d61740e78d6d..b6664c579f9a 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
> void __iomem *port_mmio = ahci_port_base(ap);
> u32 tmp;
>
> + /*
> + * On some controllers, stopping a port's DMA engine
> + * while the port is in ALPM state (partial or slumber)
> + * results in failures on subsequent DMA engine starts.
> + * For those controllers, put the port back in active
> + * state before stopping it's DMA engine.
> + */
> + if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
...
So, I'm not too comfortable with open coding ALPM switching in
ahci_stop_engine(). Shouldn't it be possible to update and/or
refactor and reuse ahci_set_lpm()?
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -236,6 +236,8 @@ enum {
>
> /* bits 24:31 of ap->flags are reserved for LLD specific flags */
>
> + /* struct ata_port flags2 */
> + ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
What's wrong with bits 2-5 in ATA_FLAG_* space? Also, should this be
a ahci priv flag?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
@ 2016-01-08 16:36 ` Tejun Heo
0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:36 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Thu, Jan 07, 2016 at 04:03:32PM -0800, Florian Fainelli wrote:
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index d61740e78d6d..b6664c579f9a 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
> void __iomem *port_mmio = ahci_port_base(ap);
> u32 tmp;
>
> + /*
> + * On some controllers, stopping a port's DMA engine
> + * while the port is in ALPM state (partial or slumber)
> + * results in failures on subsequent DMA engine starts.
> + * For those controllers, put the port back in active
> + * state before stopping it's DMA engine.
> + */
> + if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
...
So, I'm not too comfortable with open coding ALPM switching in
ahci_stop_engine(). Shouldn't it be possible to update and/or
refactor and reuse ahci_set_lpm()?
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -236,6 +236,8 @@ enum {
>
> /* bits 24:31 of ap->flags are reserved for LLD specific flags */
>
> + /* struct ata_port flags2 */
> + ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
What's wrong with bits 2-5 in ATA_FLAG_* space? Also, should this be
a ahci priv flag?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] libata: skip debounce delay on link resume
2016-01-08 0:03 ` Florian Fainelli
@ 2016-01-08 16:50 ` Tejun Heo
-1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:50 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0,
Danesh Petigara
On Thu, Jan 07, 2016 at 04:03:33PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> The link resume logic uses a 200msec delay while debouncing
> the SControl register. The rationale behind that delay is
> to accommodate some PHYs that behave badly if their SStatus/
> SControl registers are pounded immediately on resume.
> The Broadcom STB SATA PHY does not seem to have this issue.
> This patch introduces a new link flag that allows platforms
> to skip the debounce delay if it isn't needed.
>
> Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied to libata/for-4.5.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] libata: skip debounce delay on link resume
@ 2016-01-08 16:50 ` Tejun Heo
0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-08 16:50 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 07, 2016 at 04:03:33PM -0800, Florian Fainelli wrote:
> From: Danesh Petigara <dpetigara@broadcom.com>
>
> The link resume logic uses a 200msec delay while debouncing
> the SControl register. The rationale behind that delay is
> to accommodate some PHYs that behave badly if their SStatus/
> SControl registers are pounded immediately on resume.
> The Broadcom STB SATA PHY does not seem to have this issue.
> This patch introduces a new link flag that allows platforms
> to skip the debounce delay if it isn't needed.
>
> Signed-off-by: Danesh Petigara <dpetigara@broadcom.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied to libata/for-4.5.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
2016-01-08 16:36 ` Tejun Heo
(?)
@ 2016-01-08 23:20 ` Danesh Petigara
-1 siblings, 0 replies; 24+ messages in thread
From: Danesh Petigara @ 2016-01-08 23:20 UTC (permalink / raw)
To: Tejun Heo, Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0
Hi Tejun,
On 1/8/2016 8:36 AM, Tejun Heo wrote:
> Hello,
>
> On Thu, Jan 07, 2016 at 04:03:32PM -0800, Florian Fainelli wrote:
>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>> index d61740e78d6d..b6664c579f9a 100644
>> --- a/drivers/ata/libahci.c
>> +++ b/drivers/ata/libahci.c
>> @@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
>> void __iomem *port_mmio = ahci_port_base(ap);
>> u32 tmp;
>>
>> + /*
>> + * On some controllers, stopping a port's DMA engine
>> + * while the port is in ALPM state (partial or slumber)
>> + * results in failures on subsequent DMA engine starts.
>> + * For those controllers, put the port back in active
>> + * state before stopping it's DMA engine.
>> + */
>> + if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
> ...
>
> So, I'm not too comfortable with open coding ALPM switching in
> ahci_stop_engine(). Shouldn't it be possible to update and/or
> refactor and reuse ahci_set_lpm()?
>
ahci_set_lpm in it's current form cannot be used here as it also
modifies the ALPE/ASP bits. The idea is to wake the link before DMA stop
without changing the ALPM bits so the link can return to it's configured
low power state when it's idle.
We could however update that function by introducing a new hints flag
that allows us to skip unneeded logic for this scenario. ahci_set_lpm
can then be called from ahci_stop_engine.
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -236,6 +236,8 @@ enum {
>>
>> /* bits 24:31 of ap->flags are reserved for LLD specific flags */
>>
>> + /* struct ata_port flags2 */
>> + ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
>
> What's wrong with bits 2-5 in ATA_FLAG_* space? Also, should this be
> a ahci priv flag?
I wasn't sure whether those bits were left unused on purpose, so I did
not use them. The point to make this a AHCI_HFLAG_ flag makes perfect sense.
>
> Thanks.
>
Thanks for the feedback.
Danesh
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
@ 2016-01-08 23:20 ` Danesh Petigara
0 siblings, 0 replies; 24+ messages in thread
From: Danesh Petigara @ 2016-01-08 23:20 UTC (permalink / raw)
To: Tejun Heo, Florian Fainelli
Cc: linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0
Hi Tejun,
On 1/8/2016 8:36 AM, Tejun Heo wrote:
> Hello,
>
> On Thu, Jan 07, 2016 at 04:03:32PM -0800, Florian Fainelli wrote:
>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>> index d61740e78d6d..b6664c579f9a 100644
>> --- a/drivers/ata/libahci.c
>> +++ b/drivers/ata/libahci.c
>> @@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
>> void __iomem *port_mmio = ahci_port_base(ap);
>> u32 tmp;
>>
>> + /*
>> + * On some controllers, stopping a port's DMA engine
>> + * while the port is in ALPM state (partial or slumber)
>> + * results in failures on subsequent DMA engine starts.
>> + * For those controllers, put the port back in active
>> + * state before stopping it's DMA engine.
>> + */
>> + if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
> ...
>
> So, I'm not too comfortable with open coding ALPM switching in
> ahci_stop_engine(). Shouldn't it be possible to update and/or
> refactor and reuse ahci_set_lpm()?
>
ahci_set_lpm in it's current form cannot be used here as it also
modifies the ALPE/ASP bits. The idea is to wake the link before DMA stop
without changing the ALPM bits so the link can return to it's configured
low power state when it's idle.
We could however update that function by introducing a new hints flag
that allows us to skip unneeded logic for this scenario. ahci_set_lpm
can then be called from ahci_stop_engine.
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -236,6 +236,8 @@ enum {
>>
>> /* bits 24:31 of ap->flags are reserved for LLD specific flags */
>>
>> + /* struct ata_port flags2 */
>> + ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
>
> What's wrong with bits 2-5 in ATA_FLAG_* space? Also, should this be
> a ahci priv flag?
I wasn't sure whether those bits were left unused on purpose, so I did
not use them. The point to make this a AHCI_HFLAG_ flag makes perfect sense.
>
> Thanks.
>
Thanks for the feedback.
Danesh
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
@ 2016-01-08 23:20 ` Danesh Petigara
0 siblings, 0 replies; 24+ messages in thread
From: Danesh Petigara @ 2016-01-08 23:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tejun,
On 1/8/2016 8:36 AM, Tejun Heo wrote:
> Hello,
>
> On Thu, Jan 07, 2016 at 04:03:32PM -0800, Florian Fainelli wrote:
>> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>> index d61740e78d6d..b6664c579f9a 100644
>> --- a/drivers/ata/libahci.c
>> +++ b/drivers/ata/libahci.c
>> @@ -595,6 +595,58 @@ int ahci_stop_engine(struct ata_port *ap)
>> void __iomem *port_mmio = ahci_port_base(ap);
>> u32 tmp;
>>
>> + /*
>> + * On some controllers, stopping a port's DMA engine
>> + * while the port is in ALPM state (partial or slumber)
>> + * results in failures on subsequent DMA engine starts.
>> + * For those controllers, put the port back in active
>> + * state before stopping it's DMA engine.
>> + */
>> + if (ap->flags2 & ATA_FLAG2_WAKE_BEFORE_STOP) {
> ...
>
> So, I'm not too comfortable with open coding ALPM switching in
> ahci_stop_engine(). Shouldn't it be possible to update and/or
> refactor and reuse ahci_set_lpm()?
>
ahci_set_lpm in it's current form cannot be used here as it also
modifies the ALPE/ASP bits. The idea is to wake the link before DMA stop
without changing the ALPM bits so the link can return to it's configured
low power state when it's idle.
We could however update that function by introducing a new hints flag
that allows us to skip unneeded logic for this scenario. ahci_set_lpm
can then be called from ahci_stop_engine.
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -236,6 +236,8 @@ enum {
>>
>> /* bits 24:31 of ap->flags are reserved for LLD specific flags */
>>
>> + /* struct ata_port flags2 */
>> + ATA_FLAG2_WAKE_BEFORE_STOP = (1 << 0), /* wake link before DMA stop */
>
> What's wrong with bits 2-5 in ATA_FLAG_* space? Also, should this be
> a ahci priv flag?
I wasn't sure whether those bits were left unused on purpose, so I did
not use them. The point to make this a AHCI_HFLAG_ flag makes perfect sense.
>
> Thanks.
>
Thanks for the feedback.
Danesh
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
2016-01-08 23:20 ` Danesh Petigara
@ 2016-01-10 17:29 ` Tejun Heo
-1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-10 17:29 UTC (permalink / raw)
To: Danesh Petigara
Cc: Florian Fainelli, linux-ide, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list, computersforpeace, gregory.0xf0
Hello, Danesh.
On Fri, Jan 08, 2016 at 03:20:44PM -0800, Danesh Petigara wrote:
> ahci_set_lpm in it's current form cannot be used here as it also
> modifies the ALPE/ASP bits. The idea is to wake the link before DMA stop
> without changing the ALPM bits so the link can return to it's configured
> low power state when it's idle.
>
> We could however update that function by introducing a new hints flag
> that allows us to skip unneeded logic for this scenario. ahci_set_lpm
> can then be called from ahci_stop_engine.
Yeah, please either update the function or factor the necessary part
out.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM
@ 2016-01-10 17:29 ` Tejun Heo
0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2016-01-10 17:29 UTC (permalink / raw)
To: linux-arm-kernel
Hello, Danesh.
On Fri, Jan 08, 2016 at 03:20:44PM -0800, Danesh Petigara wrote:
> ahci_set_lpm in it's current form cannot be used here as it also
> modifies the ALPE/ASP bits. The idea is to wake the link before DMA stop
> without changing the ALPM bits so the link can return to it's configured
> low power state when it's idle.
>
> We could however update that function by introducing a new hints flag
> that allows us to skip unneeded logic for this scenario. ahci_set_lpm
> can then be called from ahci_stop_engine.
Yeah, please either update the function or factor the necessary part
out.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-01-10 17:29 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-08 0:03 [PATCH 0/4] ata: ahci_brcmstb: ALPM support Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 0:03 ` [PATCH 1/4] ata: ahci_brcmstb: enable support for ALPM Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 16:29 ` Tejun Heo
2016-01-08 16:29 ` Tejun Heo
2016-01-08 0:03 ` [PATCH 2/4] ata: ahci_brcmstb: disable DIPM support Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 16:30 ` Tejun Heo
2016-01-08 16:30 ` Tejun Heo
2016-01-08 0:03 ` [PATCH 3/4] drivers: ata: wake port before DMA stop for ALPM Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 16:36 ` Tejun Heo
2016-01-08 16:36 ` Tejun Heo
2016-01-08 23:20 ` Danesh Petigara
2016-01-08 23:20 ` Danesh Petigara
2016-01-08 23:20 ` Danesh Petigara
2016-01-10 17:29 ` Tejun Heo
2016-01-10 17:29 ` Tejun Heo
2016-01-08 0:03 ` [PATCH 4/4] libata: skip debounce delay on link resume Florian Fainelli
2016-01-08 0:03 ` Florian Fainelli
2016-01-08 16:50 ` Tejun Heo
2016-01-08 16:50 ` Tejun Heo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.