[PATCH v2 0/2] PCI: mediatek-gen3: Support controlling power and

[PATCH v2 0/2] PCI: mediatek-gen3: Support controlling power and

[Jian Yang]

From: "jian.yang" <jian.yang@mediatek.com>

These series patches add support for controlling power supplies and reset
GPIO of a downstream component in Mediatek's PCIe GEN3 controller driver.

Changes in v2:
1. Remove an unnecessary property in dt-bindings file.
2. Use the flag 'GPIOD_OUT_LOW' to set initial state of a downstream
component's reset GPIO.
3. Keep downstream component powered on in suspend state if it need to
wakeup the system.

jian.yang (2):
  dt-bindings: PCI: mediatek-gen3: Add support for controlling power and
    reset
  PCI: mediatek-gen3: Add power and reset control feature for downstream
    component

 .../bindings/pci/mediatek-pcie-gen3.yaml      | 17 ++++
 drivers/pci/controller/pcie-mediatek-gen3.c   | 86 ++++++++++++++++++-
 2 files changed, 102 insertions(+), 1 deletion(-)

-- 
2.18.0

[PATCH v2 1/2] dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset

[Jian Yang]

From: "jian.yang" <jian.yang@mediatek.com>

Add new properties to support control power supplies and reset pin of
a downstream component.

Signed-off-by: jian.yang <jian.yang@mediatek.com>
---
 .../bindings/pci/mediatek-pcie-gen3.yaml        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
index 7e8c7a2a5f9b..a59f9d0a782e 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
@@ -84,6 +84,23 @@ properties:
     items:
       enum: [ phy, mac ]
 
+  pcie1v8-supply:
+    description:
+      The regulator phandle that provides 1.8V power to a downstream component.
+
+  pcie3v3-supply:
+    description:
+      The regulator phandle that provides 3.3V power to a downstream component.
+
+  pcie12v-supply:
+    description:
+      The regulator phandle that provides 12V power to a downstream component.
+
+  dsc-reset-gpios:
+    description:
+      The extra reset pin other than PERST# required by a downstream component.
+    maxItems: 1
+
   clocks:
     minItems: 4
     maxItems: 6
-- 
2.18.0

[PATCH v2 2/2] PCI: mediatek-gen3: Add power and reset control feature for downstream component

[Jian Yang]

From: "jian.yang" <jian.yang@mediatek.com>

Make MediaTek's controller driver capable of controlling power
supplies and reset pin of a downstream component in power-on and
power-off flow.

Some downstream components (e.g., a WIFI chip) may need an extra
reset other than PERST# and their power supplies, depending on
the requirements of platform, may need to controlled by their
parent's driver. To meet the requirements described above, I add this
feature to MediaTek's PCIe controller driver as a optional feature.

Signed-off-by: jian.yang <jian.yang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 86 ++++++++++++++++++++-
 1 file changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index b8612ce5f4d0..45e368b03ed2 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -8,6 +8,8 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/iopoll.h>
 #include <linux/irq.h>
 #include <linux/irqchip/chained_irq.h>
@@ -15,11 +17,14 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/msi.h>
+#include <linux/of_gpio.h>
 #include <linux/pci.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
+#include <linux/pm_wakeup.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
 #include "../pci.h"
@@ -100,6 +105,13 @@
 #define PCIE_ATR_TLP_TYPE_MEM		PCIE_ATR_TLP_TYPE(0)
 #define PCIE_ATR_TLP_TYPE_IO		PCIE_ATR_TLP_TYPE(2)
 
