| Message ID | 20180524143624.26718-7-marek.vasut+renesas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, May 24, 2018 at 4:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > If anything fails past phy_init_fn() and the system is a Gen3 with > a PHY, the PHY will be left on and inited. This is caused by the > phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() > function, which starts the PHY, yet has no counterpart in the failpath. > Add that counterpart. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Note that there's another unhandled failure mode: rcar_pcie_phy_init_gen3() doesn't call phy_exit() if phy_power_on() fails. Gr{oetje,eeting}s, Geert
On 05/25/2018 12:09 PM, Geert Uytterhoeven wrote: > On Thu, May 24, 2018 at 4:36 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> If anything fails past phy_init_fn() and the system is a Gen3 with >> a PHY, the PHY will be left on and inited. This is caused by the >> phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() >> function, which starts the PHY, yet has no counterpart in the failpath. >> Add that counterpart. >> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Note that there's another unhandled failure mode: rcar_pcie_phy_init_gen3() > doesn't call phy_exit() if phy_power_on() fails. Should be fixed by subsequent patch, thanks.
On Thu, May 24, 2018 at 04:36:24PM +0200, Marek Vasut wrote: > If anything fails past phy_init_fn() and the system is a Gen3 with > a PHY, the PHY will be left on and inited. This is caused by the > phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() > function, which starts the PHY, yet has no counterpart in the failpath. > Add that counterpart. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > 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 > Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") Acked-by: Simon Horman <horms+renesas@verge.net.au> > --- > V4: New patch > --- > drivers/pci/host/pcie-rcar.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 636c3c5095d2..695781934f0a 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -1163,7 +1163,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) > if (rcar_pcie_hw_init(pcie)) { > dev_info(dev, "PCIe link down\n"); > err = -ENODEV; > - goto err_clk_disable; > + goto err_phy_shutdown; > } > > data = rcar_pci_read_reg(pcie, MACSR); > @@ -1175,7 +1175,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) > dev_err(dev, > "failed to enable MSI support: %d\n", > err); > - goto err_clk_disable; > + goto err_phy_shutdown; > } > } > > @@ -1189,6 +1189,12 @@ static int rcar_pcie_probe(struct platform_device *pdev) > if (IS_ENABLED(CONFIG_PCI_MSI)) > rcar_pcie_teardown_msi(pcie); > > +err_phy_shutdown: > + if (pcie->phy) { > + phy_power_off(pcie->phy); > + phy_exit(pcie->phy); > + } > + > err_clk_disable: > clk_disable_unprepare(pcie->bus_clk); > > -- > 2.16.2 >
On 05/28/2018 10:48 AM, Simon Horman wrote: > On Thu, May 24, 2018 at 04:36:24PM +0200, Marek Vasut wrote: >> If anything fails past phy_init_fn() and the system is a Gen3 with >> a PHY, the PHY will be left on and inited. This is caused by the >> phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() >> function, which starts the PHY, yet has no counterpart in the failpath. >> Add that counterpart. >> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >> Cc: Geert Uytterhoeven <geert+renesas@glider.be> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >> 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 >> Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") > > Acked-by: Simon Horman <horms+renesas@verge.net.au> While I see the first 5 in today's next, I don't see this one there, so it'd be nice to add it too.
On Thu, May 24, 2018 at 04:36:24PM +0200, Marek Vasut wrote: > If anything fails past phy_init_fn() and the system is a Gen3 with > a PHY, the PHY will be left on and inited. This is caused by the > phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() > function, which starts the PHY, yet has no counterpart in the failpath. > Add that counterpart. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > 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 > Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") > --- > V4: New patch > --- > drivers/pci/host/pcie-rcar.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) Applied to pci/controller-fixes to be tentatively merged at -rc4, thanks. Lorenzo > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > index 636c3c5095d2..695781934f0a 100644 > --- a/drivers/pci/host/pcie-rcar.c > +++ b/drivers/pci/host/pcie-rcar.c > @@ -1163,7 +1163,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) > if (rcar_pcie_hw_init(pcie)) { > dev_info(dev, "PCIe link down\n"); > err = -ENODEV; > - goto err_clk_disable; > + goto err_phy_shutdown; > } > > data = rcar_pci_read_reg(pcie, MACSR); > @@ -1175,7 +1175,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) > dev_err(dev, > "failed to enable MSI support: %d\n", > err); > - goto err_clk_disable; > + goto err_phy_shutdown; > } > } > > @@ -1189,6 +1189,12 @@ static int rcar_pcie_probe(struct platform_device *pdev) > if (IS_ENABLED(CONFIG_PCI_MSI)) > rcar_pcie_teardown_msi(pcie); > > +err_phy_shutdown: > + if (pcie->phy) { > + phy_power_off(pcie->phy); > + phy_exit(pcie->phy); > + } > + > err_clk_disable: > clk_disable_unprepare(pcie->bus_clk); > > -- > 2.16.2 >
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c index 636c3c5095d2..695781934f0a 100644 --- a/drivers/pci/host/pcie-rcar.c +++ b/drivers/pci/host/pcie-rcar.c @@ -1163,7 +1163,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) if (rcar_pcie_hw_init(pcie)) { dev_info(dev, "PCIe link down\n"); err = -ENODEV; - goto err_clk_disable; + goto err_phy_shutdown; } data = rcar_pci_read_reg(pcie, MACSR); @@ -1175,7 +1175,7 @@ static int rcar_pcie_probe(struct platform_device *pdev) dev_err(dev, "failed to enable MSI support: %d\n", err); - goto err_clk_disable; + goto err_phy_shutdown; } } @@ -1189,6 +1189,12 @@ static int rcar_pcie_probe(struct platform_device *pdev) if (IS_ENABLED(CONFIG_PCI_MSI)) rcar_pcie_teardown_msi(pcie); +err_phy_shutdown: + if (pcie->phy) { + phy_power_off(pcie->phy); + phy_exit(pcie->phy); + } + err_clk_disable: clk_disable_unprepare(pcie->bus_clk);
If anything fails past phy_init_fn() and the system is a Gen3 with a PHY, the PHY will be left on and inited. This is caused by the phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3() function, which starts the PHY, yet has no counterpart in the failpath. Add that counterpart. Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> 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 Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") --- V4: New patch --- drivers/pci/host/pcie-rcar.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)