[PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[marek.vasut]

From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>

Reestablish the PCIe link very early in the resume process in case it
went down to prevent PCI accesses from hanging the bus. Such accesses
can happen early in the PCI resume process, as early as the
SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
driver resume_noirq() callback.

Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
Signed-off-by: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
[lorenzo.pieralisi@arm.com: reformatted commit log]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: stable@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Use BIT() macro for (1 << n)
    - Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
      add extra changes to this function anymore
    - Make resume_noirq return early and clean up parenthesis therein
V3: - Add missing PMSR register definition, dropped due to patch reshuffling
---
 drivers/pci/controller/pcie-rcar.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index c8febb009454..6a4e435bd35f 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -46,6 +46,7 @@
 
 /* Transfer control */
 #define PCIETCTLR		0x02000
+#define  DL_DOWN		BIT(3)
 #define  CFINIT			1
 #define PCIETSTR		0x02004
 #define  DATA_LINK_ACTIVE	1
@@ -94,6 +95,7 @@
 #define MACCTLR			0x011058
 #define  SPEED_CHANGE		BIT(24)
 #define  SCRAMBLE_DISABLE	BIT(27)
+#define PMSR			0x01105c
 #define MACS2R			0x011078
 #define MACCGSPSETR		0x011084
 #define  SPCNGRSN		BIT(31)
@@ -1130,6 +1132,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
+	platform_set_drvdata(pdev, pcie);
 
 	err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
 	if (err)
@@ -1221,10 +1224,28 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 	return err;
 }
 
+static int rcar_pcie_resume_noirq(struct device *dev)
+{
+	struct rcar_pcie *pcie = dev_get_drvdata(dev);
+
+	if (rcar_pci_read_reg(pcie, PMSR) &&
+	    !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
+		return 0;
+
+	/* Re-establish the PCIe link */
+	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
+	return rcar_pcie_wait_for_dl(pcie);
+}
+
+static const struct dev_pm_ops rcar_pcie_pm_ops = {
+	.resume_noirq = rcar_pcie_resume_noirq,
+};
+
 static struct platform_driver rcar_pcie_driver = {
 	.driver = {
 		.name = "rcar-pcie",
 		.of_match_table = rcar_pcie_of_match,
+		.pm = &rcar_pcie_pm_ops,
 		.suppress_bind_attrs = true,
 	},
 	.probe = rcar_pcie_probe,
-- 
2.20.1

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Geert Uytterhoeven]

On Mon, Mar 25, 2019 at 8:43 PM <marek.vasut@gmail.com> wrote:
> From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
>
> Reestablish the PCIe link very early in the resume process in case it
> went down to prevent PCI accesses from hanging the bus. Such accesses
> can happen early in the PCI resume process, as early as the
> SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> driver resume_noirq() callback.
>
> Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
> Signed-off-by: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> [lorenzo.pieralisi@arm.com: reformatted commit log]
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Lorenzo Pieralisi]

On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
> From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> 
> Reestablish the PCIe link very early in the resume process in case it
> went down to prevent PCI accesses from hanging the bus. Such accesses
> can happen early in the PCI resume process, as early as the
> SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> driver resume_noirq() callback.
> 
> Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
> Signed-off-by: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> [lorenzo.pieralisi@arm.com: reformatted commit log]
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: stable@vger.kernel.org
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Phil Edworthy <phil.edworthy@renesas.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: - Use BIT() macro for (1 << n)
>     - Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
>       add extra changes to this function anymore
>     - Make resume_noirq return early and clean up parenthesis therein
> V3: - Add missing PMSR register definition, dropped due to patch reshuffling
> ---
>  drivers/pci/controller/pcie-rcar.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)

