On 2/26/24 9:04 AM, Sumit Garg wrote:
Add support for GPCv2 power domains and clock handling for PCIe and
PCIe PHY.
Signed-off-by: Sumit Garg <sumit.g...@linaro.org>
---
drivers/power/domain/imx8mp-hsiomix.c | 101 ++++++++++++++++++++------
1 file changed, 78 insertions(+), 23 deletions(-)
diff --git a/drivers/power/domain/imx8mp-hsiomix.c
b/drivers/power/domain/imx8mp-hsiomix.c
index e2d772c5ec78..58cc3f63bb56 100644
--- a/drivers/power/domain/imx8mp-hsiomix.c
+++ b/drivers/power/domain/imx8mp-hsiomix.c
@@ -16,48 +16,73 @@
#define GPR_REG0 0x0
#define PCIE_CLOCK_MODULE_EN BIT(0)
#define USB_CLOCK_MODULE_EN BIT(1)
+#define PCIE_PHY_APB_RST BIT(4)
+#define PCIE_PHY_INIT_RST BIT(5)
struct imx8mp_hsiomix_priv {
void __iomem *base;
struct clk clk_usb;
+ struct clk clk_pcie;
struct power_domain pd_bus;
struct power_domain pd_usb;
+ struct power_domain pd_pcie;
struct power_domain pd_usb_phy1;
struct power_domain pd_usb_phy2;
+ struct power_domain pd_pcie_phy;
};
static int imx8mp_hsiomix_on(struct power_domain *power_domain)
{
struct udevice *dev = power_domain->dev;
struct imx8mp_hsiomix_priv *priv = dev_get_priv(dev);
- struct power_domain *domain;
+ struct power_domain *domain = NULL;
+ struct clk *clk = NULL;
+ u32 gpr_reg0_bits = 0;
int ret;
- ret = power_domain_on(&priv->pd_bus);
- if (ret)
- return ret;
-
- if (power_domain->id == IMX8MP_HSIOBLK_PD_USB) {
+ switch (power_domain->id) {
+ case IMX8MP_HSIOBLK_PD_USB:
domain = &priv->pd_usb;
- } else if (power_domain->id == IMX8MP_HSIOBLK_PD_USB_PHY1) {
+ clk = &priv->clk_usb;
+ gpr_reg0_bits |= USB_CLOCK_MODULE_EN;
+ break;
+ case IMX8MP_HSIOBLK_PD_USB_PHY1:
domain = &priv->pd_usb_phy1;
- } else if (power_domain->id == IMX8MP_HSIOBLK_PD_USB_PHY2) {
+ break;
+ case IMX8MP_HSIOBLK_PD_USB_PHY2:
domain = &priv->pd_usb_phy2;
- } else {
- ret = -EINVAL;
- goto err_pd;
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE:
+ domain = &priv->pd_pcie;
+ clk = &priv->clk_pcie;
+ gpr_reg0_bits |= PCIE_CLOCK_MODULE_EN;
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE_PHY:
+ domain = &priv->pd_pcie_phy;
+ /* Bits to deassert PCIe PHY reset */
+ gpr_reg0_bits |= PCIE_PHY_APB_RST | PCIE_PHY_INIT_RST;
+ break;
+ default:
+ dev_err(dev, "unknown power domain id: %ld\n",
+ power_domain->id);
+ return -EINVAL;
}
+ ret = power_domain_on(&priv->pd_bus);
+ if (ret)
+ return ret;
+
ret = power_domain_on(domain);
if (ret)
goto err_pd;
- ret = clk_enable(&priv->clk_usb);
- if (ret)
- goto err_clk;
+ if (clk) {
+ ret = clk_enable(clk);
+ if (ret)
+ goto err_clk;
+ }
- if (power_domain->id == IMX8MP_HSIOBLK_PD_USB)
- setbits_le32(priv->base + GPR_REG0, USB_CLOCK_MODULE_EN);
+ setbits_le32(priv->base + GPR_REG0, gpr_reg0_bits);
Are you sure it is OK to do this unconditionally ?
Why not this:
if (gpr_reg0_bits)
setbits_le32(priv->base + GPR_REG0, gpr_reg0_bits);
return 0;
@@ -73,17 +98,31 @@ static int imx8mp_hsiomix_off(struct power_domain *power_domain)
struct udevice *dev = power_domain->dev;
struct imx8mp_hsiomix_priv *priv = dev_get_priv(dev);
- if (power_domain->id == IMX8MP_HSIOBLK_PD_USB)
+ switch (power_domain->id) {
+ case IMX8MP_HSIOBLK_PD_USB:
clrbits_le32(priv->base + GPR_REG0, USB_CLOCK_MODULE_EN);
-
- clk_disable(&priv->clk_usb);
-
- if (power_domain->id == IMX8MP_HSIOBLK_PD_USB)
+ clk_disable(&priv->clk_usb);
power_domain_off(&priv->pd_usb);
- else if (power_domain->id == IMX8MP_HSIOBLK_PD_USB_PHY1)
+ break;
+ case IMX8MP_HSIOBLK_PD_USB_PHY1:
power_domain_off(&priv->pd_usb_phy1);
- else if (power_domain->id == IMX8MP_HSIOBLK_PD_USB_PHY2)
+ break;
+ case IMX8MP_HSIOBLK_PD_USB_PHY2:
power_domain_off(&priv->pd_usb_phy2);
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE:
Maybe we need some function ... imx8mp_hsiomix_id_to_pd() to avoid this
duplicate switch statement:
pd = imx8mp_hsiomix_id_to_pd(...)
...
power_domain_off(fd);
+ clrbits_le32(priv->base + GPR_REG0, PCIE_CLOCK_MODULE_EN);
+ clk_disable(&priv->clk_pcie);
+ power_domain_off(&priv->pd_pcie);
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE_PHY:
+ clrbits_le32(priv->base + GPR_REG0, PCIE_PHY_APB_RST |
+ PCIE_PHY_INIT_RST);
+ power_domain_off(&priv->pd_pcie_phy);
Is it OK to turn off the bus PD after this ?
Please double-check if power_domain_on/off() is refcounted .
+ break;
+ default:
+ break;
+ }
power_domain_off(&priv->pd_bus);
@@ -109,6 +148,10 @@ static int imx8mp_hsiomix_probe(struct udevice *dev)
if (ret < 0)
return ret;
+ ret = clk_get_by_name(dev, "pcie", &priv->clk_pcie);
+ if (ret < 0)
+ return ret;
+
ret = power_domain_get_by_name(dev, &priv->pd_bus, "bus");
if (ret < 0)
return ret;
Some clk_put() seems to be missing for clk_pcie .
