| Message ID | 20230306064059.7239-3-jian.yang@mediatek.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: mediatek-gen3: Support controlling power and | expand |
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
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
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)