diff mbox series

[v2,2/6] PCI: brcmstb: Add control of EP voltage regulator(s)

Message ID 20201130211145.3012-3-james.quinlan@broadcom.com
State New
Headers show
Series brcmstb: add EP regulators and panic handler | expand

Commit Message

Jim Quinlan Nov. 30, 2020, 9:11 p.m. UTC 
Control of EP regulators by the RC is needed because of the chicken-and-egg
situation: although the regulator is "owned" by the EP and would be best
handled on its driver, the EP cannot be discovered and probed unless its
regulator is already turned on.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
---
 drivers/pci/controller/pcie-brcmstb.c | 38 ++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

Comments

Florian Fainelli Nov. 30, 2020, 9:32 p.m. UTC | #1 
On 11/30/2020 1:11 PM, Jim Quinlan wrote:
> Control of EP regulators by the RC is needed because of the chicken-and-egg
> situation: although the regulator is "owned" by the EP and would be best
> handled on its driver, the EP cannot be discovered and probed unless its
> regulator is already turned on.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>  drivers/pci/controller/pcie-brcmstb.c | 38 ++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index bea86899bd5d..9d4ac42b3bee 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/pci.h>
>  #include <linux/printk.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> @@ -210,6 +211,10 @@ enum pcie_type {
>  	BCM2711,
>  };
>  
> +static const char * const ep_regulator_names[] = {
> +	"vpcie12v", "vpcie3v3", "vpcie1v8", "vpcie0v9",

Only if you need to re-spin this patch series, I would be keen on
putting each string on its own line, that way when adding a subsequent
regulator name, it is just a matter of an one line d

> +};
> +
>  struct pcie_cfg_data {
>  	const int *offsets;
>  	const enum pcie_type type;
> @@ -287,8 +292,25 @@ struct brcm_pcie {
>  	u32			hw_rev;
>  	void			(*perst_set)(struct brcm_pcie *pcie, u32 val);
>  	void			(*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(ep_regulator_names)];
>  };
>  
> +static void brcm_set_regulators(struct brcm_pcie *pcie, bool on)
> +{
> +	struct device *dev = pcie->dev;
> +	int ret;
> +
> +	if (on)
> +		ret = regulator_bulk_enable(ARRAY_SIZE(ep_regulator_names),
> +					    pcie->supplies);
> +	else
> +		ret = regulator_bulk_disable(ARRAY_SIZE(ep_regulator_names),
> +					     pcie->supplies);
> +	if (ret)
> +		dev_err(dev, "failed to %s EP regulators\n",
> +			on ? "enable" : "disable");

Should not you propagate the return value to the caller here?
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index bea86899bd5d..9d4ac42b3bee 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -23,6 +23,7 @@ 
 #include <linux/of_platform.h>
 #include <linux/pci.h>
 #include <linux/printk.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
@@ -210,6 +211,10 @@  enum pcie_type {
 	BCM2711,
 };
 
+static const char * const ep_regulator_names[] = {
+	"vpcie12v", "vpcie3v3", "vpcie1v8", "vpcie0v9",
+};
+
 struct pcie_cfg_data {
 	const int *offsets;
 	const enum pcie_type type;
@@ -287,8 +292,25 @@  struct brcm_pcie {
 	u32			hw_rev;
 	void			(*perst_set)(struct brcm_pcie *pcie, u32 val);
 	void			(*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
+	struct regulator_bulk_data supplies[ARRAY_SIZE(ep_regulator_names)];
 };
 
+static void brcm_set_regulators(struct brcm_pcie *pcie, bool on)
+{
+	struct device *dev = pcie->dev;
+	int ret;
+
+	if (on)
+		ret = regulator_bulk_enable(ARRAY_SIZE(ep_regulator_names),
+					    pcie->supplies);
+	else
+		ret = regulator_bulk_disable(ARRAY_SIZE(ep_regulator_names),
+					     pcie->supplies);
+	if (ret)
+		dev_err(dev, "failed to %s EP regulators\n",
+			on ? "enable" : "disable");
+}
+
 /*
  * This is to convert the size of the inbound "BAR" region to the
  * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.SIZE
@@ -1139,6 +1161,7 @@  static int brcm_pcie_suspend(struct device *dev)
 	brcm_pcie_turn_off(pcie);
 	ret = brcm_phy_stop(pcie);
 	clk_disable_unprepare(pcie->clk);
+	brcm_set_regulators(pcie, false);
 
 	return ret;
 }
@@ -1151,6 +1174,7 @@  static int brcm_pcie_resume(struct device *dev)
 	int ret;
 
 	base = pcie->base;
+	brcm_set_regulators(pcie, true);
 	clk_prepare_enable(pcie->clk);
 
 	ret = brcm_phy_start(pcie);
@@ -1189,6 +1213,7 @@  static void __brcm_pcie_remove(struct brcm_pcie *pcie)
 	brcm_phy_stop(pcie);
 	reset_control_assert(pcie->rescal);
 	clk_disable_unprepare(pcie->clk);
+	brcm_set_regulators(pcie, false);
 }
 
 static int brcm_pcie_remove(struct platform_device *pdev)
@@ -1218,7 +1243,7 @@  static int brcm_pcie_probe(struct platform_device *pdev)
 	struct pci_host_bridge *bridge;
 	const struct pcie_cfg_data *data;
 	struct brcm_pcie *pcie;
-	int ret;
+	int ret, i;
 
 	bridge = devm_pci_alloc_host_bridge(&pdev->dev, sizeof(*pcie));
 	if (!bridge)
@@ -1246,6 +1271,16 @@  static int brcm_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(pcie->clk))
 		return PTR_ERR(pcie->clk);
 
+	for (i = 0; i < ARRAY_SIZE(ep_regulator_names); i++)
+		pcie->supplies[i].supply = ep_regulator_names[i];
+
+	ret = devm_regulator_bulk_get(pcie->dev, ARRAY_SIZE(ep_regulator_names),
+				      pcie->supplies);
+	if (ret) {
+		dev_err(pcie->dev, "failed to get regulators\n");
+		return ret;
+	}
+
 	ret = of_pci_get_max_link_speed(np);
 	pcie->gen = (ret < 0) ? 0 : ret;
 
@@ -1273,6 +1308,7 @@  static int brcm_pcie_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	brcm_set_regulators(pcie, true);
 	ret = brcm_pcie_setup(pcie);
 	if (ret)
 		goto fail;