+/* Downstream Component power supplies used by MediaTek PCIe */
+static const char *const dsc_power_supplies[] = {
+	"pcie1v8",
+	"pcie3v3",
+	"pcie12v",
+};
+
 /**
  * struct mtk_msi_set - MSI information for each set
  * @base: IO mapped register base
@@ -122,6 +134,9 @@ struct mtk_msi_set {
  * @phy: PHY controller block
  * @clks: PCIe clocks
  * @num_clks: PCIe clocks count for this port
+ * @supplies: Downstream Component power supplies
+ * @num_supplies: Downstream Component power supplies count
+ * @dsc_reset: The GPIO pin to reset Downstream component
  * @irq: PCIe controller interrupt number
  * @saved_irq_state: IRQ enable state saved at suspend time
  * @irq_lock: lock protecting IRQ register access
@@ -141,6 +156,9 @@ struct mtk_gen3_pcie {
 	struct phy *phy;
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct regulator_bulk_data *supplies;
+	int num_supplies;
+	struct gpio_desc *dsc_reset;
 
 	int irq;
 	u32 saved_irq_state;
@@ -763,7 +781,7 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct resource *regs;
-	int ret;
+	int ret, i;
 
 	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcie-mac");
 	if (!regs)
@@ -809,14 +827,72 @@ static int mtk_pcie_parse_port(struct mtk_gen3_pcie *pcie)
 		return pcie->num_clks;
 	}
 
+	pcie->num_supplies = ARRAY_SIZE(dsc_power_supplies);
+	pcie->supplies = devm_kcalloc(dev, pcie->num_supplies,
+				      sizeof(*pcie->supplies),
+				      GFP_KERNEL);
+	if (!pcie->supplies)
+		return -ENOMEM;
+
+	for (i = 0; i < pcie->num_supplies; i++)
+		pcie->supplies[i].supply = dsc_power_supplies[i];
+
+	ret = devm_regulator_bulk_get(dev, pcie->num_supplies, pcie->supplies);
+	if (ret)
+		return ret;
+
+	pcie->dsc_reset = devm_gpiod_get_optional(dev, "dsc-reset",
+						  GPIOD_OUT_LOW);
+	if (IS_ERR(pcie->dsc_reset)) {
+		ret = PTR_ERR(pcie->dsc_reset);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to request DSC reset gpio\n");
+
+		return ret;
+	}
+
 	return 0;
 }
 
+static int mtk_pcie_dsc_power_up(struct mtk_gen3_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	int ret;
+
+	/* Assert Downstream Component reset */
+	if (pcie->dsc_reset)
+		gpiod_set_value_cansleep(pcie->dsc_reset, 1);
+
+	ret = regulator_bulk_enable(pcie->num_supplies, pcie->supplies);
+	if (ret)
+		dev_err(dev, "failed to enable DSC power supplies: %d\n", ret);
+
+	/* De-assert Downstream Component reset */
+	if (pcie->dsc_reset)
+		gpiod_set_value_cansleep(pcie->dsc_reset, 0);
+
+	return ret;
+}
+
+static void mtk_pcie_dsc_power_down(struct mtk_gen3_pcie *pcie)
+{
+	/* Assert Downstream Component reset */
+	if (pcie->dsc_reset)
+		gpiod_set_value_cansleep(pcie->dsc_reset, 1);
+
+	regulator_bulk_disable(pcie->num_supplies, pcie->supplies);
+}
+
 static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	int err;
 
+	/* Downstream Component power up before RC */
+	err = mtk_pcie_dsc_power_up(pcie);
+	if (err)
+		return err;
+
 	/* PHY power on and enable pipe clock */
 	reset_control_deassert(pcie->phy_reset);
 
@@ -855,6 +931,7 @@ static int mtk_pcie_power_up(struct mtk_gen3_pcie *pcie)
 	phy_exit(pcie->phy);
 err_phy_init:
 	reset_control_assert(pcie->phy_reset);
+	mtk_pcie_dsc_power_down(pcie);
 
 	return err;
 }
@@ -870,6 +947,13 @@ static void mtk_pcie_power_down(struct mtk_gen3_pcie *pcie)
 	phy_power_off(pcie->phy);
 	phy_exit(pcie->phy);
 	reset_control_assert(pcie->phy_reset);
+
+	/*
+	 * Keep downstream component powered on if it might need to wake up the
+	 * system in suspend state
+	 */
+	if (!pcie->dev->power.is_suspended || !device_wakeup_path(pcie->dev))
+		mtk_pcie_dsc_power_down(pcie);
 }
 
 static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
-- 
2.18.0

Re: [PATCH v2 0/2] PCI: mediatek-gen3: Support controlling power and

[Jian Yang (杨戬)]

Dear Maintainers,

Gentle ping for this patch series, if there is anything I can do to get
these patches merged, please let me know.

Best regards,
Jian Yang

