[PATCH v6 0/3] ata: ahci_brcm: Fix use of BCM7216 reset controller

[PATCH v6 0/3] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Jim Quinlan]

v6 -- Added new commit which adds a missing function to the reset API.
      This fixes 557acb3d2cd9 and should be destined for linux stable.

v5 -- Improved (I hope) commit description (Bjorn).
   -- Rnamed error labels (Krzyszt).
   -- Fixed typos.

v4 -- does not rely on a pending commit, unlike v3.

v3 -- discard commit from v2; instead rely on the new function
      reset_control_rearm provided in a recent commit [1] applied
      to reset/next.
   -- New commit to correct pcie-brcmstb.c usage of a reset controller
      to use reset/rearm verses deassert/assert.

v2 -- refactor rescal-reset driver to implement assert/deassert rather than
      reset because the reset call only fires once per lifetime and we need
      to reset after every resume from S2 or S3.
   -- Split the use of "ahci" and "rescal" controllers in separate fields
      to keep things simple.

v1 -- original

Jim Quinlan (3):
  reset: add missing empty function reset_control_rearm()
  ata: ahci_brcm: Fix use of BCM7216 reset controller
  PCI: brcmstb: Use reset/rearm instead of deassert/assert

 drivers/ata/ahci_brcm.c               | 46 +++++++++++++--------------
 drivers/pci/controller/pcie-brcmstb.c | 19 +++++++----
 include/linux/reset.h                 |  5 +++
 3 files changed, 41 insertions(+), 29 deletions(-)

-- 
2.17.1

[PATCH v6 1/3] reset: add missing empty function reset_control_rearm()

[Jim Quinlan]

All other functions are defined for when CONFIG_RESET_CONTROLLER
is not set.

Fixes: 557acb3d2cd9 ("reset: make shared pulsed reset controls re-triggerable")
CC: stable@vger.kernel.org # v5.11+
Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
---
 include/linux/reset.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/reset.h b/include/linux/reset.h
index b9109efa2a5c..9700124affa3 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -47,6 +47,11 @@ static inline int reset_control_reset(struct reset_control *rstc)
 	return 0;
 }
 
+static inline int reset_control_rearm(struct reset_control *rstc)
+{
+	return 0;
+}
+
 static inline int reset_control_assert(struct reset_control *rstc)
 {
 	return 0;
-- 
2.17.1

[PATCH v6 2/3] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Jim Quinlan]

This driver may use one of two resets controllers.  Keep them in separate
variables to keep things simple.  The reset controller "rescal" is shared
between the AHCI driver and the PCIe driver for the BrcmSTB 7216 chip.  Use
devm_reset_control_get_optional_shared() to handle this sharing.

Fixes: 272ecd60a636 ("ata: ahci_brcm: BCM7216 reset is self de-asserting")
Fixes: c345ec6a50e9 ("ata: ahci_brcm: Support BCM7216 reset controller name")
Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/ata/ahci_brcm.c | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 5b32df5d33ad..6e9c5ade4c2e 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -86,7 +86,8 @@ struct brcm_ahci_priv {
 	u32 port_mask;
 	u32 quirks;
 	enum brcm_ahci_version version;
-	struct reset_control *rcdev;
+	struct reset_control *rcdev_rescal;
+	struct reset_control *rcdev_ahci;
 };
 
 static inline u32 brcm_sata_readreg(void __iomem *addr)
@@ -352,8 +353,8 @@ static int brcm_ahci_suspend(struct device *dev)
 	else
 		ret = 0;
 
-	if (priv->version != BRCM_SATA_BCM7216)
-		reset_control_assert(priv->rcdev);
+	reset_control_assert(priv->rcdev_ahci);
+	reset_control_rearm(priv->rcdev_rescal);
 
 	return ret;
 }
@@ -365,10 +366,10 @@ static int __maybe_unused brcm_ahci_resume(struct device *dev)
 	struct brcm_ahci_priv *priv = hpriv->plat_data;
 	int ret = 0;
 
-	if (priv->version == BRCM_SATA_BCM7216)
-		ret = reset_control_reset(priv->rcdev);
-	else
-		ret = reset_control_deassert(priv->rcdev);
+	ret = reset_control_deassert(priv->rcdev_ahci);
+	if (ret)
+		return ret;
+	ret = reset_control_reset(priv->rcdev_rescal);
 	if (ret)
 		return ret;
 
@@ -434,7 +435,6 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
-	const char *reset_name = NULL;
 	struct brcm_ahci_priv *priv;
 	struct ahci_host_priv *hpriv;
 	struct resource *res;
@@ -456,15 +456,15 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->top_ctrl))
 		return PTR_ERR(priv->top_ctrl);
 
