[PATCH 0/3] libata: prevent writes to read-only registers

[PATCH 0/3] libata: prevent writes to read-only registers

[Doug Berger]

Recent Broadcom SoCs allow for the trapping of write accesses to
read-only registers. This is only useful if such accesses are
exceptional, so it is desirable to prevent such accesses in normal
operation. To that end, this set of commits proposes adding two
flags to the libata core.

The first allows for a quirk that exists in some Broadcom devices
that are capable of supporting Aggresive Link Power Management even
though it is not reported by their read-only capability register.
This removes a need for the Broadcom driver to write to it's
read-only capability register.

The second is a notification to the libata-core that it should not
write to any standard registers that are defined to be read-only.

The Broadcom driver is then modified to use these two general 
purpose flags.

Doug Berger (3):
  libata: Add the AHCI_HFLAG_YES_ALPM flag
  libata: Add the AHCI_HFLAG_NO_WRITE_TO_RO flag
  ata: ahci_brcm: Avoid writing to read-only registers

 drivers/ata/ahci.h      |  3 +++
 drivers/ata/ahci_brcm.c | 12 ++++--------
 drivers/ata/libahci.c   |  8 +++++++-
 3 files changed, 14 insertions(+), 9 deletions(-)

-- 
2.13.0

[PATCH 1/3] libata: Add the AHCI_HFLAG_YES_ALPM flag

[Doug Berger]

Some hardware is capable of supporting Aggresive Link Power Management
even though it is not indicated by the Host Capability register.

This commit adds the AHCI_HFLAG_YES_ALPM flag to the AHCI library to
allow indication of this quirk when the Host Capability register is
Read Only and therefore cannot be changed.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/ata/ahci.h    | 1 +
 drivers/ata/libahci.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 30f67a1a4f54..ee176e4af97a 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -248,6 +248,7 @@ enum {
 	AHCI_HFLAG_MULTI_MSI		= 0,
 #endif
 	AHCI_HFLAG_WAKE_BEFORE_STOP	= (1 << 22), /* wake before DMA stop */
+	AHCI_HFLAG_YES_ALPM		= (1 << 23), /* force ALPM cap on */
 
 	/* ap->flags bits */
 
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index b3a685ad9b87..4462f8a8cf2c 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -504,6 +504,11 @@ void ahci_save_initial_config(struct device *dev, struct ahci_host_priv *hpriv)
 		cap &= ~HOST_CAP_FBS;
 	}
 
+	if (!(cap & HOST_CAP_ALPM) && (hpriv->flags & AHCI_HFLAG_YES_ALPM)) {
+		dev_info(dev, "controller can do ALPM, turning on CAP_ALPM\n");
+		cap |= HOST_CAP_ALPM;
+	}
+
 	if (hpriv->force_port_map && port_map != hpriv->force_port_map) {
 		dev_info(dev, "forcing port_map 0x%x -> 0x%x\n",
 			 port_map, hpriv->force_port_map);
-- 
2.13.0

[PATCH 2/3] libata: Add the AHCI_HFLAG_NO_WRITE_TO_RO flag

[Doug Berger]

While most hardware will simply ignore a write to a read-only register,
some hardware will signal an abort if this occurs.

This commit introduces the flag AHCI_HFLAG_NO_WRITE_TO_RO to prevent the
AHCI library from attempting to write to the HOST_CAP, HOST_CAP2, and
HOST_PORTS_IMPL registers which may be read-only.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/ata/ahci.h    | 2 ++
 drivers/ata/libahci.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index ee176e4af97a..8b61123d2c3c 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -249,6 +249,8 @@ enum {
 #endif
 	AHCI_HFLAG_WAKE_BEFORE_STOP	= (1 << 22), /* wake before DMA stop */
 	AHCI_HFLAG_YES_ALPM		= (1 << 23), /* force ALPM cap on */
+	AHCI_HFLAG_NO_WRITE_TO_RO	= (1 << 24), /* don't write to read
+							only registers */
 
 	/* ap->flags bits */
 
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 4462f8a8cf2c..3e286d86ab42 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -945,7 +945,8 @@ int ahci_reset_controller(struct ata_host *host)
 		/* Some registers might be cleared on reset.  Restore
 		 * initial values.
 		 */
-		ahci_restore_initial_config(host);
+		if (!(hpriv->flags & AHCI_HFLAG_NO_WRITE_TO_RO))
+			ahci_restore_initial_config(host);
 	} else
 		dev_info(host->dev, "skipping global host reset\n");
 
-- 
2.13.0

[PATCH 3/3] ata: ahci_brcm: Avoid writing to read-only registers

[Doug Berger]

This commit makes use of the AHCI_HFLAG_YES_ALPM flag to prevent
the driver from writing to the read-only Host Capability register.

It also sets the AHCI_HFLAG_NO_WRITE_TO_RO flag to prevent the AHCI
library from writing to read-only registers.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 drivers/ata/ahci_brcm.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 6f8a7341fa08..5936d1679bf3 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -39,7 +39,6 @@
  #define PIODATA_ENDIAN_SHIFT				6
   #define ENDIAN_SWAP_NONE				0
   #define ENDIAN_SWAP_FULL				2
- #define OVERRIDE_HWINIT				BIT(16)
 #define SATA_TOP_CTRL_TP_CTRL				0x8
 #define SATA_TOP_CTRL_PHY_CTRL				0xc
  #define SATA_TOP_CTRL_PHY_CTRL_1			0x0
@@ -126,17 +125,13 @@ static inline void brcm_sata_writereg(u32 val, void __iomem *addr)
 static void brcm_sata_alpm_init(struct ahci_host_priv *hpriv)
 {
 	struct brcm_ahci_priv *priv = hpriv->plat_data;
-	u32 bus_ctrl, port_ctrl, host_caps;
+	u32 port_ctrl, host_caps;
 	int i;
 
 	/* 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);
+	if (!(host_caps & HOST_CAP_ALPM))
+		hpriv->flags |= AHCI_HFLAG_YES_ALPM;
 
 	/*
 	 * Adjust timeout to allow PLL sufficient time to lock while waking
@@ -360,6 +355,7 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 
 	if (priv->quirks & BRCM_AHCI_QUIRK_NO_NCQ)
 		hpriv->flags |= AHCI_HFLAG_NO_NCQ;
+	hpriv->flags |= AHCI_HFLAG_NO_WRITE_TO_RO;
 
 	ret = ahci_platform_init_host(pdev, hpriv, &ahci_brcm_port_info,
 				      &ahci_platform_sht);
-- 
2.13.0

Re: [PATCH 0/3] libata: prevent writes to read-only registers

[Tejun Heo]

On Wed, Jun 21, 2017 at 04:20:11PM -0700, Doug Berger wrote:
> Recent Broadcom SoCs allow for the trapping of write accesses to
> read-only registers. This is only useful if such accesses are
> exceptional, so it is desirable to prevent such accesses in normal
> operation. To that end, this set of commits proposes adding two
> flags to the libata core.
> 
> The first allows for a quirk that exists in some Broadcom devices
> that are capable of supporting Aggresive Link Power Management even
> though it is not reported by their read-only capability register.
> This removes a need for the Broadcom driver to write to it's
> read-only capability register.
> 
> The second is a notification to the libata-core that it should not
> write to any standard registers that are defined to be read-only.
> 
> The Broadcom driver is then modified to use these two general 
> purpose flags.

Applied 1-3 to libata/for-4.13.

Thanks.

-- 
tejun