Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
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 > Cc: Geert Uytterhoeven > Cc: Lorenzo Pieralisi > Cc: Phil Edworthy > Cc: Simon Horman > Cc: Wolfram Sang > 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 >
Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
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 >> Cc: Geert Uytterhoeven >> Cc: Lorenzo Pieralisi >> Cc: Phil Edworthy >> Cc: Simon Horman >> Cc: Wolfram Sang >> Cc: linux-renesas-soc@vger.kernel.org >> Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") > > Acked-by: Simon Horman 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. -- Best regards, Marek Vasut
Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
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 > Cc: Geert Uytterhoeven > Cc: Lorenzo Pieralisi > Cc: Phil Edworthy > Cc: Simon Horman > Cc: Wolfram Sang > Cc: linux-renesas-soc@vger.kernel.org > Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support") Acked-by: Simon Horman > --- > 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 >
Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
On 05/25/2018 12:09 PM, Geert Uytterhoeven wrote: > On Thu, May 24, 2018 at 4:36 PM, 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 > > Reviewed-by: Geert Uytterhoeven > > 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. -- Best regards, Marek Vasut
Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
On Thu, May 24, 2018 at 4:36 PM, 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 Reviewed-by: Geert Uytterhoeven 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 -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
[PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath
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 Cc: Geert Uytterhoeven Cc: Lorenzo Pieralisi Cc: Phil Edworthy Cc: Simon Horman Cc: Wolfram Sang 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(-) 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