-	/* Reset is optional depending on platform and named differently */
-	if (priv->version == BRCM_SATA_BCM7216)
-		reset_name = "rescal";
-	else
-		reset_name = "ahci";
-
-	priv->rcdev = devm_reset_control_get_optional(&pdev->dev, reset_name);
-	if (IS_ERR(priv->rcdev))
-		return PTR_ERR(priv->rcdev);
+	if (priv->version == BRCM_SATA_BCM7216) {
+		priv->rcdev_rescal = devm_reset_control_get_optional_shared(
+			&pdev->dev, "rescal");
+		if (IS_ERR(priv->rcdev_rescal))
+			return PTR_ERR(priv->rcdev_rescal);
+	}
+	priv->rcdev_ahci = devm_reset_control_get_optional(&pdev->dev, "ahci");
+	if (IS_ERR(priv->rcdev_ahci))
+		return PTR_ERR(priv->rcdev_ahci);
 
 	hpriv = ahci_platform_get_resources(pdev, 0);
 	if (IS_ERR(hpriv))
@@ -485,10 +485,10 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 		break;
 	}
 
-	if (priv->version == BRCM_SATA_BCM7216)
-		ret = reset_control_reset(priv->rcdev);
-	else
-		ret = reset_control_deassert(priv->rcdev);
+	ret = reset_control_reset(priv->rcdev_rescal);
+	if (ret)
+		return ret;
+	ret = reset_control_deassert(priv->rcdev_ahci);
 	if (ret)
 		return ret;
 
@@ -539,8 +539,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
 out_disable_clks:
 	ahci_platform_disable_clks(hpriv);
 out_reset:
-	if (priv->version != BRCM_SATA_BCM7216)
-		reset_control_assert(priv->rcdev);
+	reset_control_assert(priv->rcdev_ahci);
+	reset_control_rearm(priv->rcdev_rescal);
 	return ret;
 }
 
-- 
2.17.1

[PATCH v6 3/3] PCI: brcmstb: Use reset/rearm instead of deassert/assert

[Jim Quinlan]

The Broadcom STB PCIe RC uses a reset control "rescal" for certain chips.
The "rescal" implements a "pulse reset" so using assert/deassert is wrong
for this device.  Instead, we use reset/rearm.  We need to use rearm so
that we can reset it after a suspend/resume cycle; w/o using "rearm", the
"rescal" device will only ever fire once.

Of course for suspend/resume to work we also need to put the reset/rearm
calls in the suspend and resume routines.

Fixes: 740d6c3708a9 ("PCI: brcmstb: Add control of rescal reset")
Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index e330e6811f0b..3b35d629035e 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1148,6 +1148,7 @@ static int brcm_pcie_suspend(struct device *dev)
 
 	brcm_pcie_turn_off(pcie);
 	ret = brcm_phy_stop(pcie);
+	reset_control_rearm(pcie->rescal);
 	clk_disable_unprepare(pcie->clk);
 
 	return ret;
@@ -1163,9 +1164,13 @@ static int brcm_pcie_resume(struct device *dev)
 	base = pcie->base;
 	clk_prepare_enable(pcie->clk);
 
+	ret = reset_control_reset(pcie->rescal);
+	if (ret)
+		goto err_disable_clk;
+
 	ret = brcm_phy_start(pcie);
 	if (ret)
-		goto err;
+		goto err_reset;
 
 	/* Take bridge out of reset so we can access the SERDES reg */
 	pcie->bridge_sw_init_set(pcie, 0);
@@ -1180,14 +1185,16 @@ static int brcm_pcie_resume(struct device *dev)
 
 	ret = brcm_pcie_setup(pcie);
 	if (ret)
-		goto err;
+		goto err_reset;
 
 	if (pcie->msi)
 		brcm_msi_set_regs(pcie->msi);
 
 	return 0;
 
-err:
+err_reset:
+	reset_control_rearm(pcie->rescal);
+err_disable_clk:
 	clk_disable_unprepare(pcie->clk);
 	return ret;
 }
@@ -1197,7 +1204,7 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
 	brcm_msi_remove(pcie);
 	brcm_pcie_turn_off(pcie);
 	brcm_phy_stop(pcie);
-	reset_control_assert(pcie->rescal);
+	reset_control_rearm(pcie->rescal);
 	clk_disable_unprepare(pcie->clk);
 }
 
@@ -1278,13 +1285,13 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pcie->perst_reset);
 	}
 
-	ret = reset_control_deassert(pcie->rescal);
+	ret = reset_control_reset(pcie->rescal);
 	if (ret)
 		dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
 
 	ret = brcm_phy_start(pcie);
 	if (ret) {
-		reset_control_assert(pcie->rescal);
+		reset_control_rearm(pcie->rescal);
 		clk_disable_unprepare(pcie->clk);
 		return ret;
 	}
-- 
2.17.1

Re: [PATCH v6 1/3] reset: add missing empty function reset_control_rearm()

[Bjorn Helgaas]