On Mon, 2023-03-06 at 14:40 +0800, Jian Yang wrote:
> From: "jian.yang" <jian.yang@mediatek.com>
> 
> These series patches add support for controlling power supplies and
> reset
> GPIO of a downstream component in Mediatek's PCIe GEN3 controller
> driver.
> 
> Changes in v2:
> 1. Remove an unnecessary property in dt-bindings file.
> 2. Use the flag 'GPIOD_OUT_LOW' to set initial state of a downstream
> component's reset GPIO.
> 3. Keep downstream component powered on in suspend state if it need
> to
> wakeup the system.
> 
> jian.yang (2):
>   dt-bindings: PCI: mediatek-gen3: Add support for controlling power
> and
>     reset
>   PCI: mediatek-gen3: Add power and reset control feature for
> downstream
>     component
> 
>  .../bindings/pci/mediatek-pcie-gen3.yaml      | 17 ++++
>  drivers/pci/controller/pcie-mediatek-gen3.c   | 86
> ++++++++++++++++++-
>  2 files changed, 102 insertions(+), 1 deletion(-)
> 

Re: [PATCH v2 2/2] PCI: mediatek-gen3: Add power and reset control feature for downstream component

[Rob Herring]

On Mon, Mar 06, 2023 at 02:40:59PM +0800, Jian Yang wrote:
> From: "jian.yang" <jian.yang@mediatek.com>
> 
> Make MediaTek's controller driver capable of controlling power
> supplies and reset pin of a downstream component in power-on and
> power-off flow.
> 
> Some downstream components (e.g., a WIFI chip) may need an extra
> reset other than PERST# and their power supplies, depending on
> the requirements of platform, may need to controlled by their
> parent's driver. To meet the requirements described above, I add this
> feature to MediaTek's PCIe controller driver as a optional feature.

If you have PCI devices with extra stuff that's not standard PCI stuff 
(i.e. what's on a standard connector), then you should be describing 
the PCI device in the DT.

The standard stuff should really be in the root port node rather than 
the host bridge node. That's often omitted too because many host bridges 
only have a single root port.

> 
> Signed-off-by: jian.yang <jian.yang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek-gen3.c | 86 ++++++++++++++++++++-
>  1 file changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b8612ce5f4d0..45e368b03ed2 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -8,6 +8,8 @@
>  
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/iopoll.h>
>  #include <linux/irq.h>
>  #include <linux/irqchip/chained_irq.h>
> @@ -15,11 +17,14 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/msi.h>
> +#include <linux/of_gpio.h>

This header is getting removed. You shouldn't depend on it.

Rob

Re: [PATCH v2 2/2] PCI: mediatek-gen3: Add power and reset control feature for downstream component

[Jian Yang (杨戬)]

Dear Rob,

Sorry for the late response and thanks for your comment.

On Tue, 2023-03-21 at 11:57 -0500, Rob Herring wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Mon, Mar 06, 2023 at 02:40:59PM +0800, Jian Yang wrote:
> > From: "jian.yang" <jian.yang@mediatek.com>
> > 
> > Make MediaTek's controller driver capable of controlling power
> > supplies and reset pin of a downstream component in power-on and
> > power-off flow.
> > 
> > Some downstream components (e.g., a WIFI chip) may need an extra
> > reset other than PERST# and their power supplies, depending on
> > the requirements of platform, may need to controlled by their
> > parent's driver. To meet the requirements described above, I add
> > this
> > feature to MediaTek's PCIe controller driver as a optional feature.
> 
> If you have PCI devices with extra stuff that's not standard PCI
> stuff
> (i.e. what's on a standard connector), then you should be describing
> the PCI device in the DT.
> 
> The standard stuff should really be in the root port node rather than
> the host bridge node. That's often omitted too because many host
> bridges
> only have a single root port.

I will add a description in DT binding of this driver later.

> > 
> > Signed-off-by: jian.yang <jian.yang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 86
> > ++++++++++++++++++++-
> >  1 file changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index b8612ce5f4d0..45e368b03ed2 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -8,6 +8,8 @@
> > 
> >  #include <linux/clk.h>
> >  #include <linux/delay.h>
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/irq.h>
> >  #include <linux/irqchip/chained_irq.h>
> > @@ -15,11 +17,14 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/msi.h>
> > +#include <linux/of_gpio.h>
> 
> This header is getting removed. You shouldn't depend on it.

I will remove it in V3 patch, thanks a lot.

Best regards,
Jian Yang