2023-01-11 03:53:50

by Jian Yang

[permalink] [raw]
Subject: [PATCH 1/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 of 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 | 114 +++++++++++++++++++-
1 file changed, 113 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index b8612ce5f4d0..5ddbc2a0c020 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,13 @@
#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/regulator/consumer.h>
#include <linux/reset.h>

#include "../pci.h"
@@ -100,6 +104,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 +133,10 @@ 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
+ * @dsc_reset_delay_ms: Delay in ms before the deassertion of reset GPIO
* @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,10 @@ 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;
+ u32 dsc_reset_delay_ms;

int irq;
u32 saved_irq_state;
@@ -763,7 +782,9 @@ 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;
+ enum of_gpio_flags flags;
+ enum gpiod_flags dsc_reset_init_flags;
+ int ret, i;

regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcie-mac");
if (!regs)
@@ -809,14 +830,103 @@ 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;
+
+ ret = of_get_named_gpio_flags(dev->of_node, "dsc-reset-gpios", 0,
+ &flags);
+ if (ret < 0) {
+ if (ret == -EPROBE_DEFER)
+ return ret;
+
+ /*
+ * It's okay that the reset GPIO of a downstream component not
+ * defined in related devicetree node since it's an optional
+ * property.
+ */
+ return 0;
+ }
+
+ dsc_reset_init_flags = (flags & OF_GPIO_ACTIVE_LOW) ? GPIOD_OUT_HIGH :
+ GPIOD_OUT_LOW;
+ pcie->dsc_reset = devm_gpiod_get_optional(dev, "dsc-reset",
+ dsc_reset_init_flags);
+ 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;
+ }
+
+ ret = of_property_read_u32(dev->of_node, "dsc-reset-msleep",
+ &pcie->dsc_reset_delay_ms);
+ if (ret) {
+ dev_info(dev, "Failed to get delay time of DSC, set it to default 5ms\n");
+ pcie->dsc_reset_delay_ms = 5;
+ }
+
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) {
+ /*
+ * Wait for a short time before we de-assert the reset GPIO.
+ * Depends on the requirement of a specific Downstream
+ * Component.
+ */
+ usleep_range(1000 * pcie->dsc_reset_delay_ms,
+ 1000 * pcie->dsc_reset_delay_ms + 100);
+ 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 +965,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 +981,7 @@ 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);
+ mtk_pcie_dsc_power_down(pcie);
}

static int mtk_pcie_setup(struct mtk_gen3_pcie *pcie)
--
2.18.0


2023-01-11 22:35:16

by Bjorn Helgaas

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

Hi,

On Wed, Jan 11, 2023 at 11:25:41AM +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 of 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.

Is this delay (dsc-reset-msleep) specific to a device downstream from
the MediaTek controller, not to the MediaTek controller itself? If
so, it sounds like it should be a generic value that could be used by
other drivers, too.

How do you determine the value? If there's some PCIe spec that
determines this, please include a citation to it.

Bjorn

2023-02-03 03:27:47

by Jian Yang

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

Dear Bjorn,

Sorry for the late response and thanks for your comment.

On Wed, 2023-01-11 at 16:14 -0600, Bjorn Helgaas wrote:
> Hi,
>
> On Wed, Jan 11, 2023 at 11:25:41AM +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 of 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.
>
> Is this delay (dsc-reset-msleep) specific to a device downstream from
> the MediaTek controller, not to the MediaTek controller itself? If
> so, it sounds like it should be a generic value that could be used by
> other drivers, too.
>
> How do you determine the value? If there's some PCIe spec that
> determines this, please include a citation to it.

Yes. This delay was defined for a downstream device (e.g., a PCIe EP)
which need an extra reset pin, not for Mediatek's PCIe controller
itself. I suppose we need to add a property in devicetree to let user
determine the delay time due to differences in requirements between
various devices.

Best regards,
Jian Yang