On Fri, Apr 30, 2021 at 11:21:54AM -0400, Jim Quinlan wrote:
> All other functions are defined for when CONFIG_RESET_CONTROLLER
> is not set.
> 
> Fixes: 557acb3d2cd9 ("reset: make shared pulsed reset controls re-triggerable")
> CC: stable@vger.kernel.org # v5.11+
> Signed-off-by: Jim Quinlan <jim2101024@gmail.com>

Philipp, I'd like to merge this via the PCI tree since the brcmstb
patch depends on it.  It looks correct to me, but I'd really like to
have your ack before merging it.

> ---
>  include/linux/reset.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index b9109efa2a5c..9700124affa3 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -47,6 +47,11 @@ static inline int reset_control_reset(struct reset_control *rstc)
>  	return 0;
>  }
>  
> +static inline int reset_control_rearm(struct reset_control *rstc)
> +{
> +	return 0;
> +}
> +
>  static inline int reset_control_assert(struct reset_control *rstc)
>  {
>  	return 0;
> -- 
> 2.17.1
> 

Re: [PATCH v6 0/3] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Bjorn Helgaas]

On Fri, Apr 30, 2021 at 11:21:53AM -0400, Jim Quinlan wrote:
> v6 -- Added new commit which adds a missing function to the reset API.
>       This fixes 557acb3d2cd9 and should be destined for linux stable.
> 
> v5 -- Improved (I hope) commit description (Bjorn).
>    -- Rnamed error labels (Krzyszt).
>    -- Fixed typos.
> 
> v4 -- does not rely on a pending commit, unlike v3.
> 
> v3 -- discard commit from v2; instead rely on the new function
>       reset_control_rearm provided in a recent commit [1] applied
>       to reset/next.
>    -- New commit to correct pcie-brcmstb.c usage of a reset controller
>       to use reset/rearm verses deassert/assert.
> 
> v2 -- refactor rescal-reset driver to implement assert/deassert rather than
>       reset because the reset call only fires once per lifetime and we need
>       to reset after every resume from S2 or S3.
>    -- Split the use of "ahci" and "rescal" controllers in separate fields
>       to keep things simple.
> 
> v1 -- original
> 
> Jim Quinlan (3):
>   reset: add missing empty function reset_control_rearm()
>   ata: ahci_brcm: Fix use of BCM7216 reset controller
>   PCI: brcmstb: Use reset/rearm instead of deassert/assert
> 
>  drivers/ata/ahci_brcm.c               | 46 +++++++++++++--------------
>  drivers/pci/controller/pcie-brcmstb.c | 19 +++++++----
>  include/linux/reset.h                 |  5 +++
>  3 files changed, 41 insertions(+), 29 deletions(-)

I provisionally applied these to my pci/brcmstb branch for v5.13.

I carried forward Jens' ack on "ata: ahci_brcm: Fix use of BCM7216
reset controller" since the patch is identical to the v5 version that
he acked.

I'm hoping to get an ack from Philipp for the reset.h change.

Bjorn

Re: [PATCH v6 1/3] reset: add missing empty function reset_control_rearm()

[Philipp Zabel]

On Mon, 2021-05-03 at 13:56 -0500, Bjorn Helgaas wrote:
> On Fri, Apr 30, 2021 at 11:21:54AM -0400, Jim Quinlan wrote:
> > All other functions are defined for when CONFIG_RESET_CONTROLLER
> > is not set.
> > 
> > Fixes: 557acb3d2cd9 ("reset: make shared pulsed reset controls re-triggerable")
> > CC: stable@vger.kernel.org # v5.11+
> > Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
> 
> Philipp, I'd like to merge this via the PCI tree since the brcmstb
> patch depends on it.  It looks correct to me, but I'd really like to
> have your ack before merging it.

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH v6 1/3] reset: add missing empty function reset_control_rearm()

[Bjorn Helgaas]

On Mon, May 10, 2021 at 09:45:54AM +0200, Philipp Zabel wrote:
> On Mon, 2021-05-03 at 13:56 -0500, Bjorn Helgaas wrote:
> > On Fri, Apr 30, 2021 at 11:21:54AM -0400, Jim Quinlan wrote:
> > > All other functions are defined for when CONFIG_RESET_CONTROLLER
> > > is not set.
> > > 
> > > Fixes: 557acb3d2cd9 ("reset: make shared pulsed reset controls re-triggerable")
> > > CC: stable@vger.kernel.org # v5.11+
> > > Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
> > 
> > Philipp, I'd like to merge this via the PCI tree since the brcmstb
> > patch depends on it.  It looks correct to me, but I'd really like to
> > have your ack before merging it.
> 
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Thanks a lot!  I wanted to include this for v5.13, so I took the
liberty of including it without your ack, so I'm glad you agree ;)
Sorry for the late ack request.

https://git.kernel.org/linus/48582b2e3b87