| Message ID | 20231009084957.18536-3-jian.yang@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: mediatek-gen3: Support controlling power supplies | expand |
Il 09/10/23 10:49, Jian Yang ha scritto: > 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 | 93 ++++++++++++++++++++- > 1 file changed, 92 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c > index e0e27645fdf4..ad4b25c34f5d 100644 > --- a/drivers/pci/controller/pcie-mediatek-gen3.c > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c > @@ -8,6 +8,7 @@ > > #include <linux/clk.h> > #include <linux/delay.h> > +#include <linux/gpio/consumer.h> > #include <linux/iopoll.h> > #include <linux/irq.h> > #include <linux/irqchip/chained_irq.h> > @@ -20,6 +21,8 @@ > #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 +103,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", > +}; Please.... static const char *const dsc_power_supplies[] = { "vpcie1v8", "vpcie3v3", "vpcie12v", }; > + > /** > * struct mtk_msi_set - MSI information for each set > * @base: IO mapped register base > @@ -122,6 +132,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 +154,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 +779,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; Since you anyway have to send a v4, can you please also order these by name? int i, ret; > > regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcie-mac"); > if (!regs) > @@ -809,14 +825,86 @@ 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"); dev_err_probe() does exactly what you're doing here, but it's shorter :-) > + > + return ret; > + } > + > return 0; > } > > +static int mtk_pcie_dsc_power_up(struct mtk_gen3_pcie *pcie) > +{ > + struct device *dev = pcie->dev; > + int ret; > + > + /* > + * Skip downstream component's power-up flow if it was kept power-on * Skip powering up the downstream component if it was kept powered on > + * while system entered suspend state > + */ > + if (device_wakeup_path(dev)) > + return 0; > + > + /* 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) > +{ > + /* > + * Keep downstream component power-on if it need to wake up the * Keep downstream component powered on if it is capable of waking up * the system from suspend > + * system in suspend state > + */ > + if (device_wakeup_path(pcie->dev)) > + return; > + > + /* Assert Downstream Component reset */ > + if (pcie->dsc_reset) > + gpiod_set_value_cansleep(pcie->dsc_reset, 1); > + > + regulator_bulk_disable(pcie->num_supplies, pcie->supplies); > +} > + Regards, Angelo
Hi Angelo, On Tue, 2023-10-10 at 11:51 +0200, AngeloGioacchino Del Regno wrote: > Il 09/10/23 10:49, Jian Yang ha scritto: > > 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 | 93 > > ++++++++++++++++++++- > > 1 file changed, 92 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c > > b/drivers/pci/controller/pcie-mediatek-gen3.c > > index e0e27645fdf4..ad4b25c34f5d 100644 > > --- a/drivers/pci/controller/pcie-mediatek-gen3.c > > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c > > @@ -8,6 +8,7 @@ > > > > #include <linux/clk.h> > > #include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > #include <linux/iopoll.h> > > #include <linux/irq.h> > > #include <linux/irqchip/chained_irq.h> > > @@ -20,6 +21,8 @@ > > #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 +103,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", > > +}; > > Please.... > > static const char *const dsc_power_supplies[] = { > "vpcie1v8", > "vpcie3v3", > "vpcie12v", > }; > OK. > > + > > /** > > * struct mtk_msi_set - MSI information for each set > > * @base: IO mapped register base > > @@ -122,6 +132,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 +154,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 +779,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; > > Since you anyway have to send a v4, can you please also order these > by name? > > int i, ret; > I will re-order them in next version. > > > > regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, > > "pcie-mac"); > > if (!regs) > > @@ -809,14 +825,86 @@ 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"); > > dev_err_probe() does exactly what you're doing here, but it's shorter > :-) > Got it > > + > > + return ret; > > + } > > + > > return 0; > > } > > > > +static int mtk_pcie_dsc_power_up(struct mtk_gen3_pcie *pcie) > > +{ > > + struct device *dev = pcie->dev; > > + int ret; > > + > > + /* > > + * Skip downstream component's power-up flow if it was kept > > power-on > > * Skip powering up the downstream component if it was kept powered on > Thanks for correcting that. > > > + * while system entered suspend state > > + */ > > + if (device_wakeup_path(dev)) > > + return 0; > > + > > + /* 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) > > +{ > > + /* > > + * Keep downstream component power-on if it need to wake up the > > * Keep downstream component powered on if it is capable of waking up > * the system from suspend > Thanks for correcting and sorry for my English :) > > + * system in suspend state > > + */ > > + if (device_wakeup_path(pcie->dev)) > > + return; > > + > > + /* Assert Downstream Component reset */ > > + if (pcie->dsc_reset) > > + gpiod_set_value_cansleep(pcie->dsc_reset, 1); > > + > > + regulator_bulk_disable(pcie->num_supplies, pcie->supplies); > > +} > > + > > Regards, > Angelo > Best regards, Jian Yang
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c index e0e27645fdf4..ad4b25c34f5d 100644 --- a/drivers/pci/controller/pcie-mediatek-gen3.c +++ b/drivers/pci/controller/pcie-mediatek-gen3.c @@ -8,6 +8,7 @@ #include <linux/clk.h> #include <linux/delay.h> +#include <linux/gpio/consumer.h> #include <linux/iopoll.h> #include <linux/irq.h> #include <linux/irqchip/chained_irq.h> @@ -20,6 +21,8 @@ #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 +103,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 +132,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 +154,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 +779,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 +825,86 @@ 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; + + /* + * Skip downstream component's power-up flow if it was kept power-on + * while system entered suspend state + */ + if (device_wakeup_path(dev)) + return 0; + + /* 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) +{ + /* + * Keep downstream component power-on if it need to wake up the + * system in suspend state + */ + if (device_wakeup_path(pcie->dev)) + return; + + /* 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 +943,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 +959,8 @@ 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)