2023-03-06 06:41:26

by Jian Yang

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

From: "jian.yang" <[email protected]>

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



2023-03-06 06:41:28

by Jian Yang

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

From: "jian.yang" <[email protected]>

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

Signed-off-by: jian.yang <[email protected]>
---
.../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


2023-03-06 06:41:31

by Jian Yang

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

From: "jian.yang" <[email protected]>

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 <[email protected]>
---
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


2023-03-20 09:03:04

by Jian Yang

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

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" <[email protected]>
>
> 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(-)
>

2023-03-21 16:58:02

by Rob Herring

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

On Mon, Mar 06, 2023 at 02:40:59PM +0800, Jian Yang wrote:
> From: "jian.yang" <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2023-04-04 02:50:04

by Jian Yang

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

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" <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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