linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller
@ 2021-03-08 19:50 Jim Quinlan
  2021-03-08 19:50 ` [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert Jim Quinlan
  0 siblings, 1 reply; 4+ messages in thread
From: Jim Quinlan @ 2021-03-08 19:50 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Nicolas Saenz Julienne,
	bcm-kernel-feedback-list, james.quinlan, jim2101024
  Cc: Florian Fainelli, Hans de Goede, Jens Axboe, Jim Quinlan,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	open list,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Lorenzo Pieralisi, Rob Herring

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 (2):
  ata: ahci_brcm: Fix use of BCM7216 reset controller
  nPCI: brcmstb: Use reset/rearm instead of deassert/assert

 drivers/ata/ahci_brcm.c               | 46 +++++++++++++--------------
 drivers/pci/controller/pcie-brcmstb.c | 19 +++++++----
 2 files changed, 36 insertions(+), 29 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert
  2021-03-08 19:50 [PATCH v4 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
@ 2021-03-08 19:50 ` Jim Quinlan
  2021-03-08 23:35   ` Krzysztof Wilczyński
  2021-03-09  1:34   ` Bjorn Helgaas
  0 siblings, 2 replies; 4+ messages in thread
From: Jim Quinlan @ 2021-03-08 19:50 UTC (permalink / raw)
  To: Bjorn Helgaas, linux-pci, Nicolas Saenz Julienne,
	bcm-kernel-feedback-list, james.quinlan, jim2101024
  Cc: Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Florian Fainelli,
	Philipp Zabel, Jim Quinlan,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

The Brcmstb PCIe RC uses a reset control "rescal" for certain chips.  This
reset implements a "pulse reset" so it matches more the reset/rearm
calls instead of the deassert/assert calls.

Also, add reset_control calls in suspend/resume functions.

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..18f23cba7e3a 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 err0;
+
 	ret = brcm_phy_start(pcie);
 	if (ret)
-		goto err;
+		goto err1;
 
 	/* 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 err1;
 
 	if (pcie->msi)
 		brcm_msi_set_regs(pcie->msi);
 
 	return 0;
 
-err:
+err1:
+	reset_control_rearm(pcie->rescal);
+err0:
 	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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert
  2021-03-08 19:50 ` [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert Jim Quinlan
@ 2021-03-08 23:35   ` Krzysztof Wilczyński
  2021-03-09  1:34   ` Bjorn Helgaas
  1 sibling, 0 replies; 4+ messages in thread
From: Krzysztof Wilczyński @ 2021-03-08 23:35 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: Bjorn Helgaas, linux-pci, Nicolas Saenz Julienne,
	bcm-kernel-feedback-list, james.quinlan, Lorenzo Pieralisi,
	Rob Herring, Bjorn Helgaas, Florian Fainelli, Philipp Zabel,
	Jim Quinlan,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

Hi,

Thank you for sending the patches over!

> The Brcmstb PCIe RC uses a reset control "rescal" for certain chips.  This
[...]

A small suggestion: it would be nicer to mention "Broadcom STB" rather
than "Brcmstb" in the sentence above.

[...]
> +err1:
> +	reset_control_rearm(pcie->rescal);
> +err0:
>  	clk_disable_unprepare(pcie->clk);
>  	return ret;
[...]

A small nitpick.  Now that there are two labels on the error recovery
path, it might be better to name both of these labels a little bit
better.  Some examples from the PCI tree:

  error_clock_unprepare
  err_disable_clock
  err_disable_clk
  err_clk_disable
  
So it could be:

  err_reset:                                <-- or err_rearm or even err_reset_rearm, etc.
  	reset_control_rearm(pcie->rescal);
  err_disable_clk:
   	clk_disable_unprepare(pcie->clk);

What do you think?

Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert
  2021-03-08 19:50 ` [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert Jim Quinlan
  2021-03-08 23:35   ` Krzysztof Wilczyński
@ 2021-03-09  1:34   ` Bjorn Helgaas
  1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2021-03-09  1:34 UTC (permalink / raw)
  To: Jim Quinlan
  Cc: linux-pci, Nicolas Saenz Julienne, bcm-kernel-feedback-list,
	james.quinlan, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas,
	Florian Fainelli, Philipp Zabel, Jim Quinlan,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	open list

If you update this, please fix the s/nPCI: /PCI: / in the subject

On Mon, Mar 08, 2021 at 02:50:37PM -0500, Jim Quinlan wrote:
> The Brcmstb PCIe RC uses a reset control "rescal" for certain chips.  This
> reset implements a "pulse reset" so it matches more the reset/rearm
> calls instead of the deassert/assert calls.

You say "also" below, but the paragraph above doesn't tell us the
*first* thing this patch does.  It just tells us that some chips use
"rescal" and that "rescal" implements a "pulse reset".

I guess you're replacing reset_control_deassert() with
reset_control_reset(), and reset_control_assert() with
reset_control_rearm().

It's not obvious to me that those are equivalent or why it's safe to
do this for all chips, including those that don't use the "rescal"
(since it sounds like only certain chips have that).

> Also, add reset_control calls in suspend/resume functions.
> 
> 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..18f23cba7e3a 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 err0;
> +
>  	ret = brcm_phy_start(pcie);
>  	if (ret)
> -		goto err;
> +		goto err1;
>  
>  	/* 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 err1;
>  
>  	if (pcie->msi)
>  		brcm_msi_set_regs(pcie->msi);
>  
>  	return 0;
>  
> -err:
> +err1:
> +	reset_control_rearm(pcie->rescal);
> +err0:
>  	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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-09  1:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 19:50 [PATCH v4 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller Jim Quinlan
2021-03-08 19:50 ` [PATCH v4 2/2] nPCI: brcmstb: Use reset/rearm instead of deassert/assert Jim Quinlan
2021-03-08 23:35   ` Krzysztof Wilczyński
2021-03-09  1:34   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).