Re: [PATCH v2 2/2] PCI: histb: add an optional regulator for PCIe port power control

From: Bjorn Helgaas <helgaas@kernel.org>
To: Shawn Guo <shawn.guo@linaro.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh+dt@kernel.org>,
	Fabio Estevam <festevam@gmail.com>,
	Jianguo Sun <sunjianguo1@huawei.com>,
	linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	project-aspen-dev@linaro.org
Subject: Re: [PATCH v2 2/2] PCI: histb: add an optional regulator for PCIe port power control
Date: Thu, 1 Mar 2018 09:09:11 -0600	[thread overview]
Message-ID: <20180301150910.GB13722@bhelgaas-glaptop.roam.corp.google.com> (raw)
In-Reply-To: <1516722337-1533-3-git-send-email-shawn.guo@linaro.org>

On Tue, Jan 23, 2018 at 11:45:37PM +0800, Shawn Guo wrote:
> The power supply to PCIe port are often controlled by GPIO on some board
> designs.  Let's add an optional regulator which can be backed by GPIO to
> control the power.
> 
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  .../bindings/pci/hisilicon-histb-pcie.txt           |  1 +
>  drivers/pci/dwc/pcie-histb.c                        | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> index c84bc027930b..f995664e6d7f 100644
> --- a/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> @@ -34,6 +34,7 @@ Required properties
>  
>  Optional properties:
>  - reset-gpios: The gpio to generate PCIe PERST# assert and deassert signal.
> +- vpcie-supply: Should specify the regulator in charge of PCIe port power.

s/Should specify the/The/

>  - phys: List of phandle and phy mode specifier, should be 0.
>  - phy-names: Must be "phy".
>  
> diff --git a/drivers/pci/dwc/pcie-histb.c b/drivers/pci/dwc/pcie-histb.c
> index 6395394be5b4..8eb3028432b3 100644
> --- a/drivers/pci/dwc/pcie-histb.c
> +++ b/drivers/pci/dwc/pcie-histb.c
> @@ -64,6 +64,7 @@ struct histb_pcie {
>  	struct reset_control *bus_reset;
>  	void __iomem *ctrl;
>  	int reset_gpio;
> +	struct regulator *vpcie;
>  };
>  
>  static u32 histb_pcie_readl(struct histb_pcie *histb_pcie, u32 reg)
> @@ -230,6 +231,9 @@ static void histb_pcie_host_disable(struct histb_pcie *hipcie)
>  
>  	if (gpio_is_valid(hipcie->reset_gpio))
>  		gpio_set_value_cansleep(hipcie->reset_gpio, 0);
> +
> +	if (hipcie->vpcie)
> +		regulator_disable(hipcie->vpcie);
>  }
>  
>  static int histb_pcie_host_enable(struct pcie_port *pp)
> @@ -240,6 +244,14 @@ static int histb_pcie_host_enable(struct pcie_port *pp)
>  	int ret;
>  
>  	/* power on PCIe device if have */
> +	if (hipcie->vpcie) {
> +		ret = regulator_enable(hipcie->vpcie);
> +		if (ret) {
> +			dev_err(dev, "failed to enable regulator: %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
>  	if (gpio_is_valid(hipcie->reset_gpio))
>  		gpio_set_value_cansleep(hipcie->reset_gpio, 1);
>  
> @@ -285,6 +297,8 @@ static int histb_pcie_host_enable(struct pcie_port *pp)
>  err_sys_clk:
>  	clk_disable_unprepare(hipcie->bus_clk);
>  err_bus_clk:
> +	if (hipcie->vpcie)
> +		regulator_disable(hipcie->vpcie);
>  
>  	return ret;
>  }
> @@ -334,6 +348,13 @@ static int histb_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pci->dbi_base);
>  	}
>  
> +	hipcie->vpcie = devm_regulator_get_optional(dev, "vpcie");
> +	if (IS_ERR(hipcie->vpcie)) {
> +		if (PTR_ERR(hipcie->vpcie) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		hipcie->vpcie = NULL;
> +	}
> +
>  	hipcie->reset_gpio = of_get_named_gpio_flags(np,
>  				"reset-gpios", 0, &of_flags);
>  	if (of_flags & OF_GPIO_ACTIVE_LOW)
> -- 
> 1.9.1
>