[RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Jim Quinlan]

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


[1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
    found at git://git.pengutronix.de/git/pza/linux.git
    branch reset/shared-retrigger

Jim Quinlan (2):
  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 +++++++----
 2 files changed, 36 insertions(+), 29 deletions(-)

-- 
2.17.1

[RESEND PATCH v3 2/2] PCI: brcmstb: use reset/rearm instead of deassert/assert

[Jim Quinlan]

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.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.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 d41257f43a8f..9f190db4cfcf 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1127,6 +1127,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;
@@ -1142,9 +1143,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);
@@ -1159,14 +1164,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;
 }
@@ -1176,7 +1183,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);
 }
 
@@ -1251,13 +1258,13 @@ static int brcm_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pcie->rescal);
 	}
 
-	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: [RESEND PATCH v3 2/2] PCI: brcmstb: use reset/rearm instead of deassert/assert

[Florian Fainelli]

On 12/16/20 1:41 PM, 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.
> 
> Also, add reset_control calls in suspend/resume functions.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>

This should probably deserve a Fixes tag as well:

Fixes: 740d6c3708a9 ("PCI: brcmstb: Add control of rescal reset")
Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks!
-- 
Florian

Re: [RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Florian Fainelli]

On 12/16/2020 1:41 PM, Jim Quinlan wrote:
> 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
> 
> 
> [1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
>     found at git://git.pengutronix.de/git/pza/linux.git
>     branch reset/shared-retrigger

The changes in that branch above have now landed in Linus' tree with:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=557acb3d2cd9c82de19f944f6cc967a347735385

It would be good if we could get both patches applied via the same tree
or within the same cycle to avoid having either PCIe or SATA broken on
these platforms.

Thanks!
-- 
Florian

Re: [RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Florian Fainelli]

On 12/23/20 4:05 PM, Florian Fainelli wrote:
> 
> 
> On 12/16/2020 1:41 PM, Jim Quinlan wrote:
>> 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
>>
>>
>> [1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
>>     found at git://git.pengutronix.de/git/pza/linux.git
>>     branch reset/shared-retrigger
> 
> The changes in that branch above have now landed in Linus' tree with:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=557acb3d2cd9c82de19f944f6cc967a347735385
> 
> It would be good if we could get both patches applied via the same tree
> or within the same cycle to avoid having either PCIe or SATA broken on
> these platforms.

Ping? Can someone apply those patches if you are happy with them? Thank you.
-- 
Florian

Re: [RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Florian Fainelli]

On 1/5/21 1:22 PM, Florian Fainelli wrote:
> On 12/23/20 4:05 PM, Florian Fainelli wrote:
>>
>>
>> On 12/16/2020 1:41 PM, Jim Quinlan wrote:
>>> 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
>>>
>>>
>>> [1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
>>>     found at git://git.pengutronix.de/git/pza/linux.git
>>>     branch reset/shared-retrigger
>>
>> The changes in that branch above have now landed in Linus' tree with:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=557acb3d2cd9c82de19f944f6cc967a347735385
>>
>> It would be good if we could get both patches applied via the same tree
>> or within the same cycle to avoid having either PCIe or SATA broken on
>> these platforms.
> 
> Ping? Can someone apply those patches if you are happy with them? Thank you.

Ping? Can we review and ideally also apply these patches? Thanks
-- 
Florian

Re: [RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Florian Fainelli]

On 1/14/2021 12:46 PM, Florian Fainelli wrote:
> On 1/5/21 1:22 PM, Florian Fainelli wrote:
>> On 12/23/20 4:05 PM, Florian Fainelli wrote:
>>>
>>>
>>> On 12/16/2020 1:41 PM, Jim Quinlan wrote:
>>>> 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
>>>>
>>>>
>>>> [1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
>>>>     found at git://git.pengutronix.de/git/pza/linux.git
>>>>     branch reset/shared-retrigger
>>>
>>> The changes in that branch above have now landed in Linus' tree with:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=557acb3d2cd9c82de19f944f6cc967a347735385
>>>
>>> It would be good if we could get both patches applied via the same tree
>>> or within the same cycle to avoid having either PCIe or SATA broken on
>>> these platforms.
>>
>> Ping? Can someone apply those patches if you are happy with them? Thank you.
> 
> Ping? Can we review and ideally also apply these patches? Thanks

Is there something going on preventing these patches from being reviewed
and/or applied?
-- 
Florian

Re: [RESEND PATCH v3 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller

[Bjorn Helgaas]

On Thu, Jan 21, 2021 at 12:48:07PM -0800, Florian Fainelli wrote:
> On 1/14/2021 12:46 PM, Florian Fainelli wrote:
> > On 1/5/21 1:22 PM, Florian Fainelli wrote:
> >> On 12/23/20 4:05 PM, Florian Fainelli wrote:
> >>>
> >>>
> >>> On 12/16/2020 1:41 PM, Jim Quinlan wrote:
> >>>> 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
> >>>>
> >>>>
> >>>> [1] Applied commit "reset: make shared pulsed reset controls re-triggerable"
> >>>>     found at git://git.pengutronix.de/git/pza/linux.git
> >>>>     branch reset/shared-retrigger
> >>>
> >>> The changes in that branch above have now landed in Linus' tree with:
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=557acb3d2cd9c82de19f944f6cc967a347735385
> >>>
> >>> It would be good if we could get both patches applied via the same tree
> >>> or within the same cycle to avoid having either PCIe or SATA broken on
> >>> these platforms.
> >>
> >> Ping? Can someone apply those patches if you are happy with them? Thank you.
> > 
> > Ping? Can we review and ideally also apply these patches? Thanks
> 
> Is there something going on preventing these patches from being reviewed
> and/or applied?

It mentions a dependency, which makes it harder to apply.  I see that
dependency seems to have been applied, so maybe post an updated
version of the series, including both patches.

The series touches both ATA and PCI, so not immediately obvious where
it should go.  It looks like the ATA part is bigger, but only 2/2 went
to linux-pci, so it's harder to figure this out than it should be.  I
poked around a bit on lore, but I can't find 1/2 at all:

  https://lore.kernel.org/linux-arm-kernel/20201216214106.32851-1-james.quinlan@broadcom.com/

Maybe also include Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
directly in "To:".  I know I look at stuff addressed directly to me
first; the "CC:" flood gets overwhelming fast.  I assume Lorenzo will
look for at least an ack from Nicolas before doing anything.

If you repost, it's nice if you match existing style, e.g.,

  - PCI: brcmstb: use reset/rearm instead of deassert/assert
  + PCI: brcmstb: Use reset/rearm instead of deassert/assert

Bjorn