Applied to pci/controller-fixes for one of the upcoming -rc*.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> index c8febb009454..6a4e435bd35f 100644
> --- a/drivers/pci/controller/pcie-rcar.c
> +++ b/drivers/pci/controller/pcie-rcar.c
> @@ -46,6 +46,7 @@
>  
>  /* Transfer control */
>  #define PCIETCTLR		0x02000
> +#define  DL_DOWN		BIT(3)
>  #define  CFINIT			1
>  #define PCIETSTR		0x02004
>  #define  DATA_LINK_ACTIVE	1
> @@ -94,6 +95,7 @@
>  #define MACCTLR			0x011058
>  #define  SPEED_CHANGE		BIT(24)
>  #define  SCRAMBLE_DISABLE	BIT(27)
> +#define PMSR			0x01105c
>  #define MACS2R			0x011078
>  #define MACCGSPSETR		0x011084
>  #define  SPCNGRSN		BIT(31)
> @@ -1130,6 +1132,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
>  
>  	pcie->dev = dev;
> +	platform_set_drvdata(pdev, pcie);
>  
>  	err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
>  	if (err)
> @@ -1221,10 +1224,28 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	return err;
>  }
>  
> +static int rcar_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rcar_pcie *pcie = dev_get_drvdata(dev);
> +
> +	if (rcar_pci_read_reg(pcie, PMSR) &&
> +	    !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
> +		return 0;
> +
> +	/* Re-establish the PCIe link */
> +	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
> +	return rcar_pcie_wait_for_dl(pcie);
> +}
> +
> +static const struct dev_pm_ops rcar_pcie_pm_ops = {
> +	.resume_noirq = rcar_pcie_resume_noirq,
> +};
> +
>  static struct platform_driver rcar_pcie_driver = {
>  	.driver = {
>  		.name = "rcar-pcie",
>  		.of_match_table = rcar_pcie_of_match,
> +		.pm = &rcar_pcie_pm_ops,
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe = rcar_pcie_probe,
> -- 
> 2.20.1
> 

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Bjorn Helgaas]

On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
> From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> 
> Reestablish the PCIe link very early in the resume process in case it
> went down to prevent PCI accesses from hanging the bus. Such accesses
> can happen early in the PCI resume process, as early as the
> SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> driver resume_noirq() callback.
> 
> Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")

I'm fine with the fix itself, but since e015f88c368d appeared more
than two years ago in v4.5, the justification for merging this after
the merge window is a little weak.

Is there a more recent change that exposed this problem?  The usual
situation is that we merged something during the v5.1 merge window
that caused a regression, and we're now fixing that before v5.1 final.

Bjorn

> Signed-off-by: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> [lorenzo.pieralisi@arm.com: reformatted commit log]
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: stable@vger.kernel.org
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Phil Edworthy <phil.edworthy@renesas.com>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: - Use BIT() macro for (1 << n)
>     - Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
>       add extra changes to this function anymore
>     - Make resume_noirq return early and clean up parenthesis therein
> V3: - Add missing PMSR register definition, dropped due to patch reshuffling
> ---
>  drivers/pci/controller/pcie-rcar.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> index c8febb009454..6a4e435bd35f 100644
> --- a/drivers/pci/controller/pcie-rcar.c
> +++ b/drivers/pci/controller/pcie-rcar.c
> @@ -46,6 +46,7 @@
>  
>  /* Transfer control */
>  #define PCIETCTLR		0x02000
> +#define  DL_DOWN		BIT(3)
>  #define  CFINIT			1
>  #define PCIETSTR		0x02004
>  #define  DATA_LINK_ACTIVE	1
> @@ -94,6 +95,7 @@
>  #define MACCTLR			0x011058
>  #define  SPEED_CHANGE		BIT(24)
>  #define  SCRAMBLE_DISABLE	BIT(27)
> +#define PMSR			0x01105c
>  #define MACS2R			0x011078
>  #define MACCGSPSETR		0x011084
>  #define  SPCNGRSN		BIT(31)
> @@ -1130,6 +1132,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
>  
>  	pcie->dev = dev;
> +	platform_set_drvdata(pdev, pcie);
>  
>  	err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
>  	if (err)
> @@ -1221,10 +1224,28 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	return err;
>  }
>  
> +static int rcar_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rcar_pcie *pcie = dev_get_drvdata(dev);
> +
> +	if (rcar_pci_read_reg(pcie, PMSR) &&
> +	    !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
> +		return 0;
> +
> +	/* Re-establish the PCIe link */
> +	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
> +	return rcar_pcie_wait_for_dl(pcie);
> +}
> +
> +static const struct dev_pm_ops rcar_pcie_pm_ops = {
> +	.resume_noirq = rcar_pcie_resume_noirq,
> +};
> +
>  static struct platform_driver rcar_pcie_driver = {
>  	.driver = {
>  		.name = "rcar-pcie",
>  		.of_match_table = rcar_pcie_of_match,
> +		.pm = &rcar_pcie_pm_ops,
>  		.suppress_bind_attrs = true,
>  	},
>  	.probe = rcar_pcie_probe,
> -- 
> 2.20.1
> 

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Lorenzo Pieralisi]

