[PATCH v2] pci: ftpci100: Make clocks compulsory

[PATCH v2] pci: ftpci100: Make clocks compulsory

[Linus Walleij]

This fixes a regression on the FTPCI100 PCI driver.

When the clock controller was augmented to probe clocks
from the platform device except to the most basic clocks
pertaining to timers, clocks may return an error pointer
containing -EPROBE_DEFER.

This hit the PCI driver which would try to continue
without the clocks, but the actual clocks appeared later,
so we did not pick them up properly, and in the end of
the boot, these clocks get gated off if no users register,
so the PCI host became numbed off and silently nonworking.

The clocks for this PCI bus are defined properly for all
platforms implementing it, so we should make them compulsory
before new users arrive so we have our resources under
control.

This makes the driver bail out of probe with any error
code from the clock, including -EPROBE_DEFER so that we
again have a clean boot.

Fixes: cea186ac1e45 ("pci: ftpci100: Add clock handling")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Inadvertedly managed to switch PTR_ERR() for ERR_PTR()
  in the code, and the compile screamed.
---
 drivers/pci/host/pci-ftpci100.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index d3bf153a2ab2..db31ab21884e 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -463,24 +463,20 @@ static int faraday_pci_probe(struct platform_device *pdev)
 
 	/* Retrieve and enable optional clocks */
 	clk = devm_clk_get(dev, "PCLK");
-	if (IS_ERR(clk)) {
-		dev_err(dev, "no PCLK available\n");
-	} else {
-		ret = clk_prepare_enable(clk);
-		if (ret) {
-			dev_err(dev, "could not prepare PCLK\n");
-			return ret;
-		}
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(dev, "could not prepare PCLK\n");
+		return ret;
 	}
 	p->bus_clk = devm_clk_get(dev, "PCICLK");
-	if (IS_ERR(p->bus_clk)) {
-		dev_err(dev, "no PCICLK available\n");
-	} else {
-		ret = clk_prepare_enable(p->bus_clk);
-		if (ret) {
-			dev_err(dev, "could not prepare PCICLK\n");
-			return ret;
-		}
+	if (IS_ERR(p->bus_clk))
+		return PTR_ERR(clk);
+	ret = clk_prepare_enable(p->bus_clk);
+	if (ret) {
+		dev_err(dev, "could not prepare PCICLK\n");
+		return ret;
 	}
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
2.9.4

Re: [PATCH v2] pci: ftpci100: Make clocks compulsory

[Bjorn Helgaas]

On Wed, Jun 21, 2017 at 10:57:30AM +0200, Linus Walleij wrote:
> This fixes a regression on the FTPCI100 PCI driver.
> 
> When the clock controller was augmented to probe clocks
> from the platform device except to the most basic clocks
> pertaining to timers, clocks may return an error pointer
> containing -EPROBE_DEFER.
> 
> This hit the PCI driver which would try to continue
> without the clocks, but the actual clocks appeared later,
> so we did not pick them up properly, and in the end of
> the boot, these clocks get gated off if no users register,
> so the PCI host became numbed off and silently nonworking.
> 
> The clocks for this PCI bus are defined properly for all
> platforms implementing it, so we should make them compulsory
> before new users arrive so we have our resources under
> control.
> 
> This makes the driver bail out of probe with any error
> code from the clock, including -EPROBE_DEFER so that we
> again have a clean boot.
> 
> Fixes: cea186ac1e45 ("pci: ftpci100: Add clock handling")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I folded this into the "pci: ftpci100: Add clock handling" commit,
thanks!

> ---
> ChangeLog v1->v2:
> - Inadvertedly managed to switch PTR_ERR() for ERR_PTR()
>   in the code, and the compile screamed.
> ---
>  drivers/pci/host/pci-ftpci100.c | 28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
> index d3bf153a2ab2..db31ab21884e 100644
> --- a/drivers/pci/host/pci-ftpci100.c
> +++ b/drivers/pci/host/pci-ftpci100.c
> @@ -463,24 +463,20 @@ static int faraday_pci_probe(struct platform_device *pdev)
>  
>  	/* Retrieve and enable optional clocks */
>  	clk = devm_clk_get(dev, "PCLK");
> -	if (IS_ERR(clk)) {
> -		dev_err(dev, "no PCLK available\n");
> -	} else {
> -		ret = clk_prepare_enable(clk);
> -		if (ret) {
> -			dev_err(dev, "could not prepare PCLK\n");
> -			return ret;
> -		}
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		dev_err(dev, "could not prepare PCLK\n");
> +		return ret;
>  	}
>  	p->bus_clk = devm_clk_get(dev, "PCICLK");
> -	if (IS_ERR(p->bus_clk)) {
> -		dev_err(dev, "no PCICLK available\n");
> -	} else {
> -		ret = clk_prepare_enable(p->bus_clk);
> -		if (ret) {
> -			dev_err(dev, "could not prepare PCICLK\n");
> -			return ret;
> -		}
> +	if (IS_ERR(p->bus_clk))
> +		return PTR_ERR(clk);
> +	ret = clk_prepare_enable(p->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "could not prepare PCICLK\n");
> +		return ret;
>  	}
>  
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -- 
> 2.9.4
>