On Thu, Mar 28, 2019 at 09:18:22AM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
> > From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> > 
> > Reestablish the PCIe link very early in the resume process in case it
> > went down to prevent PCI accesses from hanging the bus. Such accesses
> > can happen early in the PCI resume process, as early as the
> > SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> > driver resume_noirq() callback.
> > 
> > Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
> 
> I'm fine with the fix itself, but since e015f88c368d appeared more
> than two years ago in v4.5, the justification for merging this after
> the merge window is a little weak.
> 
> Is there a more recent change that exposed this problem?  The usual
> situation is that we merged something during the v5.1 merge window
> that caused a regression, and we're now fixing that before v5.1 final.

I do not think that's urgent enough to merge it in one of the -rc* but I
won't speak for the authors - it will trickle back anyway through the
stable mechanism, I just queued it as a fix since that's how it
qualifies.

So postponing it to v5.2 is fine by me, just let me know how it is best
to handle it.

Thanks,
Lorenzo

> Bjorn
> 
> > Signed-off-by: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> > Signed-off-by: Gaku Inami <gaku.inami.xw@bp.renesas.com>
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> > [lorenzo.pieralisi@arm.com: reformatted commit log]
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Cc: stable@vger.kernel.org
> > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > Cc: Phil Edworthy <phil.edworthy@renesas.com>
> > Cc: Simon Horman <horms+renesas@verge.net.au>
> > Cc: Wolfram Sang <wsa@the-dreams.de>
> > Cc: linux-renesas-soc@vger.kernel.org
> > ---
> > V2: - Use BIT() macro for (1 << n)
> >     - Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
> >       add extra changes to this function anymore
> >     - Make resume_noirq return early and clean up parenthesis therein
> > V3: - Add missing PMSR register definition, dropped due to patch reshuffling
> > ---
> >  drivers/pci/controller/pcie-rcar.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
> > index c8febb009454..6a4e435bd35f 100644
> > --- a/drivers/pci/controller/pcie-rcar.c
> > +++ b/drivers/pci/controller/pcie-rcar.c
> > @@ -46,6 +46,7 @@
> >  
> >  /* Transfer control */
> >  #define PCIETCTLR		0x02000
> > +#define  DL_DOWN		BIT(3)
> >  #define  CFINIT			1
> >  #define PCIETSTR		0x02004
> >  #define  DATA_LINK_ACTIVE	1
> > @@ -94,6 +95,7 @@
> >  #define MACCTLR			0x011058
> >  #define  SPEED_CHANGE		BIT(24)
> >  #define  SCRAMBLE_DISABLE	BIT(27)
> > +#define PMSR			0x01105c
> >  #define MACS2R			0x011078
> >  #define MACCGSPSETR		0x011084
> >  #define  SPCNGRSN		BIT(31)
> > @@ -1130,6 +1132,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> >  	pcie = pci_host_bridge_priv(bridge);
> >  
> >  	pcie->dev = dev;
> > +	platform_set_drvdata(pdev, pcie);
> >  
> >  	err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
> >  	if (err)
> > @@ -1221,10 +1224,28 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> >  	return err;
> >  }
> >  
> > +static int rcar_pcie_resume_noirq(struct device *dev)
> > +{
> > +	struct rcar_pcie *pcie = dev_get_drvdata(dev);
> > +
> > +	if (rcar_pci_read_reg(pcie, PMSR) &&
> > +	    !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
> > +		return 0;
> > +
> > +	/* Re-establish the PCIe link */
> > +	rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
> > +	return rcar_pcie_wait_for_dl(pcie);
> > +}
> > +
> > +static const struct dev_pm_ops rcar_pcie_pm_ops = {
> > +	.resume_noirq = rcar_pcie_resume_noirq,
> > +};
> > +
> >  static struct platform_driver rcar_pcie_driver = {
> >  	.driver = {
> >  		.name = "rcar-pcie",
> >  		.of_match_table = rcar_pcie_of_match,
> > +		.pm = &rcar_pcie_pm_ops,
> >  		.suppress_bind_attrs = true,
> >  	},
> >  	.probe = rcar_pcie_probe,
> > -- 
> > 2.20.1
> > 

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Geert Uytterhoeven]

Hi Bjorn,

On Thu, Mar 28, 2019 at 3:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
> > From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> >
> > Reestablish the PCIe link very early in the resume process in case it
> > went down to prevent PCI accesses from hanging the bus. Such accesses
> > can happen early in the PCI resume process, as early as the
> > SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> > driver resume_noirq() callback.
> >
> > Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
>
> I'm fine with the fix itself, but since e015f88c368d appeared more
> than two years ago in v4.5, the justification for merging this after
> the merge window is a little weak.

V1 of this fix was posted in November 2017, but IIRC, the series became
the target of some bike-shedding...

> Is there a more recent change that exposed this problem?  The usual
> situation is that we merged something during the v5.1 merge window
> that caused a regression, and we're now fixing that before v5.1 final.

There are several reasons most people couldn't even run suspend/resume
cycles on their systems:
  1. Early releases of the affected boards came with firmware revisions with
     non-functional PSCI system suspend,
  2. Preparing the PMIC for suspend required ugly assistance from i2cset
     in userspace, until the Linux driver learned to take care of that itself
     in v4.18/v4.19.

I guess the fix can survive postponing to v5.2, though...

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Bjorn Helgaas]

On Thu, Mar 28, 2019 at 03:59:11PM +0100, Geert Uytterhoeven wrote:
> On Thu, Mar 28, 2019 at 3:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
> > > From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
> > >
> > > Reestablish the PCIe link very early in the resume process in case it
> > > went down to prevent PCI accesses from hanging the bus. Such accesses
> > > can happen early in the PCI resume process, as early as the
> > > SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
> > > driver resume_noirq() callback.
> > >
> > > Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
> >
> > I'm fine with the fix itself, but since e015f88c368d appeared more
> > than two years ago in v4.5, the justification for merging this after
> > the merge window is a little weak.
> 
> V1 of this fix was posted in November 2017, but IIRC, the series became
> the target of some bike-shedding...
> 
> > Is there a more recent change that exposed this problem?  The usual
> > situation is that we merged something during the v5.1 merge window
> > that caused a regression, and we're now fixing that before v5.1 final.
> 
> There are several reasons most people couldn't even run suspend/resume
> cycles on their systems:
>   1. Early releases of the affected boards came with firmware revisions with
>      non-functional PSCI system suspend,
>   2. Preparing the PMIC for suspend required ugly assistance from i2cset
>      in userspace, until the Linux driver learned to take care of that itself
>      in v4.18/v4.19.
> 
> I guess the fix can survive postponing to v5.2, though...

Ok, I'll merge it to -next for v5.2, thanks.

Bjorn

Re: [PATCH V3] PCI: rcar: Add the initialization of PCIe link in resume_noirq()

[Marek Vasut]

On 3/28/19 7:59 PM, Bjorn Helgaas wrote:
> On Thu, Mar 28, 2019 at 03:59:11PM +0100, Geert Uytterhoeven wrote:
>> On Thu, Mar 28, 2019 at 3:18 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> On Mon, Mar 25, 2019 at 08:43:19PM +0100, marek.vasut@gmail.com wrote:
>>>> From: Kazufumi Ikeda <kaz-ikeda@xc.jp.nec.com>
>>>>
>>>> Reestablish the PCIe link very early in the resume process in case it
>>>> went down to prevent PCI accesses from hanging the bus. Such accesses
>>>> can happen early in the PCI resume process, as early as the
>>>> SUSPEND_RESUME_NOIRQ step, thus the link must be reestablished in the
>>>> driver resume_noirq() callback.
>>>>
>>>> Fixes: e015f88c368d ("PCI: rcar: Add support for R-Car H3 to pcie-rcar")
>>>
>>> I'm fine with the fix itself, but since e015f88c368d appeared more
>>> than two years ago in v4.5, the justification for merging this after
>>> the merge window is a little weak.
>>
>> V1 of this fix was posted in November 2017, but IIRC, the series became
>> the target of some bike-shedding...
>>
>>> Is there a more recent change that exposed this problem?  The usual
>>> situation is that we merged something during the v5.1 merge window
>>> that caused a regression, and we're now fixing that before v5.1 final.
>>
>> There are several reasons most people couldn't even run suspend/resume
>> cycles on their systems:
>>   1. Early releases of the affected boards came with firmware revisions with
>>      non-functional PSCI system suspend,
>>   2. Preparing the PMIC for suspend required ugly assistance from i2cset
>>      in userspace, until the Linux driver learned to take care of that itself
>>      in v4.18/v4.19.
>>
>> I guess the fix can survive postponing to v5.2, though...
> 
> Ok, I'll merge it to -next for v5.2, thanks.

ACK.

-- 
Best regards,
Marek Vasut