* [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-11 16:27 ` Rob Herring
2022-02-13 2:03 ` Shawn Guo
2022-02-07 19:25 ` [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains Lucas Stach
` (7 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
This adds the DT defines for the GPC power domains found on the
i.MX8MP SoC.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
.../bindings/power/fsl,imx-gpcv2.yaml | 2 ++
include/dt-bindings/power/imx8mp-power.h | 29 +++++++++++++++++++
2 files changed, 31 insertions(+)
create mode 100644 include/dt-bindings/power/imx8mp-power.h
diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
index 01bdda167eef..747622bdc57b 100644
--- a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
+++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.yaml
@@ -28,6 +28,7 @@ properties:
- fsl,imx8mn-gpc
- fsl,imx8mq-gpc
- fsl,imx8mm-gpc
+ - fsl,imx8mp-gpc
reg:
maxItems: 1
@@ -57,6 +58,7 @@ properties:
include/dt-bindings/power/imx7-power.h for fsl,imx7d-gpc and
include/dt-bindings/power/imx8m-power.h for fsl,imx8mq-gpc
include/dt-bindings/power/imx8mm-power.h for fsl,imx8mm-gpc
+ include/dt-bindings/power/imx8mp-power.h for fsl,imx8mp-gpc
maxItems: 1
clocks:
diff --git a/include/dt-bindings/power/imx8mp-power.h b/include/dt-bindings/power/imx8mp-power.h
new file mode 100644
index 000000000000..7c67689e4faf
--- /dev/null
+++ b/include/dt-bindings/power/imx8mp-power.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (C) 2020 Pengutronix, Sascha Hauer <kernel@pengutronix.de>
+ */
+
+#ifndef __DT_BINDINGS_IMX8MP_POWER_DOMAIN_POWER_H__
+#define __DT_BINDINGS_IMX8MP_POWER_DOMAIN_POWER_H__
+
+#define IMX8MP_POWER_DOMAIN_MIPI_PHY1 0
+#define IMX8MP_POWER_DOMAIN_PCIE_PHY 1
+#define IMX8MP_POWER_DOMAIN_USB1_PHY 2
+#define IMX8MP_POWER_DOMAIN_USB2_PHY 3
+#define IMX8MP_POWER_DOMAIN_MLMIX 4
+#define IMX8MP_POWER_DOMAIN_AUDIOMIX 5
+#define IMX8MP_POWER_DOMAIN_GPU2D 6
+#define IMX8MP_POWER_DOMAIN_GPUMIX 7
+#define IMX8MP_POWER_DOMAIN_VPUMIX 8
+#define IMX8MP_POWER_DOMAIN_GPU3D 9
+#define IMX8MP_POWER_DOMAIN_MEDIAMIX 10
+#define IMX8MP_POWER_DOMAIN_VPU_G1 11
+#define IMX8MP_POWER_DOMAIN_VPU_G2 12
+#define IMX8MP_POWER_DOMAIN_VPU_VC8000E 13
+#define IMX8MP_POWER_DOMAIN_HDMIMIX 14
+#define IMX8MP_POWER_DOMAIN_HDMI_PHY 15
+#define IMX8MP_POWER_DOMAIN_MIPI_PHY2 16
+#define IMX8MP_POWER_DOMAIN_HSIOMIX 17
+#define IMX8MP_POWER_DOMAIN_MEDIAMIX_ISPDWP 18
+
+#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain
2022-02-07 19:25 ` [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain Lucas Stach
@ 2022-02-11 16:27 ` Rob Herring
2022-02-13 2:03 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Rob Herring @ 2022-02-11 16:27 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, devicetree, Rob Herring, Fabio Estevam,
linux-arm-kernel, patchwork-lst, NXP Linux Team,
Pengutronix Kernel Team
On Mon, 07 Feb 2022 20:25:40 +0100, Lucas Stach wrote:
> This adds the DT defines for the GPC power domains found on the
> i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> .../bindings/power/fsl,imx-gpcv2.yaml | 2 ++
> include/dt-bindings/power/imx8mp-power.h | 29 +++++++++++++++++++
> 2 files changed, 31 insertions(+)
> create mode 100644 include/dt-bindings/power/imx8mp-power.h
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain
2022-02-07 19:25 ` [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain Lucas Stach
2022-02-11 16:27 ` Rob Herring
@ 2022-02-13 2:03 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Shawn Guo @ 2022-02-13 2:03 UTC (permalink / raw)
To: Lucas Stach
Cc: Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
On Mon, Feb 07, 2022 at 08:25:40PM +0100, Lucas Stach wrote:
> This adds the DT defines for the GPC power domains found on the
> i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Applied, thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
2022-02-07 19:25 ` [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-18 23:02 ` Laurent Pinchart
2022-02-07 19:25 ` [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains Lucas Stach
` (6 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
This adds driver support for all the GPC power domains found on
the i.MX8MP SoC.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 386 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 01f46b078df3..a7c92bdfc53b 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -21,10 +21,12 @@
#include <dt-bindings/power/imx8mq-power.h>
#include <dt-bindings/power/imx8mm-power.h>
#include <dt-bindings/power/imx8mn-power.h>
+#include <dt-bindings/power/imx8mp-power.h>
#define GPC_LPCR_A_CORE_BSC 0x000
#define GPC_PGC_CPU_MAPPING 0x0ec
+#define IMX8MP_GPC_PGC_CPU_MAPPING 0x1cc
#define IMX7_USB_HSIC_PHY_A_CORE_DOMAIN BIT(6)
#define IMX7_USB_OTG2_PHY_A_CORE_DOMAIN BIT(5)
@@ -65,6 +67,29 @@
#define IMX8MN_OTG1_A53_DOMAIN BIT(4)
#define IMX8MN_MIPI_A53_DOMAIN BIT(2)
+#define IMX8MP_MEDIA_ISPDWP_A53_DOMAIN BIT(20)
+#define IMX8MP_HSIOMIX_A53_DOMAIN BIT(19)
+#define IMX8MP_MIPI_PHY2_A53_DOMAIN BIT(18)
+#define IMX8MP_HDMI_PHY_A53_DOMAIN BIT(17)
+#define IMX8MP_HDMIMIX_A53_DOMAIN BIT(16)
+#define IMX8MP_VPU_VC8000E_A53_DOMAIN BIT(15)
+#define IMX8MP_VPU_G2_A53_DOMAIN BIT(14)
+#define IMX8MP_VPU_G1_A53_DOMAIN BIT(13)
+#define IMX8MP_MEDIAMIX_A53_DOMAIN BIT(12)
+#define IMX8MP_GPU3D_A53_DOMAIN BIT(11)
+#define IMX8MP_VPUMIX_A53_DOMAIN BIT(10)
+#define IMX8MP_GPUMIX_A53_DOMAIN BIT(9)
+#define IMX8MP_GPU2D_A53_DOMAIN BIT(8)
+#define IMX8MP_AUDIOMIX_A53_DOMAIN BIT(7)
+#define IMX8MP_MLMIX_A53_DOMAIN BIT(6)
+#define IMX8MP_USB2_PHY_A53_DOMAIN BIT(5)
+#define IMX8MP_USB1_PHY_A53_DOMAIN BIT(4)
+#define IMX8MP_PCIE_PHY_A53_DOMAIN BIT(3)
+#define IMX8MP_MIPI_PHY1_A53_DOMAIN BIT(2)
+
+#define IMX8MP_GPC_PU_PGC_SW_PUP_REQ 0x0d8
+#define IMX8MP_GPC_PU_PGC_SW_PDN_REQ 0x0e4
+
#define GPC_PU_PGC_SW_PUP_REQ 0x0f8
#define GPC_PU_PGC_SW_PDN_REQ 0x104
@@ -107,8 +132,30 @@
#define IMX8MN_OTG1_SW_Pxx_REQ BIT(2)
#define IMX8MN_MIPI_SW_Pxx_REQ BIT(0)
+#define IMX8MP_DDRMIX_Pxx_REQ BIT(19)
+#define IMX8MP_MEDIA_ISP_DWP_Pxx_REQ BIT(18)
+#define IMX8MP_HSIOMIX_Pxx_REQ BIT(17)
+#define IMX8MP_MIPI_PHY2_Pxx_REQ BIT(16)
+#define IMX8MP_HDMI_PHY_Pxx_REQ BIT(15)
+#define IMX8MP_HDMIMIX_Pxx_REQ BIT(14)
+#define IMX8MP_VPU_VC8K_Pxx_REQ BIT(13)
+#define IMX8MP_VPU_G2_Pxx_REQ BIT(12)
+#define IMX8MP_VPU_G1_Pxx_REQ BIT(11)
+#define IMX8MP_MEDIMIX_Pxx_REQ BIT(10)
+#define IMX8MP_GPU_3D_Pxx_REQ BIT(9)
+#define IMX8MP_VPU_MIX_SHARE_LOGIC_Pxx_REQ BIT(8)
+#define IMX8MP_GPU_SHARE_LOGIC_Pxx_REQ BIT(7)
+#define IMX8MP_GPU_2D_Pxx_REQ BIT(6)
+#define IMX8MP_AUDIOMIX_Pxx_REQ BIT(5)
+#define IMX8MP_MLMIX_Pxx_REQ BIT(4)
+#define IMX8MP_USB2_PHY_Pxx_REQ BIT(3)
+#define IMX8MP_USB1_PHY_Pxx_REQ BIT(2)
+#define IMX8MP_PCIE_PHY_SW_Pxx_REQ BIT(1)
+#define IMX8MP_MIPI_PHY1_SW_Pxx_REQ BIT(0)
+
#define GPC_M4_PU_PDN_FLG 0x1bc
+#define IMX8MP_GPC_PU_PWRHSK 0x190
#define GPC_PU_PWRHSK 0x1fc
#define IMX8M_GPU_HSK_PWRDNACKN BIT(26)
@@ -118,7 +165,6 @@
#define IMX8M_VPU_HSK_PWRDNREQN BIT(5)
#define IMX8M_DISP_HSK_PWRDNREQN BIT(4)
-
#define IMX8MM_GPUMIX_HSK_PWRDNACKN BIT(29)
#define IMX8MM_GPU_HSK_PWRDNACKN (BIT(27) | BIT(28))
#define IMX8MM_VPUMIX_HSK_PWRDNACKN BIT(26)
@@ -137,6 +183,21 @@
#define IMX8MN_DISPMIX_HSK_PWRDNREQN BIT(7)
#define IMX8MN_HSIO_HSK_PWRDNREQN BIT(5)
+#define IMX8MP_MEDIAMIX_PWRDNACKN BIT(3)
+#define IMX8MP_HDMIMIX_PWRDNACKN BIT(29)
+#define IMX8MP_HSIOMIX_PWRDNACKN BIT(28)
+#define IMX8MP_VPUMIX_PWRDNACKN BIT(26)
+#define IMX8MP_GPUMIX_PWRDNACKN BIT(25)
+#define IMX8MP_MLMIX_PWRDNACKN (BIT(23) | BIT(24))
+#define IMX8MP_AUDIOMIX_PWRDNACKN (BIT(20) | BIT(31))
+#define IMX8MP_MEDIAMIX_PWRDNREQN BIT(14)
+#define IMX8MP_HDMIMIX_PWRDNREQN BIT(13)
+#define IMX8MP_HSIOMIX_PWRDNREQN BIT(12)
+#define IMX8MP_VPUMIX_PWRDNREQN BIT(10)
+#define IMX8MP_GPUMIX_PWRDNREQN BIT(9)
+#define IMX8MP_MLMIX_PWRDNREQN (BIT(7) | BIT(8))
+#define IMX8MP_AUDIOMIX_PWRDNREQN (BIT(4) | BIT(15))
+
/*
* The PGC offset values in Reference Manual
* (Rev. 1, 01/2018 and the older ones) GPC chapter's
@@ -179,6 +240,28 @@
#define IMX8MN_PGC_GPUMIX 23
#define IMX8MN_PGC_DISPMIX 26
+#define IMX8MP_PGC_NOC 9
+#define IMX8MP_PGC_MIPI1 12
+#define IMX8MP_PGC_PCIE 13
+#define IMX8MP_PGC_USB1 14
+#define IMX8MP_PGC_USB2 15
+#define IMX8MP_PGC_MLMIX 16
+#define IMX8MP_PGC_AUDIOMIX 17
+#define IMX8MP_PGC_GPU2D 18
+#define IMX8MP_PGC_GPUMIX 19
+#define IMX8MP_PGC_VPUMIX 20
+#define IMX8MP_PGC_GPU3D 21
+#define IMX8MP_PGC_MEDIAMIX 22
+#define IMX8MP_PGC_VPU_G1 23
+#define IMX8MP_PGC_VPU_G2 24
+#define IMX8MP_PGC_VPU_VC8000E 25
+#define IMX8MP_PGC_HDMIMIX 26
+#define IMX8MP_PGC_HDMI 27
+#define IMX8MP_PGC_MIPI2 28
+#define IMX8MP_PGC_HSIOMIX 29
+#define IMX8MP_PGC_MEDIA_ISP_DWP 30
+#define IMX8MP_PGC_DDRMIX 31
+
#define GPC_PGC_CTRL(n) (0x800 + (n) * 0x40)
#define GPC_PGC_SR(n) (GPC_PGC_CTRL(n) + 0xc)
@@ -212,6 +295,9 @@ struct imx_pgc_domain {
const int voltage;
const bool keep_clocks;
struct device *dev;
+
+ unsigned int pgc_sw_pup_reg;
+ unsigned int pgc_sw_pdn_reg;
};
struct imx_pgc_domain_data {
@@ -824,6 +910,303 @@ static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
.pgc_regs = &imx7_pgc_regs,
};
+static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
+ [IMX8MP_POWER_DOMAIN_MIPI_PHY1] = {
+ .genpd = {
+ .name = "mipi-phy1",
+ },
+ .bits = {
+ .pxx = IMX8MP_MIPI_PHY1_SW_Pxx_REQ,
+ .map = IMX8MP_MIPI_PHY1_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_MIPI1),
+ },
+
+ [IMX8MP_POWER_DOMAIN_PCIE_PHY] = {
+ .genpd = {
+ .name = "pcie-phy1",
+ },
+ .bits = {
+ .pxx = IMX8MP_PCIE_PHY_SW_Pxx_REQ,
+ .map = IMX8MP_PCIE_PHY_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_PCIE),
+ },
+
+ [IMX8MP_POWER_DOMAIN_USB1_PHY] = {
+ .genpd = {
+ .name = "usb-otg1",
+ },
+ .bits = {
+ .pxx = IMX8MP_USB1_PHY_Pxx_REQ,
+ .map = IMX8MP_USB1_PHY_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_USB1),
+ },
+
+ [IMX8MP_POWER_DOMAIN_USB2_PHY] = {
+ .genpd = {
+ .name = "usb-otg2",
+ },
+ .bits = {
+ .pxx = IMX8MP_USB2_PHY_Pxx_REQ,
+ .map = IMX8MP_USB2_PHY_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_USB2),
+ },
+
+ [IMX8MP_POWER_DOMAIN_MLMIX] = {
+ .genpd = {
+ .name = "mlmix",
+ },
+ .bits = {
+ .pxx = IMX8MP_MLMIX_Pxx_REQ,
+ .map = IMX8MP_MLMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_MLMIX_PWRDNREQN,
+ .hskack = IMX8MP_MLMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_MLMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_AUDIOMIX] = {
+ .genpd = {
+ .name = "audiomix",
+ },
+ .bits = {
+ .pxx = IMX8MP_AUDIOMIX_Pxx_REQ,
+ .map = IMX8MP_AUDIOMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_AUDIOMIX_PWRDNREQN,
+ .hskack = IMX8MP_AUDIOMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_AUDIOMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_GPU2D] = {
+ .genpd = {
+ .name = "gpu2d",
+ },
+ .bits = {
+ .pxx = IMX8MP_GPU_2D_Pxx_REQ,
+ .map = IMX8MP_GPU2D_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_GPU2D),
+ },
+
+ [IMX8MP_POWER_DOMAIN_GPUMIX] = {
+ .genpd = {
+ .name = "gpumix",
+ },
+ .bits = {
+ .pxx = IMX8MP_GPU_SHARE_LOGIC_Pxx_REQ,
+ .map = IMX8MP_GPUMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_GPUMIX_PWRDNREQN,
+ .hskack = IMX8MP_GPUMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_GPUMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_VPUMIX] = {
+ .genpd = {
+ .name = "vpumix",
+ },
+ .bits = {
+ .pxx = IMX8MP_VPU_MIX_SHARE_LOGIC_Pxx_REQ,
+ .map = IMX8MP_VPUMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_VPUMIX_PWRDNREQN,
+ .hskack = IMX8MP_VPUMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_VPUMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_GPU3D] = {
+ .genpd = {
+ .name = "gpu3d",
+ },
+ .bits = {
+ .pxx = IMX8MP_GPU_3D_Pxx_REQ,
+ .map = IMX8MP_GPU3D_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_GPU3D),
+ },
+
+ [IMX8MP_POWER_DOMAIN_MEDIAMIX] = {
+ .genpd = {
+ .name = "mediamix",
+ },
+ .bits = {
+ .pxx = IMX8MP_MEDIMIX_Pxx_REQ,
+ .map = IMX8MP_MEDIAMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_MEDIAMIX_PWRDNREQN,
+ .hskack = IMX8MP_MEDIAMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_MEDIAMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_VPU_G1] = {
+ .genpd = {
+ .name = "vpu-g1",
+ },
+ .bits = {
+ .pxx = IMX8MP_VPU_G1_Pxx_REQ,
+ .map = IMX8MP_VPU_G1_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_VPU_G1),
+ },
+
+ [IMX8MP_POWER_DOMAIN_VPU_G2] = {
+ .genpd = {
+ .name = "vpu-g2",
+ },
+ .bits = {
+ .pxx = IMX8MP_VPU_G2_Pxx_REQ,
+ .map = IMX8MP_VPU_G2_A53_DOMAIN
+ },
+ .pgc = BIT(IMX8MP_PGC_VPU_G2),
+ },
+
+ [IMX8MP_POWER_DOMAIN_VPU_VC8000E] = {
+ .genpd = {
+ .name = "vpu-h1",
+ },
+ .bits = {
+ .pxx = IMX8MP_VPU_VC8K_Pxx_REQ,
+ .map = IMX8MP_VPU_VC8000E_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_VPU_VC8000E),
+ },
+
+ [IMX8MP_POWER_DOMAIN_HDMIMIX] = {
+ .genpd = {
+ .name = "hdmimix",
+ },
+ .bits = {
+ .pxx = IMX8MP_HDMIMIX_Pxx_REQ,
+ .map = IMX8MP_HDMIMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_HDMIMIX_PWRDNREQN,
+ .hskack = IMX8MP_HDMIMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_HDMIMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_HDMI_PHY] = {
+ .genpd = {
+ .name = "hdmi-phy",
+ },
+ .bits = {
+ .pxx = IMX8MP_HDMI_PHY_Pxx_REQ,
+ .map = IMX8MP_HDMI_PHY_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_HDMI),
+ },
+
+ [IMX8MP_POWER_DOMAIN_MIPI_PHY2] = {
+ .genpd = {
+ .name = "mipi-phy2",
+ },
+ .bits = {
+ .pxx = IMX8MP_MIPI_PHY2_Pxx_REQ,
+ .map = IMX8MP_MIPI_PHY2_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_MIPI2),
+ },
+
+ [IMX8MP_POWER_DOMAIN_HSIOMIX] = {
+ .genpd = {
+ .name = "hsiomix",
+ },
+ .bits = {
+ .pxx = IMX8MP_HSIOMIX_Pxx_REQ,
+ .map = IMX8MP_HSIOMIX_A53_DOMAIN,
+ .hskreq = IMX8MP_HSIOMIX_PWRDNREQN,
+ .hskack = IMX8MP_HSIOMIX_PWRDNACKN,
+ },
+ .pgc = BIT(IMX8MP_PGC_HSIOMIX),
+ .keep_clocks = true,
+ },
+
+ [IMX8MP_POWER_DOMAIN_MEDIAMIX_ISPDWP] = {
+ .genpd = {
+ .name = "mediamix-isp-dwp",
+ },
+ .bits = {
+ .pxx = IMX8MP_MEDIA_ISP_DWP_Pxx_REQ,
+ .map = IMX8MP_MEDIA_ISPDWP_A53_DOMAIN,
+ },
+ .pgc = BIT(IMX8MP_PGC_MEDIA_ISP_DWP),
+ },
+};
+
+static const struct regmap_range imx8mp_yes_ranges[] = {
+ regmap_reg_range(GPC_LPCR_A_CORE_BSC,
+ IMX8MP_GPC_PGC_CPU_MAPPING),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_NOC),
+ GPC_PGC_SR(IMX8MP_PGC_NOC)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MIPI1),
+ GPC_PGC_SR(IMX8MP_PGC_MIPI1)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_PCIE),
+ GPC_PGC_SR(IMX8MP_PGC_PCIE)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_USB1),
+ GPC_PGC_SR(IMX8MP_PGC_USB1)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_USB2),
+ GPC_PGC_SR(IMX8MP_PGC_USB2)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MLMIX),
+ GPC_PGC_SR(IMX8MP_PGC_MLMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_AUDIOMIX),
+ GPC_PGC_SR(IMX8MP_PGC_AUDIOMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPU2D),
+ GPC_PGC_SR(IMX8MP_PGC_GPU2D)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPUMIX),
+ GPC_PGC_SR(IMX8MP_PGC_GPUMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPUMIX),
+ GPC_PGC_SR(IMX8MP_PGC_VPUMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPU3D),
+ GPC_PGC_SR(IMX8MP_PGC_GPU3D)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MEDIAMIX),
+ GPC_PGC_SR(IMX8MP_PGC_MEDIAMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_G1),
+ GPC_PGC_SR(IMX8MP_PGC_VPU_G1)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_G2),
+ GPC_PGC_SR(IMX8MP_PGC_VPU_G2)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_VC8000E),
+ GPC_PGC_SR(IMX8MP_PGC_VPU_VC8000E)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HDMIMIX),
+ GPC_PGC_SR(IMX8MP_PGC_HDMIMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HDMI),
+ GPC_PGC_SR(IMX8MP_PGC_HDMI)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MIPI2),
+ GPC_PGC_SR(IMX8MP_PGC_MIPI2)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HSIOMIX),
+ GPC_PGC_SR(IMX8MP_PGC_HSIOMIX)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MEDIA_ISP_DWP),
+ GPC_PGC_SR(IMX8MP_PGC_MEDIA_ISP_DWP)),
+ regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_DDRMIX),
+ GPC_PGC_SR(IMX8MP_PGC_DDRMIX)),
+};
+
+static const struct regmap_access_table imx8mp_access_table = {
+ .yes_ranges = imx8mp_yes_ranges,
+ .n_yes_ranges = ARRAY_SIZE(imx8mp_yes_ranges),
+};
+
+static const struct imx_pgc_regs imx8mp_pgc_regs = {
+ .map = IMX8MP_GPC_PGC_CPU_MAPPING,
+ .pup = IMX8MP_GPC_PU_PGC_SW_PUP_REQ,
+ .pdn = IMX8MP_GPC_PU_PGC_SW_PDN_REQ,
+ .hsk = IMX8MP_GPC_PU_PWRHSK,
+};
+static const struct imx_pgc_domain_data imx8mp_pgc_domain_data = {
+ .domains = imx8mp_pgc_domains,
+ .domains_num = ARRAY_SIZE(imx8mp_pgc_domains),
+ .reg_access_table = &imx8mp_access_table,
+ .pgc_regs = &imx8mp_pgc_regs,
+};
+
static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
[IMX8MN_POWER_DOMAIN_HSIOMIX] = {
.genpd = {
@@ -1119,6 +1502,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
domain = pd_pdev->dev.platform_data;
domain->regmap = regmap;
domain->regs = domain_data->pgc_regs;
+
domain->genpd.power_on = imx_pgc_power_up;
domain->genpd.power_off = imx_pgc_power_down;
@@ -1140,6 +1524,7 @@ static const struct of_device_id imx_gpcv2_dt_ids[] = {
{ .compatible = "fsl,imx7d-gpc", .data = &imx7_pgc_domain_data, },
{ .compatible = "fsl,imx8mm-gpc", .data = &imx8mm_pgc_domain_data, },
{ .compatible = "fsl,imx8mn-gpc", .data = &imx8mn_pgc_domain_data, },
+ { .compatible = "fsl,imx8mp-gpc", .data = &imx8mp_pgc_domain_data, },
{ .compatible = "fsl,imx8mq-gpc", .data = &imx8m_pgc_domain_data, },
{ }
};
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
2022-02-07 19:25 ` [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains Lucas Stach
@ 2022-02-18 23:02 ` Laurent Pinchart
2022-02-20 11:35 ` Laurent Pinchart
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-18 23:02 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> This adds driver support for all the GPC power domains found on
> the i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 386 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 01f46b078df3..a7c92bdfc53b 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -21,10 +21,12 @@
> #include <dt-bindings/power/imx8mq-power.h>
> #include <dt-bindings/power/imx8mm-power.h>
> #include <dt-bindings/power/imx8mn-power.h>
> +#include <dt-bindings/power/imx8mp-power.h>
>
> #define GPC_LPCR_A_CORE_BSC 0x000
>
> #define GPC_PGC_CPU_MAPPING 0x0ec
> +#define IMX8MP_GPC_PGC_CPU_MAPPING 0x1cc
>
> #define IMX7_USB_HSIC_PHY_A_CORE_DOMAIN BIT(6)
> #define IMX7_USB_OTG2_PHY_A_CORE_DOMAIN BIT(5)
> @@ -65,6 +67,29 @@
> #define IMX8MN_OTG1_A53_DOMAIN BIT(4)
> #define IMX8MN_MIPI_A53_DOMAIN BIT(2)
>
> +#define IMX8MP_MEDIA_ISPDWP_A53_DOMAIN BIT(20)
> +#define IMX8MP_HSIOMIX_A53_DOMAIN BIT(19)
> +#define IMX8MP_MIPI_PHY2_A53_DOMAIN BIT(18)
> +#define IMX8MP_HDMI_PHY_A53_DOMAIN BIT(17)
> +#define IMX8MP_HDMIMIX_A53_DOMAIN BIT(16)
> +#define IMX8MP_VPU_VC8000E_A53_DOMAIN BIT(15)
> +#define IMX8MP_VPU_G2_A53_DOMAIN BIT(14)
> +#define IMX8MP_VPU_G1_A53_DOMAIN BIT(13)
> +#define IMX8MP_MEDIAMIX_A53_DOMAIN BIT(12)
> +#define IMX8MP_GPU3D_A53_DOMAIN BIT(11)
> +#define IMX8MP_VPUMIX_A53_DOMAIN BIT(10)
> +#define IMX8MP_GPUMIX_A53_DOMAIN BIT(9)
> +#define IMX8MP_GPU2D_A53_DOMAIN BIT(8)
> +#define IMX8MP_AUDIOMIX_A53_DOMAIN BIT(7)
> +#define IMX8MP_MLMIX_A53_DOMAIN BIT(6)
> +#define IMX8MP_USB2_PHY_A53_DOMAIN BIT(5)
> +#define IMX8MP_USB1_PHY_A53_DOMAIN BIT(4)
> +#define IMX8MP_PCIE_PHY_A53_DOMAIN BIT(3)
> +#define IMX8MP_MIPI_PHY1_A53_DOMAIN BIT(2)
> +
> +#define IMX8MP_GPC_PU_PGC_SW_PUP_REQ 0x0d8
> +#define IMX8MP_GPC_PU_PGC_SW_PDN_REQ 0x0e4
> +
> #define GPC_PU_PGC_SW_PUP_REQ 0x0f8
> #define GPC_PU_PGC_SW_PDN_REQ 0x104
>
> @@ -107,8 +132,30 @@
> #define IMX8MN_OTG1_SW_Pxx_REQ BIT(2)
> #define IMX8MN_MIPI_SW_Pxx_REQ BIT(0)
>
> +#define IMX8MP_DDRMIX_Pxx_REQ BIT(19)
> +#define IMX8MP_MEDIA_ISP_DWP_Pxx_REQ BIT(18)
> +#define IMX8MP_HSIOMIX_Pxx_REQ BIT(17)
> +#define IMX8MP_MIPI_PHY2_Pxx_REQ BIT(16)
> +#define IMX8MP_HDMI_PHY_Pxx_REQ BIT(15)
> +#define IMX8MP_HDMIMIX_Pxx_REQ BIT(14)
> +#define IMX8MP_VPU_VC8K_Pxx_REQ BIT(13)
> +#define IMX8MP_VPU_G2_Pxx_REQ BIT(12)
> +#define IMX8MP_VPU_G1_Pxx_REQ BIT(11)
> +#define IMX8MP_MEDIMIX_Pxx_REQ BIT(10)
> +#define IMX8MP_GPU_3D_Pxx_REQ BIT(9)
> +#define IMX8MP_VPU_MIX_SHARE_LOGIC_Pxx_REQ BIT(8)
> +#define IMX8MP_GPU_SHARE_LOGIC_Pxx_REQ BIT(7)
> +#define IMX8MP_GPU_2D_Pxx_REQ BIT(6)
> +#define IMX8MP_AUDIOMIX_Pxx_REQ BIT(5)
> +#define IMX8MP_MLMIX_Pxx_REQ BIT(4)
> +#define IMX8MP_USB2_PHY_Pxx_REQ BIT(3)
> +#define IMX8MP_USB1_PHY_Pxx_REQ BIT(2)
> +#define IMX8MP_PCIE_PHY_SW_Pxx_REQ BIT(1)
> +#define IMX8MP_MIPI_PHY1_SW_Pxx_REQ BIT(0)
> +
> #define GPC_M4_PU_PDN_FLG 0x1bc
>
> +#define IMX8MP_GPC_PU_PWRHSK 0x190
> #define GPC_PU_PWRHSK 0x1fc
>
> #define IMX8M_GPU_HSK_PWRDNACKN BIT(26)
> @@ -118,7 +165,6 @@
> #define IMX8M_VPU_HSK_PWRDNREQN BIT(5)
> #define IMX8M_DISP_HSK_PWRDNREQN BIT(4)
>
> -
> #define IMX8MM_GPUMIX_HSK_PWRDNACKN BIT(29)
> #define IMX8MM_GPU_HSK_PWRDNACKN (BIT(27) | BIT(28))
> #define IMX8MM_VPUMIX_HSK_PWRDNACKN BIT(26)
> @@ -137,6 +183,21 @@
> #define IMX8MN_DISPMIX_HSK_PWRDNREQN BIT(7)
> #define IMX8MN_HSIO_HSK_PWRDNREQN BIT(5)
>
> +#define IMX8MP_MEDIAMIX_PWRDNACKN BIT(3)
This should be bit 30.
> +#define IMX8MP_HDMIMIX_PWRDNACKN BIT(29)
> +#define IMX8MP_HSIOMIX_PWRDNACKN BIT(28)
> +#define IMX8MP_VPUMIX_PWRDNACKN BIT(26)
> +#define IMX8MP_GPUMIX_PWRDNACKN BIT(25)
> +#define IMX8MP_MLMIX_PWRDNACKN (BIT(23) | BIT(24))
> +#define IMX8MP_AUDIOMIX_PWRDNACKN (BIT(20) | BIT(31))
> +#define IMX8MP_MEDIAMIX_PWRDNREQN BIT(14)
> +#define IMX8MP_HDMIMIX_PWRDNREQN BIT(13)
> +#define IMX8MP_HSIOMIX_PWRDNREQN BIT(12)
> +#define IMX8MP_VPUMIX_PWRDNREQN BIT(10)
> +#define IMX8MP_GPUMIX_PWRDNREQN BIT(9)
> +#define IMX8MP_MLMIX_PWRDNREQN (BIT(7) | BIT(8))
> +#define IMX8MP_AUDIOMIX_PWRDNREQN (BIT(4) | BIT(15))
> +
> /*
> * The PGC offset values in Reference Manual
> * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> @@ -179,6 +240,28 @@
> #define IMX8MN_PGC_GPUMIX 23
> #define IMX8MN_PGC_DISPMIX 26
>
> +#define IMX8MP_PGC_NOC 9
> +#define IMX8MP_PGC_MIPI1 12
> +#define IMX8MP_PGC_PCIE 13
> +#define IMX8MP_PGC_USB1 14
> +#define IMX8MP_PGC_USB2 15
> +#define IMX8MP_PGC_MLMIX 16
> +#define IMX8MP_PGC_AUDIOMIX 17
> +#define IMX8MP_PGC_GPU2D 18
> +#define IMX8MP_PGC_GPUMIX 19
> +#define IMX8MP_PGC_VPUMIX 20
> +#define IMX8MP_PGC_GPU3D 21
> +#define IMX8MP_PGC_MEDIAMIX 22
> +#define IMX8MP_PGC_VPU_G1 23
> +#define IMX8MP_PGC_VPU_G2 24
> +#define IMX8MP_PGC_VPU_VC8000E 25
> +#define IMX8MP_PGC_HDMIMIX 26
> +#define IMX8MP_PGC_HDMI 27
> +#define IMX8MP_PGC_MIPI2 28
> +#define IMX8MP_PGC_HSIOMIX 29
> +#define IMX8MP_PGC_MEDIA_ISP_DWP 30
> +#define IMX8MP_PGC_DDRMIX 31
> +
> #define GPC_PGC_CTRL(n) (0x800 + (n) * 0x40)
> #define GPC_PGC_SR(n) (GPC_PGC_CTRL(n) + 0xc)
>
> @@ -212,6 +295,9 @@ struct imx_pgc_domain {
> const int voltage;
> const bool keep_clocks;
> struct device *dev;
> +
> + unsigned int pgc_sw_pup_reg;
> + unsigned int pgc_sw_pdn_reg;
> };
>
> struct imx_pgc_domain_data {
> @@ -824,6 +910,303 @@ static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> .pgc_regs = &imx7_pgc_regs,
> };
>
> +static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
> + [IMX8MP_POWER_DOMAIN_MIPI_PHY1] = {
> + .genpd = {
> + .name = "mipi-phy1",
> + },
> + .bits = {
> + .pxx = IMX8MP_MIPI_PHY1_SW_Pxx_REQ,
> + .map = IMX8MP_MIPI_PHY1_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_MIPI1),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_PCIE_PHY] = {
> + .genpd = {
> + .name = "pcie-phy1",
> + },
> + .bits = {
> + .pxx = IMX8MP_PCIE_PHY_SW_Pxx_REQ,
> + .map = IMX8MP_PCIE_PHY_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_PCIE),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_USB1_PHY] = {
> + .genpd = {
> + .name = "usb-otg1",
> + },
> + .bits = {
> + .pxx = IMX8MP_USB1_PHY_Pxx_REQ,
> + .map = IMX8MP_USB1_PHY_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_USB1),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_USB2_PHY] = {
> + .genpd = {
> + .name = "usb-otg2",
> + },
> + .bits = {
> + .pxx = IMX8MP_USB2_PHY_Pxx_REQ,
> + .map = IMX8MP_USB2_PHY_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_USB2),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_MLMIX] = {
> + .genpd = {
> + .name = "mlmix",
> + },
> + .bits = {
> + .pxx = IMX8MP_MLMIX_Pxx_REQ,
> + .map = IMX8MP_MLMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_MLMIX_PWRDNREQN,
> + .hskack = IMX8MP_MLMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_MLMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_AUDIOMIX] = {
> + .genpd = {
> + .name = "audiomix",
> + },
> + .bits = {
> + .pxx = IMX8MP_AUDIOMIX_Pxx_REQ,
> + .map = IMX8MP_AUDIOMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_AUDIOMIX_PWRDNREQN,
> + .hskack = IMX8MP_AUDIOMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_AUDIOMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_GPU2D] = {
> + .genpd = {
> + .name = "gpu2d",
> + },
> + .bits = {
> + .pxx = IMX8MP_GPU_2D_Pxx_REQ,
> + .map = IMX8MP_GPU2D_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_GPU2D),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_GPUMIX] = {
> + .genpd = {
> + .name = "gpumix",
> + },
> + .bits = {
> + .pxx = IMX8MP_GPU_SHARE_LOGIC_Pxx_REQ,
> + .map = IMX8MP_GPUMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_GPUMIX_PWRDNREQN,
> + .hskack = IMX8MP_GPUMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_GPUMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_VPUMIX] = {
> + .genpd = {
> + .name = "vpumix",
> + },
> + .bits = {
> + .pxx = IMX8MP_VPU_MIX_SHARE_LOGIC_Pxx_REQ,
> + .map = IMX8MP_VPUMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_VPUMIX_PWRDNREQN,
> + .hskack = IMX8MP_VPUMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_VPUMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_GPU3D] = {
> + .genpd = {
> + .name = "gpu3d",
> + },
> + .bits = {
> + .pxx = IMX8MP_GPU_3D_Pxx_REQ,
> + .map = IMX8MP_GPU3D_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_GPU3D),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_MEDIAMIX] = {
> + .genpd = {
> + .name = "mediamix",
> + },
> + .bits = {
> + .pxx = IMX8MP_MEDIMIX_Pxx_REQ,
> + .map = IMX8MP_MEDIAMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_MEDIAMIX_PWRDNREQN,
> + .hskack = IMX8MP_MEDIAMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_MEDIAMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_VPU_G1] = {
> + .genpd = {
> + .name = "vpu-g1",
> + },
> + .bits = {
> + .pxx = IMX8MP_VPU_G1_Pxx_REQ,
> + .map = IMX8MP_VPU_G1_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_VPU_G1),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_VPU_G2] = {
> + .genpd = {
> + .name = "vpu-g2",
> + },
> + .bits = {
> + .pxx = IMX8MP_VPU_G2_Pxx_REQ,
> + .map = IMX8MP_VPU_G2_A53_DOMAIN
> + },
> + .pgc = BIT(IMX8MP_PGC_VPU_G2),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_VPU_VC8000E] = {
> + .genpd = {
> + .name = "vpu-h1",
> + },
> + .bits = {
> + .pxx = IMX8MP_VPU_VC8K_Pxx_REQ,
> + .map = IMX8MP_VPU_VC8000E_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_VPU_VC8000E),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_HDMIMIX] = {
> + .genpd = {
> + .name = "hdmimix",
> + },
> + .bits = {
> + .pxx = IMX8MP_HDMIMIX_Pxx_REQ,
> + .map = IMX8MP_HDMIMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_HDMIMIX_PWRDNREQN,
> + .hskack = IMX8MP_HDMIMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_HDMIMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_HDMI_PHY] = {
> + .genpd = {
> + .name = "hdmi-phy",
> + },
> + .bits = {
> + .pxx = IMX8MP_HDMI_PHY_Pxx_REQ,
> + .map = IMX8MP_HDMI_PHY_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_HDMI),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_MIPI_PHY2] = {
> + .genpd = {
> + .name = "mipi-phy2",
> + },
> + .bits = {
> + .pxx = IMX8MP_MIPI_PHY2_Pxx_REQ,
> + .map = IMX8MP_MIPI_PHY2_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_MIPI2),
> + },
> +
> + [IMX8MP_POWER_DOMAIN_HSIOMIX] = {
> + .genpd = {
> + .name = "hsiomix",
> + },
> + .bits = {
> + .pxx = IMX8MP_HSIOMIX_Pxx_REQ,
> + .map = IMX8MP_HSIOMIX_A53_DOMAIN,
> + .hskreq = IMX8MP_HSIOMIX_PWRDNREQN,
> + .hskack = IMX8MP_HSIOMIX_PWRDNACKN,
> + },
> + .pgc = BIT(IMX8MP_PGC_HSIOMIX),
> + .keep_clocks = true,
> + },
> +
> + [IMX8MP_POWER_DOMAIN_MEDIAMIX_ISPDWP] = {
> + .genpd = {
> + .name = "mediamix-isp-dwp",
> + },
> + .bits = {
> + .pxx = IMX8MP_MEDIA_ISP_DWP_Pxx_REQ,
> + .map = IMX8MP_MEDIA_ISPDWP_A53_DOMAIN,
> + },
> + .pgc = BIT(IMX8MP_PGC_MEDIA_ISP_DWP),
> + },
> +};
> +
> +static const struct regmap_range imx8mp_yes_ranges[] = {
> + regmap_reg_range(GPC_LPCR_A_CORE_BSC,
> + IMX8MP_GPC_PGC_CPU_MAPPING),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_NOC),
> + GPC_PGC_SR(IMX8MP_PGC_NOC)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MIPI1),
> + GPC_PGC_SR(IMX8MP_PGC_MIPI1)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_PCIE),
> + GPC_PGC_SR(IMX8MP_PGC_PCIE)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_USB1),
> + GPC_PGC_SR(IMX8MP_PGC_USB1)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_USB2),
> + GPC_PGC_SR(IMX8MP_PGC_USB2)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MLMIX),
> + GPC_PGC_SR(IMX8MP_PGC_MLMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_AUDIOMIX),
> + GPC_PGC_SR(IMX8MP_PGC_AUDIOMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPU2D),
> + GPC_PGC_SR(IMX8MP_PGC_GPU2D)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPUMIX),
> + GPC_PGC_SR(IMX8MP_PGC_GPUMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPUMIX),
> + GPC_PGC_SR(IMX8MP_PGC_VPUMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_GPU3D),
> + GPC_PGC_SR(IMX8MP_PGC_GPU3D)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MEDIAMIX),
> + GPC_PGC_SR(IMX8MP_PGC_MEDIAMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_G1),
> + GPC_PGC_SR(IMX8MP_PGC_VPU_G1)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_G2),
> + GPC_PGC_SR(IMX8MP_PGC_VPU_G2)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_VPU_VC8000E),
> + GPC_PGC_SR(IMX8MP_PGC_VPU_VC8000E)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HDMIMIX),
> + GPC_PGC_SR(IMX8MP_PGC_HDMIMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HDMI),
> + GPC_PGC_SR(IMX8MP_PGC_HDMI)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MIPI2),
> + GPC_PGC_SR(IMX8MP_PGC_MIPI2)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_HSIOMIX),
> + GPC_PGC_SR(IMX8MP_PGC_HSIOMIX)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_MEDIA_ISP_DWP),
> + GPC_PGC_SR(IMX8MP_PGC_MEDIA_ISP_DWP)),
> + regmap_reg_range(GPC_PGC_CTRL(IMX8MP_PGC_DDRMIX),
> + GPC_PGC_SR(IMX8MP_PGC_DDRMIX)),
> +};
> +
> +static const struct regmap_access_table imx8mp_access_table = {
> + .yes_ranges = imx8mp_yes_ranges,
> + .n_yes_ranges = ARRAY_SIZE(imx8mp_yes_ranges),
> +};
> +
> +static const struct imx_pgc_regs imx8mp_pgc_regs = {
> + .map = IMX8MP_GPC_PGC_CPU_MAPPING,
> + .pup = IMX8MP_GPC_PU_PGC_SW_PUP_REQ,
> + .pdn = IMX8MP_GPC_PU_PGC_SW_PDN_REQ,
> + .hsk = IMX8MP_GPC_PU_PWRHSK,
> +};
> +static const struct imx_pgc_domain_data imx8mp_pgc_domain_data = {
> + .domains = imx8mp_pgc_domains,
> + .domains_num = ARRAY_SIZE(imx8mp_pgc_domains),
> + .reg_access_table = &imx8mp_access_table,
> + .pgc_regs = &imx8mp_pgc_regs,
> +};
> +
> static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
> [IMX8MN_POWER_DOMAIN_HSIOMIX] = {
> .genpd = {
> @@ -1119,6 +1502,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
> domain = pd_pdev->dev.platform_data;
> domain->regmap = regmap;
> domain->regs = domain_data->pgc_regs;
> +
> domain->genpd.power_on = imx_pgc_power_up;
> domain->genpd.power_off = imx_pgc_power_down;
>
> @@ -1140,6 +1524,7 @@ static const struct of_device_id imx_gpcv2_dt_ids[] = {
> { .compatible = "fsl,imx7d-gpc", .data = &imx7_pgc_domain_data, },
> { .compatible = "fsl,imx8mm-gpc", .data = &imx8mm_pgc_domain_data, },
> { .compatible = "fsl,imx8mn-gpc", .data = &imx8mn_pgc_domain_data, },
> + { .compatible = "fsl,imx8mp-gpc", .data = &imx8mp_pgc_domain_data, },
> { .compatible = "fsl,imx8mq-gpc", .data = &imx8m_pgc_domain_data, },
> { }
> };
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
2022-02-18 23:02 ` Laurent Pinchart
@ 2022-02-20 11:35 ` Laurent Pinchart
2022-02-21 10:09 ` Lucas Stach
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-20 11:35 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
On Sat, Feb 19, 2022 at 01:02:21AM +0200, Laurent Pinchart wrote:
> On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> > This adds driver support for all the GPC power domains found on
> > the i.MX8MP SoC.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 386 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > index 01f46b078df3..a7c92bdfc53b 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c
[snip]
> > @@ -137,6 +183,21 @@
> > #define IMX8MN_DISPMIX_HSK_PWRDNREQN BIT(7)
> > #define IMX8MN_HSIO_HSK_PWRDNREQN BIT(5)
> >
> > +#define IMX8MP_MEDIAMIX_PWRDNACKN BIT(3)
>
> This should be bit 30.
With this fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
with a soon to be posted driver for the MEDIA_BLK_CTRL.
While this shouldn't be a blocker, I'm wondering how we should deal with
the NOC configuration that TF-A handles in the power domain code ([1]).
The reference manual doesn't document the registers, which doesn't help.
There are also two registers in the MEDIA_BLK_CTRL that are specific to
the LCDIF and ISI, see [2]. Would you recommend dealing with them in the
imx8m-blk-ctrl driver (maybe in the power domain notifier, the same way
we set bit 8 in the CLK_EN register), or through a syscon phandle
directly in the LCDIF and ISI drivers ?
[1] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n156
[2] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n146
> > +#define IMX8MP_HDMIMIX_PWRDNACKN BIT(29)
> > +#define IMX8MP_HSIOMIX_PWRDNACKN BIT(28)
> > +#define IMX8MP_VPUMIX_PWRDNACKN BIT(26)
> > +#define IMX8MP_GPUMIX_PWRDNACKN BIT(25)
> > +#define IMX8MP_MLMIX_PWRDNACKN (BIT(23) | BIT(24))
> > +#define IMX8MP_AUDIOMIX_PWRDNACKN (BIT(20) | BIT(31))
> > +#define IMX8MP_MEDIAMIX_PWRDNREQN BIT(14)
> > +#define IMX8MP_HDMIMIX_PWRDNREQN BIT(13)
> > +#define IMX8MP_HSIOMIX_PWRDNREQN BIT(12)
> > +#define IMX8MP_VPUMIX_PWRDNREQN BIT(10)
> > +#define IMX8MP_GPUMIX_PWRDNREQN BIT(9)
> > +#define IMX8MP_MLMIX_PWRDNREQN (BIT(7) | BIT(8))
> > +#define IMX8MP_AUDIOMIX_PWRDNREQN (BIT(4) | BIT(15))
> > +
> > /*
> > * The PGC offset values in Reference Manual
> > * (Rev. 1, 01/2018 and the older ones) GPC chapter's
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
2022-02-20 11:35 ` Laurent Pinchart
@ 2022-02-21 10:09 ` Lucas Stach
2022-02-21 10:57 ` Laurent Pinchart
0 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2022-02-21 10:09 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Laurent,
Am Sonntag, dem 20.02.2022 um 13:35 +0200 schrieb Laurent Pinchart:
> Hi Lucas,
>
> On Sat, Feb 19, 2022 at 01:02:21AM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> > > This adds driver support for all the GPC power domains found on
> > > the i.MX8MP SoC.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > > drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 386 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > > index 01f46b078df3..a7c92bdfc53b 100644
> > > --- a/drivers/soc/imx/gpcv2.c
> > > +++ b/drivers/soc/imx/gpcv2.c
>
> [snip]
>
> > > @@ -137,6 +183,21 @@
> > > #define IMX8MN_DISPMIX_HSK_PWRDNREQN BIT(7)
> > > #define IMX8MN_HSIO_HSK_PWRDNREQN BIT(5)
> > >
> > > +#define IMX8MP_MEDIAMIX_PWRDNACKN BIT(3)
> >
> > This should be bit 30.
>
> With this fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> with a soon to be posted driver for the MEDIA_BLK_CTRL.
>
> While this shouldn't be a blocker, I'm wondering how we should deal with
> the NOC configuration that TF-A handles in the power domain code ([1]).
> The reference manual doesn't document the registers, which doesn't help.
>
Yes, that doesn't help. My hope was that at some point we could get
around to add proper interconnect drivers for those NoC nodes and have
the description for those scheduling parameters in the DT, but without
any documentation this will probably be a hard nut to crack.
> There are also two registers in the MEDIA_BLK_CTRL that are specific to
> the LCDIF and ISI, see [2]. Would you recommend dealing with them in the
> imx8m-blk-ctrl driver (maybe in the power domain notifier, the same way
> we set bit 8 in the CLK_EN register), or through a syscon phandle
> directly in the LCDIF and ISI drivers ?
For now I think it would be good enough to initialize those registers
in the power domain notifier. I don't think the ISI or LCDIF drivers
have any more information available that would make it beneficial to
change those values on the fly. As long as they are just static init
values, writing them once from the PM notifier should be good enough.
Regards,
Lucas
>
> [1] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n156
> [2] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n146
>
> > > +#define IMX8MP_HDMIMIX_PWRDNACKN BIT(29)
> > > +#define IMX8MP_HSIOMIX_PWRDNACKN BIT(28)
> > > +#define IMX8MP_VPUMIX_PWRDNACKN BIT(26)
> > > +#define IMX8MP_GPUMIX_PWRDNACKN BIT(25)
> > > +#define IMX8MP_MLMIX_PWRDNACKN (BIT(23) | BIT(24))
> > > +#define IMX8MP_AUDIOMIX_PWRDNACKN (BIT(20) | BIT(31))
> > > +#define IMX8MP_MEDIAMIX_PWRDNREQN BIT(14)
> > > +#define IMX8MP_HDMIMIX_PWRDNREQN BIT(13)
> > > +#define IMX8MP_HSIOMIX_PWRDNREQN BIT(12)
> > > +#define IMX8MP_VPUMIX_PWRDNREQN BIT(10)
> > > +#define IMX8MP_GPUMIX_PWRDNREQN BIT(9)
> > > +#define IMX8MP_MLMIX_PWRDNREQN (BIT(7) | BIT(8))
> > > +#define IMX8MP_AUDIOMIX_PWRDNREQN (BIT(4) | BIT(15))
> > > +
> > > /*
> > > * The PGC offset values in Reference Manual
> > > * (Rev. 1, 01/2018 and the older ones) GPC chapter's
>
> [snip]
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
2022-02-21 10:09 ` Lucas Stach
@ 2022-02-21 10:57 ` Laurent Pinchart
0 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 10:57 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst,
Paul Elder
Hi Lucas,
On Mon, Feb 21, 2022 at 11:09:58AM +0100, Lucas Stach wrote:
> Am Sonntag, dem 20.02.2022 um 13:35 +0200 schrieb Laurent Pinchart:
> > On Sat, Feb 19, 2022 at 01:02:21AM +0200, Laurent Pinchart wrote:
> > > On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> > > > This adds driver support for all the GPC power domains found on
> > > > the i.MX8MP SoC.
> > > >
> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > > ---
> > > > drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 386 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > > > index 01f46b078df3..a7c92bdfc53b 100644
> > > > --- a/drivers/soc/imx/gpcv2.c
> > > > +++ b/drivers/soc/imx/gpcv2.c
> >
> > [snip]
> >
> > > > @@ -137,6 +183,21 @@
> > > > #define IMX8MN_DISPMIX_HSK_PWRDNREQN BIT(7)
> > > > #define IMX8MN_HSIO_HSK_PWRDNREQN BIT(5)
> > > >
> > > > +#define IMX8MP_MEDIAMIX_PWRDNACKN BIT(3)
> > >
> > > This should be bit 30.
> >
> > With this fixed,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > with a soon to be posted driver for the MEDIA_BLK_CTRL.
> >
> > While this shouldn't be a blocker, I'm wondering how we should deal with
> > the NOC configuration that TF-A handles in the power domain code ([1]).
> > The reference manual doesn't document the registers, which doesn't help.
>
> Yes, that doesn't help. My hope was that at some point we could get
> around to add proper interconnect drivers for those NoC nodes and have
> the description for those scheduling parameters in the DT, but without
> any documentation this will probably be a hard nut to crack.
Indeed. Maybe someone from NXP could help :-)
> > There are also two registers in the MEDIA_BLK_CTRL that are specific to
> > the LCDIF and ISI, see [2]. Would you recommend dealing with them in the
> > imx8m-blk-ctrl driver (maybe in the power domain notifier, the same way
> > we set bit 8 in the CLK_EN register), or through a syscon phandle
> > directly in the LCDIF and ISI drivers ?
>
> For now I think it would be good enough to initialize those registers
> in the power domain notifier. I don't think the ISI or LCDIF drivers
> have any more information available that would make it beneficial to
> change those values on the fly. As long as they are just static init
> values, writing them once from the PM notifier should be good enough.
Sounds good to me. Paul, could you do so when posting the MEDIA_BLK_CTRL
driver ?
> > [1] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n156
> > [2] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n146
> >
> > > > +#define IMX8MP_HDMIMIX_PWRDNACKN BIT(29)
> > > > +#define IMX8MP_HSIOMIX_PWRDNACKN BIT(28)
> > > > +#define IMX8MP_VPUMIX_PWRDNACKN BIT(26)
> > > > +#define IMX8MP_GPUMIX_PWRDNACKN BIT(25)
> > > > +#define IMX8MP_MLMIX_PWRDNACKN (BIT(23) | BIT(24))
> > > > +#define IMX8MP_AUDIOMIX_PWRDNACKN (BIT(20) | BIT(31))
> > > > +#define IMX8MP_MEDIAMIX_PWRDNREQN BIT(14)
> > > > +#define IMX8MP_HDMIMIX_PWRDNREQN BIT(13)
> > > > +#define IMX8MP_HSIOMIX_PWRDNREQN BIT(12)
> > > > +#define IMX8MP_VPUMIX_PWRDNREQN BIT(10)
> > > > +#define IMX8MP_GPUMIX_PWRDNREQN BIT(9)
> > > > +#define IMX8MP_MLMIX_PWRDNREQN (BIT(7) | BIT(8))
> > > > +#define IMX8MP_AUDIOMIX_PWRDNREQN (BIT(4) | BIT(15))
> > > > +
> > > > /*
> > > > * The PGC offset values in Reference Manual
> > > > * (Rev. 1, 01/2018 and the older ones) GPC chapter's
> >
> > [snip]
> >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
2022-02-07 19:25 ` [PATCH v2 2/9] dt-bindings: power: add defines for i.MX8MP power domain Lucas Stach
2022-02-07 19:25 ` [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-11 16:28 ` Rob Herring
2022-02-13 2:03 ` Shawn Guo
2022-02-07 19:25 ` [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl Lucas Stach
` (5 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
This adds the defines for the power domains provided by the HSIO
blk-ctrl on the i.MX8MP.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
include/dt-bindings/power/imx8mp-power.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/dt-bindings/power/imx8mp-power.h b/include/dt-bindings/power/imx8mp-power.h
index 7c67689e4faf..9f90c40a2c6c 100644
--- a/include/dt-bindings/power/imx8mp-power.h
+++ b/include/dt-bindings/power/imx8mp-power.h
@@ -26,4 +26,10 @@
#define IMX8MP_POWER_DOMAIN_HSIOMIX 17
#define IMX8MP_POWER_DOMAIN_MEDIAMIX_ISPDWP 18
+#define IMX8MP_HSIOBLK_PD_USB 0
+#define IMX8MP_HSIOBLK_PD_USB_PHY1 1
+#define IMX8MP_HSIOBLK_PD_USB_PHY2 2
+#define IMX8MP_HSIOBLK_PD_PCIE 3
+#define IMX8MP_HSIOBLK_PD_PCIE_PHY 4
+
#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains
2022-02-07 19:25 ` [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains Lucas Stach
@ 2022-02-11 16:28 ` Rob Herring
2022-02-13 2:03 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Rob Herring @ 2022-02-11 16:28 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, linux-arm-kernel, devicetree, patchwork-lst,
Fabio Estevam, NXP Linux Team, Rob Herring,
Pengutronix Kernel Team
On Mon, 07 Feb 2022 20:25:42 +0100, Lucas Stach wrote:
> This adds the defines for the power domains provided by the HSIO
> blk-ctrl on the i.MX8MP.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> include/dt-bindings/power/imx8mp-power.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains
2022-02-07 19:25 ` [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains Lucas Stach
2022-02-11 16:28 ` Rob Herring
@ 2022-02-13 2:03 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Shawn Guo @ 2022-02-13 2:03 UTC (permalink / raw)
To: Lucas Stach
Cc: Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
On Mon, Feb 07, 2022 at 08:25:42PM +0100, Lucas Stach wrote:
> This adds the defines for the power domains provided by the HSIO
> blk-ctrl on the i.MX8MP.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Applied, thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (2 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 4/9] dt-bindings: power: imx8mp: add defines for HSIO blk-ctrl domains Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-11 16:29 ` Rob Herring
2022-02-13 2:04 ` Shawn Guo
2022-02-07 19:25 ` [PATCH v2 6/9] soc: imx: add " Lucas Stach
` (4 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
This adds the binding for the HSIO blk-ctrl on the i.MX8MP SoC.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
.../soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 78 +++++++++++++++++++
1 file changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
new file mode 100644
index 000000000000..c1e29d94f40e
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MP HSIO blk-ctrl
+
+maintainers:
+ - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+ The i.MX8MP HSIO blk-ctrl is a top-level peripheral providing access to
+ the NoC and ensuring proper power sequencing of the high-speed IO
+ (USB an PCIe) peripherals located in the HSIO domain of the SoC.
+
+properties:
+ compatible:
+ items:
+ - const: fsl,imx8mp-hsio-blk-ctrl
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+ '#power-domain-cells':
+ const: 1
+
+ power-domains:
+ minItems: 6
+ maxItems: 6
+
+ power-domain-names:
+ items:
+ - const: bus
+ - const: usb
+ - const: usb-phy1
+ - const: usb-phy2
+ - const: pcie
+ - const: pcie-phy
+
+ clocks:
+ minItems: 2
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: usb
+ - const: pcie
+
+required:
+ - compatible
+ - reg
+ - power-domains
+ - power-domain-names
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx8mp-clock.h>
+ #include <dt-bindings/power/imx8mp-power.h>
+
+ hsio_blk_ctrl: blk-ctrl@32f10000 {
+ compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
+ reg = <0x32f10000 0x24>;
+ clocks = <&clk IMX8MP_CLK_USB_ROOT>,
+ <&clk IMX8MP_CLK_PCIE_ROOT>;
+ clock-names = "usb", "pcie";
+ power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
+ <&pgc_usb1_phy>, <&pgc_usb2_phy>,
+ <&pgc_hsiomix>, <&pgc_pcie_phy>;
+ power-domain-names = "bus", "usb", "usb-phy1",
+ "usb-phy2", "pcie", "pcie-phy";
+ #power-domain-cells = <1>;
+ };
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 ` [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl Lucas Stach
@ 2022-02-11 16:29 ` Rob Herring
2022-02-13 2:04 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Rob Herring @ 2022-02-11 16:29 UTC (permalink / raw)
To: Lucas Stach
Cc: patchwork-lst, NXP Linux Team, Rob Herring, devicetree,
Pengutronix Kernel Team, linux-arm-kernel, Shawn Guo,
Fabio Estevam
On Mon, 07 Feb 2022 20:25:43 +0100, Lucas Stach wrote:
> This adds the binding for the HSIO blk-ctrl on the i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> .../soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-hsio-blk-ctrl.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 ` [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl Lucas Stach
2022-02-11 16:29 ` Rob Herring
@ 2022-02-13 2:04 ` Shawn Guo
1 sibling, 0 replies; 32+ messages in thread
From: Shawn Guo @ 2022-02-13 2:04 UTC (permalink / raw)
To: Lucas Stach
Cc: Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
On Mon, Feb 07, 2022 at 08:25:43PM +0100, Lucas Stach wrote:
> This adds the binding for the HSIO blk-ctrl on the i.MX8MP SoC.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Applied, thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 6/9] soc: imx: add i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (3 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 5/9] dt-bindings: soc: add binding for i.MX8MP HSIO blk-ctrl Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-18 21:26 ` Laurent Pinchart
2022-02-21 11:29 ` Laurent Pinchart
2022-02-07 19:25 ` [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains Lucas Stach
` (3 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
The i.MX8MP added some blk-ctrl peripherals that don't follow the regular
structure of the blk-ctrls in the previous SoCs. Add a new file for those
with currently only the HSIO blk-ctrl being supported. Others will be added
later on.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/soc/imx/Makefile | 1 +
drivers/soc/imx/imx8mp-blk-ctrl.c | 444 ++++++++++++++++++++++++++++++
2 files changed, 445 insertions(+)
create mode 100644 drivers/soc/imx/imx8mp-blk-ctrl.c
diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index 8a707077914c..63cd29f6d4d2 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
+obj-$(CONFIG_SOC_IMX8M) += imx8mp-blk-ctrl.o
diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
new file mode 100644
index 000000000000..7f4e1a151d2b
--- /dev/null
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+
+#include <dt-bindings/power/imx8mp-power.h>
+
+#define GPR_REG0 0x0
+#define PCIE_CLOCK_MODULE_EN BIT(0)
+#define USB_CLOCK_MODULE_EN BIT(1)
+
+struct imx8mp_hsio_blk_ctrl_domain;
+
+struct imx8mp_hsio_blk_ctrl {
+ struct device *dev;
+ struct notifier_block power_nb;
+ struct device *bus_power_dev;
+ struct regmap *regmap;
+ struct imx8mp_hsio_blk_ctrl_domain *domains;
+ struct genpd_onecell_data onecell_data;
+};
+
+struct imx8mp_hsio_blk_ctrl_domain_data {
+ const char *name;
+ const char *clk_name;
+ const char *gpc_name;
+};
+
+struct imx8mp_hsio_blk_ctrl_domain {
+ struct generic_pm_domain genpd;
+ struct clk *clk;
+ struct device *power_dev;
+ struct imx8mp_hsio_blk_ctrl *bc;
+ int id;
+};
+
+static inline struct imx8mp_hsio_blk_ctrl_domain *
+to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
+{
+ return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
+}
+
+static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
+{
+ struct imx8mp_hsio_blk_ctrl_domain *domain =
+ to_imx8mp_hsio_blk_ctrl_domain(genpd);
+ struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
+ int ret;
+
+ /* make sure bus domain is awake */
+ ret = pm_runtime_get_sync(bc->bus_power_dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(bc->bus_power_dev);
+ dev_err(bc->dev, "failed to power up bus domain\n");
+ return ret;
+ }
+
+ /* enable upstream and blk-ctrl clocks */
+ ret = clk_prepare_enable(domain->clk);
+ if (ret) {
+ dev_err(bc->dev, "failed to enable clocks\n");
+ goto bus_put;
+ }
+
+ switch (domain->id) {
+ case IMX8MP_HSIOBLK_PD_USB:
+ regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE:
+ regmap_set_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
+ break;
+ default:
+ break;
+ }
+
+ /* power up upstream GPC domain */
+ ret = pm_runtime_get_sync(domain->power_dev);
+ if (ret < 0) {
+ dev_err(bc->dev, "failed to power up peripheral domain\n");
+ goto clk_disable;
+ }
+
+ return 0;
+
+clk_disable:
+ clk_disable_unprepare(domain->clk);
+bus_put:
+ pm_runtime_put(bc->bus_power_dev);
+
+ return ret;
+}
+
+static int imx8mp_hsio_blk_ctrl_power_off(struct generic_pm_domain *genpd)
+{
+ struct imx8mp_hsio_blk_ctrl_domain *domain =
+ to_imx8mp_hsio_blk_ctrl_domain(genpd);
+ struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
+
+ /* disable clocks */
+ switch (domain->id) {
+ case IMX8MP_HSIOBLK_PD_USB:
+ regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+ break;
+ case IMX8MP_HSIOBLK_PD_PCIE:
+ regmap_clear_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
+ break;
+ default:
+ break;
+ }
+
+ clk_disable_unprepare(domain->clk);
+
+ /* power down upstream GPC domain */
+ pm_runtime_put(domain->power_dev);
+
+ /* allow bus domain to suspend */
+ pm_runtime_put(bc->bus_power_dev);
+
+ return 0;
+}
+
+static struct generic_pm_domain *
+imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
+{
+ struct genpd_onecell_data *onecell_data = data;
+ unsigned int index = args->args[0];
+
+ if (args->args_count != 1 ||
+ index >= onecell_data->num_domains)
+ return ERR_PTR(-EINVAL);
+
+ return onecell_data->domains[index];
+}
+
+static struct lock_class_key blk_ctrl_genpd_lock_class;
+
+static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
+ [IMX8MP_HSIOBLK_PD_USB] = {
+ .name = "hsioblk-usb",
+ .clk_name = "usb",
+ .gpc_name = "usb",
+ },
+ [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
+ .name = "hsioblk-ubs-phy1",
+ .gpc_name = "usb-phy1",
+ },
+ [IMX8MP_HSIOBLK_PD_USB_PHY2] = {
+ .name = "hsioblk-ubs-phy2",
+ .gpc_name = "usb-phy2",
+ },
+ [IMX8MP_HSIOBLK_PD_PCIE] = {
+ .name = "hsioblk-pcie",
+ .clk_name = "pcie",
+ .gpc_name = "pcie",
+ },
+ [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
+ .name = "hsioblk-pcie-phy",
+ .gpc_name = "pcie-phy",
+ },
+};
+
+static int imx8mp_hsio_power_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct imx8mp_hsio_blk_ctrl *bc = container_of(nb, struct imx8mp_hsio_blk_ctrl,
+ power_nb);
+ struct clk *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clk;
+ int ret;
+
+ switch (action) {
+ case GENPD_NOTIFY_ON:
+ /*
+ * enable USB clock for a moment for the power-on ADB handshake
+ * to proceed
+ */
+ ret = clk_prepare_enable(usb_clk);
+ if (ret)
+ return NOTIFY_BAD;
+ regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+
+ udelay(5);
+
+ regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+ clk_disable_unprepare(usb_clk);
+ break;
+ case GENPD_NOTIFY_PRE_OFF:
+ /* enable USB clock for the power-down ADB handshake to work */
+ ret = clk_prepare_enable(usb_clk);
+ if (ret)
+ return NOTIFY_BAD;
+
+ regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
+ break;
+ case GENPD_NOTIFY_OFF:
+ clk_disable_unprepare(usb_clk);
+ break;
+ default:
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
+{
+ int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
+ struct device *dev = &pdev->dev;
+ struct imx8mp_hsio_blk_ctrl *bc;
+ void __iomem *base;
+ int i, ret;
+
+ struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = 0x24,
+ };
+
+ bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
+ if (!bc)
+ return -ENOMEM;
+
+ bc->dev = dev;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
+ if (IS_ERR(bc->regmap))
+ return dev_err_probe(dev, PTR_ERR(bc->regmap),
+ "failed to init regmap\n");
+
+ bc->domains = devm_kcalloc(dev, num_domains,
+ sizeof(struct imx8mp_hsio_blk_ctrl_domain),
+ GFP_KERNEL);
+ if (!bc->domains)
+ return -ENOMEM;
+
+ bc->onecell_data.num_domains = num_domains;
+ bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
+ bc->onecell_data.domains =
+ devm_kcalloc(dev, num_domains,
+ sizeof(struct generic_pm_domain *), GFP_KERNEL);
+ if (!bc->onecell_data.domains)
+ return -ENOMEM;
+
+ bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
+ if (IS_ERR(bc->bus_power_dev))
+ return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
+ "failed to attach power domain\n");
+
+ for (i = 0; i < num_domains; i++) {
+ const struct imx8mp_hsio_blk_ctrl_domain_data *data =
+ &imx8mp_hsio_domain_data[i];
+ struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
+
+ if (data->clk_name) {
+ domain->clk = devm_clk_get(dev, data->clk_name);
+ if (IS_ERR(domain->clk)) {
+ ret = PTR_ERR(domain->clk);
+ dev_err_probe(dev, ret, "failed to get clock\n");
+ goto cleanup_pds;
+ }
+ }
+
+ domain->power_dev =
+ dev_pm_domain_attach_by_name(dev, data->gpc_name);
+ if (IS_ERR(domain->power_dev)) {
+ dev_err_probe(dev, PTR_ERR(domain->power_dev),
+ "failed to attach power domain\n");
+ ret = PTR_ERR(domain->power_dev);
+ goto cleanup_pds;
+ }
+
+ domain->genpd.name = data->name;
+ domain->genpd.power_on = imx8mp_hsio_blk_ctrl_power_on;
+ domain->genpd.power_off = imx8mp_hsio_blk_ctrl_power_off;
+ domain->bc = bc;
+ domain->id = i;
+
+ ret = pm_genpd_init(&domain->genpd, NULL, true);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to init power domain\n");
+ dev_pm_domain_detach(domain->power_dev, true);
+ goto cleanup_pds;
+ }
+
+ /*
+ * We use runtime PM to trigger power on/off of the upstream GPC
+ * domain, as a strict hierarchical parent/child power domain
+ * setup doesn't allow us to meet the sequencing requirements.
+ * This means we have nested locking of genpd locks, without the
+ * nesting being visible at the genpd level, so we need a
+ * separate lock class to make lockdep aware of the fact that
+ * this are separate domain locks that can be nested without a
+ * self-deadlock.
+ */
+ lockdep_set_class(&domain->genpd.mlock,
+ &blk_ctrl_genpd_lock_class);
+
+ bc->onecell_data.domains[i] = &domain->genpd;
+ }
+
+ ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to add power domain provider\n");
+ goto cleanup_pds;
+ }
+
+ bc->power_nb.notifier_call = imx8mp_hsio_power_notifier;
+ ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to add power notifier\n");
+ goto cleanup_provider;
+ }
+
+ dev_set_drvdata(dev, bc);
+
+ return 0;
+
+cleanup_provider:
+ of_genpd_del_provider(dev->of_node);
+cleanup_pds:
+ for (i--; i >= 0; i--) {
+ pm_genpd_remove(&bc->domains[i].genpd);
+ dev_pm_domain_detach(bc->domains[i].power_dev, true);
+ }
+
+ dev_pm_domain_detach(bc->bus_power_dev, true);
+
+ return ret;
+}
+
+static int imx8mp_hsio_blk_ctrl_remove(struct platform_device *pdev)
+{
+ struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
+ int i;
+
+ of_genpd_del_provider(pdev->dev.of_node);
+
+ for (i = 0; bc->onecell_data.num_domains; i++) {
+ struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
+
+ pm_genpd_remove(&domain->genpd);
+ dev_pm_domain_detach(domain->power_dev, true);
+ }
+
+ dev_pm_genpd_remove_notifier(bc->bus_power_dev);
+
+ dev_pm_domain_detach(bc->bus_power_dev, true);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int imx8mp_hsio_blk_ctrl_suspend(struct device *dev)
+{
+ struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
+ int ret, i;
+
+ /*
+ * This may look strange, but is done so the generic PM_SLEEP code
+ * can power down our domains and more importantly power them up again
+ * after resume, without tripping over our usage of runtime PM to
+ * control the upstream GPC domains. Things happen in the right order
+ * in the system suspend/resume paths due to the device parent/child
+ * hierarchy.
+ */
+ ret = pm_runtime_get_sync(bc->bus_power_dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(bc->bus_power_dev);
+ return ret;
+ }
+
+ for (i = 0; i < bc->onecell_data.num_domains; i++) {
+ struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
+
+ ret = pm_runtime_get_sync(domain->power_dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(domain->power_dev);
+ goto out_fail;
+ }
+ }
+
+ return 0;
+
+out_fail:
+ for (i--; i >= 0; i--)
+ pm_runtime_put(bc->domains[i].power_dev);
+
+ pm_runtime_put(bc->bus_power_dev);
+
+ return ret;
+}
+
+static int imx8mp_hsio_blk_ctrl_resume(struct device *dev)
+{
+ struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < bc->onecell_data.num_domains; i++)
+ pm_runtime_put(bc->domains[i].power_dev);
+
+ pm_runtime_put(bc->bus_power_dev);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops imx8mp_hsio_blk_ctrl_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx8mp_hsio_blk_ctrl_suspend,
+ imx8mp_hsio_blk_ctrl_resume)
+};
+
+static const struct of_device_id imx8mp_hsio_blk_ctrl_of_match[] = {
+ {
+ .compatible = "fsl,imx8mp-hsio-blk-ctrl",
+ }, {
+ /* Sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
+
+static struct platform_driver imx8mp_hsio_blk_ctrl_driver = {
+ .probe = imx8mp_hsio_blk_ctrl_probe,
+ .remove = imx8mp_hsio_blk_ctrl_remove,
+ .driver = {
+ .name = "imx8mp-hsio-blk-ctrl",
+ .pm = &imx8mp_hsio_blk_ctrl_pm_ops,
+ .of_match_table = imx8mp_hsio_blk_ctrl_of_match,
+ },
+};
+module_platform_driver(imx8mp_hsio_blk_ctrl_driver);
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 6/9] soc: imx: add i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 ` [PATCH v2 6/9] soc: imx: add " Lucas Stach
@ 2022-02-18 21:26 ` Laurent Pinchart
2022-02-18 21:47 ` Lucas Stach
2022-02-21 11:29 ` Laurent Pinchart
1 sibling, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-18 21:26 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:44PM +0100, Lucas Stach wrote:
> The i.MX8MP added some blk-ctrl peripherals that don't follow the regular
> structure of the blk-ctrls in the previous SoCs. Add a new file for those
> with currently only the HSIO blk-ctrl being supported. Others will be added
> later on.
I'm looking at the i.MX8MP MEDIA_BLK_CTRL, and it looks like it could be
supported in drivers/soc/imx/imx8m-blk-ctrl.c without much issues (given
that I have a working implementation that I'll post soon). Am I missing
something, do you envision that blk-ctrl to be supported in this new
driver, or can it go to imx8m-blk-ctrl.c ?
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/soc/imx/Makefile | 1 +
> drivers/soc/imx/imx8mp-blk-ctrl.c | 444 ++++++++++++++++++++++++++++++
> 2 files changed, 445 insertions(+)
> create mode 100644 drivers/soc/imx/imx8mp-blk-ctrl.c
>
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 8a707077914c..63cd29f6d4d2 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8mp-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> new file mode 100644
> index 000000000000..7f4e1a151d2b
> --- /dev/null
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
> +
> +#include <dt-bindings/power/imx8mp-power.h>
> +
> +#define GPR_REG0 0x0
> +#define PCIE_CLOCK_MODULE_EN BIT(0)
> +#define USB_CLOCK_MODULE_EN BIT(1)
> +
> +struct imx8mp_hsio_blk_ctrl_domain;
> +
> +struct imx8mp_hsio_blk_ctrl {
> + struct device *dev;
> + struct notifier_block power_nb;
> + struct device *bus_power_dev;
> + struct regmap *regmap;
> + struct imx8mp_hsio_blk_ctrl_domain *domains;
> + struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8mp_hsio_blk_ctrl_domain_data {
> + const char *name;
> + const char *clk_name;
> + const char *gpc_name;
> +};
> +
> +struct imx8mp_hsio_blk_ctrl_domain {
> + struct generic_pm_domain genpd;
> + struct clk *clk;
> + struct device *power_dev;
> + struct imx8mp_hsio_blk_ctrl *bc;
> + int id;
> +};
> +
> +static inline struct imx8mp_hsio_blk_ctrl_domain *
> +to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
> +{
> + return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> +{
> + struct imx8mp_hsio_blk_ctrl_domain *domain =
> + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> + int ret;
> +
> + /* make sure bus domain is awake */
> + ret = pm_runtime_get_sync(bc->bus_power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(bc->bus_power_dev);
> + dev_err(bc->dev, "failed to power up bus domain\n");
> + return ret;
> + }
> +
> + /* enable upstream and blk-ctrl clocks */
> + ret = clk_prepare_enable(domain->clk);
> + if (ret) {
> + dev_err(bc->dev, "failed to enable clocks\n");
> + goto bus_put;
> + }
> +
> + switch (domain->id) {
> + case IMX8MP_HSIOBLK_PD_USB:
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case IMX8MP_HSIOBLK_PD_PCIE:
> + regmap_set_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> + break;
> + default:
> + break;
> + }
> +
> + /* power up upstream GPC domain */
> + ret = pm_runtime_get_sync(domain->power_dev);
> + if (ret < 0) {
> + dev_err(bc->dev, "failed to power up peripheral domain\n");
> + goto clk_disable;
> + }
> +
> + return 0;
> +
> +clk_disable:
> + clk_disable_unprepare(domain->clk);
> +bus_put:
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> +{
> + struct imx8mp_hsio_blk_ctrl_domain *domain =
> + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> +
> + /* disable clocks */
> + switch (domain->id) {
> + case IMX8MP_HSIOBLK_PD_USB:
> + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case IMX8MP_HSIOBLK_PD_PCIE:
> + regmap_clear_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> + break;
> + default:
> + break;
> + }
> +
> + clk_disable_unprepare(domain->clk);
> +
> + /* power down upstream GPC domain */
> + pm_runtime_put(domain->power_dev);
> +
> + /* allow bus domain to suspend */
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return 0;
> +}
> +
> +static struct generic_pm_domain *
> +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> +{
> + struct genpd_onecell_data *onecell_data = data;
> + unsigned int index = args->args[0];
> +
> + if (args->args_count != 1 ||
> + index >= onecell_data->num_domains)
> + return ERR_PTR(-EINVAL);
> +
> + return onecell_data->domains[index];
> +}
> +
> +static struct lock_class_key blk_ctrl_genpd_lock_class;
> +
> +static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> + [IMX8MP_HSIOBLK_PD_USB] = {
> + .name = "hsioblk-usb",
> + .clk_name = "usb",
> + .gpc_name = "usb",
> + },
> + [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
> + .name = "hsioblk-ubs-phy1",
> + .gpc_name = "usb-phy1",
> + },
> + [IMX8MP_HSIOBLK_PD_USB_PHY2] = {
> + .name = "hsioblk-ubs-phy2",
> + .gpc_name = "usb-phy2",
> + },
> + [IMX8MP_HSIOBLK_PD_PCIE] = {
> + .name = "hsioblk-pcie",
> + .clk_name = "pcie",
> + .gpc_name = "pcie",
> + },
> + [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
> + .name = "hsioblk-pcie-phy",
> + .gpc_name = "pcie-phy",
> + },
> +};
> +
> +static int imx8mp_hsio_power_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = container_of(nb, struct imx8mp_hsio_blk_ctrl,
> + power_nb);
> + struct clk *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clk;
> + int ret;
> +
> + switch (action) {
> + case GENPD_NOTIFY_ON:
> + /*
> + * enable USB clock for a moment for the power-on ADB handshake
> + * to proceed
> + */
> + ret = clk_prepare_enable(usb_clk);
> + if (ret)
> + return NOTIFY_BAD;
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> +
> + udelay(5);
> +
> + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + clk_disable_unprepare(usb_clk);
> + break;
> + case GENPD_NOTIFY_PRE_OFF:
> + /* enable USB clock for the power-down ADB handshake to work */
> + ret = clk_prepare_enable(usb_clk);
> + if (ret)
> + return NOTIFY_BAD;
> +
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case GENPD_NOTIFY_OFF:
> + clk_disable_unprepare(usb_clk);
> + break;
> + default:
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> +{
> + int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
> + struct device *dev = &pdev->dev;
> + struct imx8mp_hsio_blk_ctrl *bc;
> + void __iomem *base;
> + int i, ret;
> +
> + struct regmap_config regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = 0x24,
> + };
> +
> + bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> + if (!bc)
> + return -ENOMEM;
> +
> + bc->dev = dev;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> + if (IS_ERR(bc->regmap))
> + return dev_err_probe(dev, PTR_ERR(bc->regmap),
> + "failed to init regmap\n");
> +
> + bc->domains = devm_kcalloc(dev, num_domains,
> + sizeof(struct imx8mp_hsio_blk_ctrl_domain),
> + GFP_KERNEL);
> + if (!bc->domains)
> + return -ENOMEM;
> +
> + bc->onecell_data.num_domains = num_domains;
> + bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> + bc->onecell_data.domains =
> + devm_kcalloc(dev, num_domains,
> + sizeof(struct generic_pm_domain *), GFP_KERNEL);
> + if (!bc->onecell_data.domains)
> + return -ENOMEM;
> +
> + bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> + if (IS_ERR(bc->bus_power_dev))
> + return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> + "failed to attach power domain\n");
> +
> + for (i = 0; i < num_domains; i++) {
> + const struct imx8mp_hsio_blk_ctrl_domain_data *data =
> + &imx8mp_hsio_domain_data[i];
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + if (data->clk_name) {
> + domain->clk = devm_clk_get(dev, data->clk_name);
> + if (IS_ERR(domain->clk)) {
> + ret = PTR_ERR(domain->clk);
> + dev_err_probe(dev, ret, "failed to get clock\n");
> + goto cleanup_pds;
> + }
> + }
> +
> + domain->power_dev =
> + dev_pm_domain_attach_by_name(dev, data->gpc_name);
> + if (IS_ERR(domain->power_dev)) {
> + dev_err_probe(dev, PTR_ERR(domain->power_dev),
> + "failed to attach power domain\n");
> + ret = PTR_ERR(domain->power_dev);
> + goto cleanup_pds;
> + }
> +
> + domain->genpd.name = data->name;
> + domain->genpd.power_on = imx8mp_hsio_blk_ctrl_power_on;
> + domain->genpd.power_off = imx8mp_hsio_blk_ctrl_power_off;
> + domain->bc = bc;
> + domain->id = i;
> +
> + ret = pm_genpd_init(&domain->genpd, NULL, true);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to init power domain\n");
> + dev_pm_domain_detach(domain->power_dev, true);
> + goto cleanup_pds;
> + }
> +
> + /*
> + * We use runtime PM to trigger power on/off of the upstream GPC
> + * domain, as a strict hierarchical parent/child power domain
> + * setup doesn't allow us to meet the sequencing requirements.
> + * This means we have nested locking of genpd locks, without the
> + * nesting being visible at the genpd level, so we need a
> + * separate lock class to make lockdep aware of the fact that
> + * this are separate domain locks that can be nested without a
> + * self-deadlock.
> + */
> + lockdep_set_class(&domain->genpd.mlock,
> + &blk_ctrl_genpd_lock_class);
> +
> + bc->onecell_data.domains[i] = &domain->genpd;
> + }
> +
> + ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add power domain provider\n");
> + goto cleanup_pds;
> + }
> +
> + bc->power_nb.notifier_call = imx8mp_hsio_power_notifier;
> + ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add power notifier\n");
> + goto cleanup_provider;
> + }
> +
> + dev_set_drvdata(dev, bc);
> +
> + return 0;
> +
> +cleanup_provider:
> + of_genpd_del_provider(dev->of_node);
> +cleanup_pds:
> + for (i--; i >= 0; i--) {
> + pm_genpd_remove(&bc->domains[i].genpd);
> + dev_pm_domain_detach(bc->domains[i].power_dev, true);
> + }
> +
> + dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_remove(struct platform_device *pdev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> + int i;
> +
> + of_genpd_del_provider(pdev->dev.of_node);
> +
> + for (i = 0; bc->onecell_data.num_domains; i++) {
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + pm_genpd_remove(&domain->genpd);
> + dev_pm_domain_detach(domain->power_dev, true);
> + }
> +
> + dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> +
> + dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int imx8mp_hsio_blk_ctrl_suspend(struct device *dev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> + int ret, i;
> +
> + /*
> + * This may look strange, but is done so the generic PM_SLEEP code
> + * can power down our domains and more importantly power them up again
> + * after resume, without tripping over our usage of runtime PM to
> + * control the upstream GPC domains. Things happen in the right order
> + * in the system suspend/resume paths due to the device parent/child
> + * hierarchy.
> + */
> + ret = pm_runtime_get_sync(bc->bus_power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(bc->bus_power_dev);
> + return ret;
> + }
> +
> + for (i = 0; i < bc->onecell_data.num_domains; i++) {
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + ret = pm_runtime_get_sync(domain->power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(domain->power_dev);
> + goto out_fail;
> + }
> + }
> +
> + return 0;
> +
> +out_fail:
> + for (i--; i >= 0; i--)
> + pm_runtime_put(bc->domains[i].power_dev);
> +
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_resume(struct device *dev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < bc->onecell_data.num_domains; i++)
> + pm_runtime_put(bc->domains[i].power_dev);
> +
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops imx8mp_hsio_blk_ctrl_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(imx8mp_hsio_blk_ctrl_suspend,
> + imx8mp_hsio_blk_ctrl_resume)
> +};
> +
> +static const struct of_device_id imx8mp_hsio_blk_ctrl_of_match[] = {
> + {
> + .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> + }, {
> + /* Sentinel */
> + }
> +};
> +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> +
> +static struct platform_driver imx8mp_hsio_blk_ctrl_driver = {
> + .probe = imx8mp_hsio_blk_ctrl_probe,
> + .remove = imx8mp_hsio_blk_ctrl_remove,
> + .driver = {
> + .name = "imx8mp-hsio-blk-ctrl",
> + .pm = &imx8mp_hsio_blk_ctrl_pm_ops,
> + .of_match_table = imx8mp_hsio_blk_ctrl_of_match,
> + },
> +};
> +module_platform_driver(imx8mp_hsio_blk_ctrl_driver);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 6/9] soc: imx: add i.MX8MP HSIO blk-ctrl
2022-02-18 21:26 ` Laurent Pinchart
@ 2022-02-18 21:47 ` Lucas Stach
0 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-18 21:47 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Laurent,
Am Freitag, dem 18.02.2022 um 23:26 +0200 schrieb Laurent Pinchart:
> Hi Lucas,
>
> Thank you for the patch.
>
> On Mon, Feb 07, 2022 at 08:25:44PM +0100, Lucas Stach wrote:
> > The i.MX8MP added some blk-ctrl peripherals that don't follow the regular
> > structure of the blk-ctrls in the previous SoCs. Add a new file for those
> > with currently only the HSIO blk-ctrl being supported. Others will be added
> > later on.
>
> I'm looking at the i.MX8MP MEDIA_BLK_CTRL, and it looks like it could be
> supported in drivers/soc/imx/imx8m-blk-ctrl.c without much issues (given
> that I have a working implementation that I'll post soon). Am I missing
> something, do you envision that blk-ctrl to be supported in this new
> driver, or can it go to imx8m-blk-ctrl.c ?
The VPU and MEDIA blk-ctrls on the i.MX8MP match the regular pattern
laid out in imx8m-blk-ctrl.c, so both should be added to this already
existing driver. HSIO, HDMI and AUDIO blk-ctrls on the 8MP however have
a different, much less regular register layout, which is why I opted to
add a new driver for those to avoid cluttering the existing driver with
special cases.
In other words: feel free to post your implementation based on
imx8m-blk-ctrl.c, it's exactly the right thing to do.
Regards,
Lucas
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/soc/imx/Makefile | 1 +
> > drivers/soc/imx/imx8mp-blk-ctrl.c | 444 ++++++++++++++++++++++++++++++
> > 2 files changed, 445 insertions(+)
> > create mode 100644 drivers/soc/imx/imx8mp-blk-ctrl.c
> >
> > diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> > index 8a707077914c..63cd29f6d4d2 100644
> > --- a/drivers/soc/imx/Makefile
> > +++ b/drivers/soc/imx/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> > obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> > obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> > obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> > +obj-$(CONFIG_SOC_IMX8M) += imx8mp-blk-ctrl.o
> > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > new file mode 100644
> > index 000000000000..7f4e1a151d2b
> > --- /dev/null
> > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > @@ -0,0 +1,444 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/clk.h>
> > +
> > +#include <dt-bindings/power/imx8mp-power.h>
> > +
> > +#define GPR_REG0 0x0
> > +#define PCIE_CLOCK_MODULE_EN BIT(0)
> > +#define USB_CLOCK_MODULE_EN BIT(1)
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain;
> > +
> > +struct imx8mp_hsio_blk_ctrl {
> > + struct device *dev;
> > + struct notifier_block power_nb;
> > + struct device *bus_power_dev;
> > + struct regmap *regmap;
> > + struct imx8mp_hsio_blk_ctrl_domain *domains;
> > + struct genpd_onecell_data onecell_data;
> > +};
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain_data {
> > + const char *name;
> > + const char *clk_name;
> > + const char *gpc_name;
> > +};
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain {
> > + struct generic_pm_domain genpd;
> > + struct clk *clk;
> > + struct device *power_dev;
> > + struct imx8mp_hsio_blk_ctrl *bc;
> > + int id;
> > +};
> > +
> > +static inline struct imx8mp_hsio_blk_ctrl_domain *
> > +to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
> > +{
> > + return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> > +{
> > + struct imx8mp_hsio_blk_ctrl_domain *domain =
> > + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> > + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> > + int ret;
> > +
> > + /* make sure bus domain is awake */
> > + ret = pm_runtime_get_sync(bc->bus_power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(bc->bus_power_dev);
> > + dev_err(bc->dev, "failed to power up bus domain\n");
> > + return ret;
> > + }
> > +
> > + /* enable upstream and blk-ctrl clocks */
> > + ret = clk_prepare_enable(domain->clk);
> > + if (ret) {
> > + dev_err(bc->dev, "failed to enable clocks\n");
> > + goto bus_put;
> > + }
> > +
> > + switch (domain->id) {
> > + case IMX8MP_HSIOBLK_PD_USB:
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case IMX8MP_HSIOBLK_PD_PCIE:
> > + regmap_set_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + /* power up upstream GPC domain */
> > + ret = pm_runtime_get_sync(domain->power_dev);
> > + if (ret < 0) {
> > + dev_err(bc->dev, "failed to power up peripheral domain\n");
> > + goto clk_disable;
> > + }
> > +
> > + return 0;
> > +
> > +clk_disable:
> > + clk_disable_unprepare(domain->clk);
> > +bus_put:
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> > +{
> > + struct imx8mp_hsio_blk_ctrl_domain *domain =
> > + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> > + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> > +
> > + /* disable clocks */
> > + switch (domain->id) {
> > + case IMX8MP_HSIOBLK_PD_USB:
> > + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case IMX8MP_HSIOBLK_PD_PCIE:
> > + regmap_clear_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + clk_disable_unprepare(domain->clk);
> > +
> > + /* power down upstream GPC domain */
> > + pm_runtime_put(domain->power_dev);
> > +
> > + /* allow bus domain to suspend */
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return 0;
> > +}
> > +
> > +static struct generic_pm_domain *
> > +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> > +{
> > + struct genpd_onecell_data *onecell_data = data;
> > + unsigned int index = args->args[0];
> > +
> > + if (args->args_count != 1 ||
> > + index >= onecell_data->num_domains)
> > + return ERR_PTR(-EINVAL);
> > +
> > + return onecell_data->domains[index];
> > +}
> > +
> > +static struct lock_class_key blk_ctrl_genpd_lock_class;
> > +
> > +static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> > + [IMX8MP_HSIOBLK_PD_USB] = {
> > + .name = "hsioblk-usb",
> > + .clk_name = "usb",
> > + .gpc_name = "usb",
> > + },
> > + [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
> > + .name = "hsioblk-ubs-phy1",
> > + .gpc_name = "usb-phy1",
> > + },
> > + [IMX8MP_HSIOBLK_PD_USB_PHY2] = {
> > + .name = "hsioblk-ubs-phy2",
> > + .gpc_name = "usb-phy2",
> > + },
> > + [IMX8MP_HSIOBLK_PD_PCIE] = {
> > + .name = "hsioblk-pcie",
> > + .clk_name = "pcie",
> > + .gpc_name = "pcie",
> > + },
> > + [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
> > + .name = "hsioblk-pcie-phy",
> > + .gpc_name = "pcie-phy",
> > + },
> > +};
> > +
> > +static int imx8mp_hsio_power_notifier(struct notifier_block *nb,
> > + unsigned long action, void *data)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = container_of(nb, struct imx8mp_hsio_blk_ctrl,
> > + power_nb);
> > + struct clk *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clk;
> > + int ret;
> > +
> > + switch (action) {
> > + case GENPD_NOTIFY_ON:
> > + /*
> > + * enable USB clock for a moment for the power-on ADB handshake
> > + * to proceed
> > + */
> > + ret = clk_prepare_enable(usb_clk);
> > + if (ret)
> > + return NOTIFY_BAD;
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > +
> > + udelay(5);
> > +
> > + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + clk_disable_unprepare(usb_clk);
> > + break;
> > + case GENPD_NOTIFY_PRE_OFF:
> > + /* enable USB clock for the power-down ADB handshake to work */
> > + ret = clk_prepare_enable(usb_clk);
> > + if (ret)
> > + return NOTIFY_BAD;
> > +
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case GENPD_NOTIFY_OFF:
> > + clk_disable_unprepare(usb_clk);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> > +{
> > + int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
> > + struct device *dev = &pdev->dev;
> > + struct imx8mp_hsio_blk_ctrl *bc;
> > + void __iomem *base;
> > + int i, ret;
> > +
> > + struct regmap_config regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x24,
> > + };
> > +
> > + bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> > + if (!bc)
> > + return -ENOMEM;
> > +
> > + bc->dev = dev;
> > +
> > + base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> > + if (IS_ERR(bc->regmap))
> > + return dev_err_probe(dev, PTR_ERR(bc->regmap),
> > + "failed to init regmap\n");
> > +
> > + bc->domains = devm_kcalloc(dev, num_domains,
> > + sizeof(struct imx8mp_hsio_blk_ctrl_domain),
> > + GFP_KERNEL);
> > + if (!bc->domains)
> > + return -ENOMEM;
> > +
> > + bc->onecell_data.num_domains = num_domains;
> > + bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> > + bc->onecell_data.domains =
> > + devm_kcalloc(dev, num_domains,
> > + sizeof(struct generic_pm_domain *), GFP_KERNEL);
> > + if (!bc->onecell_data.domains)
> > + return -ENOMEM;
> > +
> > + bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> > + if (IS_ERR(bc->bus_power_dev))
> > + return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> > + "failed to attach power domain\n");
> > +
> > + for (i = 0; i < num_domains; i++) {
> > + const struct imx8mp_hsio_blk_ctrl_domain_data *data =
> > + &imx8mp_hsio_domain_data[i];
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + if (data->clk_name) {
> > + domain->clk = devm_clk_get(dev, data->clk_name);
> > + if (IS_ERR(domain->clk)) {
> > + ret = PTR_ERR(domain->clk);
> > + dev_err_probe(dev, ret, "failed to get clock\n");
> > + goto cleanup_pds;
> > + }
> > + }
> > +
> > + domain->power_dev =
> > + dev_pm_domain_attach_by_name(dev, data->gpc_name);
> > + if (IS_ERR(domain->power_dev)) {
> > + dev_err_probe(dev, PTR_ERR(domain->power_dev),
> > + "failed to attach power domain\n");
> > + ret = PTR_ERR(domain->power_dev);
> > + goto cleanup_pds;
> > + }
> > +
> > + domain->genpd.name = data->name;
> > + domain->genpd.power_on = imx8mp_hsio_blk_ctrl_power_on;
> > + domain->genpd.power_off = imx8mp_hsio_blk_ctrl_power_off;
> > + domain->bc = bc;
> > + domain->id = i;
> > +
> > + ret = pm_genpd_init(&domain->genpd, NULL, true);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to init power domain\n");
> > + dev_pm_domain_detach(domain->power_dev, true);
> > + goto cleanup_pds;
> > + }
> > +
> > + /*
> > + * We use runtime PM to trigger power on/off of the upstream GPC
> > + * domain, as a strict hierarchical parent/child power domain
> > + * setup doesn't allow us to meet the sequencing requirements.
> > + * This means we have nested locking of genpd locks, without the
> > + * nesting being visible at the genpd level, so we need a
> > + * separate lock class to make lockdep aware of the fact that
> > + * this are separate domain locks that can be nested without a
> > + * self-deadlock.
> > + */
> > + lockdep_set_class(&domain->genpd.mlock,
> > + &blk_ctrl_genpd_lock_class);
> > +
> > + bc->onecell_data.domains[i] = &domain->genpd;
> > + }
> > +
> > + ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to add power domain provider\n");
> > + goto cleanup_pds;
> > + }
> > +
> > + bc->power_nb.notifier_call = imx8mp_hsio_power_notifier;
> > + ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to add power notifier\n");
> > + goto cleanup_provider;
> > + }
> > +
> > + dev_set_drvdata(dev, bc);
> > +
> > + return 0;
> > +
> > +cleanup_provider:
> > + of_genpd_del_provider(dev->of_node);
> > +cleanup_pds:
> > + for (i--; i >= 0; i--) {
> > + pm_genpd_remove(&bc->domains[i].genpd);
> > + dev_pm_domain_detach(bc->domains[i].power_dev, true);
> > + }
> > +
> > + dev_pm_domain_detach(bc->bus_power_dev, true);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_remove(struct platform_device *pdev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> > + int i;
> > +
> > + of_genpd_del_provider(pdev->dev.of_node);
> > +
> > + for (i = 0; bc->onecell_data.num_domains; i++) {
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + pm_genpd_remove(&domain->genpd);
> > + dev_pm_domain_detach(domain->power_dev, true);
> > + }
> > +
> > + dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> > +
> > + dev_pm_domain_detach(bc->bus_power_dev, true);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int imx8mp_hsio_blk_ctrl_suspend(struct device *dev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> > + int ret, i;
> > +
> > + /*
> > + * This may look strange, but is done so the generic PM_SLEEP code
> > + * can power down our domains and more importantly power them up again
> > + * after resume, without tripping over our usage of runtime PM to
> > + * control the upstream GPC domains. Things happen in the right order
> > + * in the system suspend/resume paths due to the device parent/child
> > + * hierarchy.
> > + */
> > + ret = pm_runtime_get_sync(bc->bus_power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(bc->bus_power_dev);
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < bc->onecell_data.num_domains; i++) {
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + ret = pm_runtime_get_sync(domain->power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(domain->power_dev);
> > + goto out_fail;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +out_fail:
> > + for (i--; i >= 0; i--)
> > + pm_runtime_put(bc->domains[i].power_dev);
> > +
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_resume(struct device *dev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> > + int i;
> > +
> > + for (i = 0; i < bc->onecell_data.num_domains; i++)
> > + pm_runtime_put(bc->domains[i].power_dev);
> > +
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops imx8mp_hsio_blk_ctrl_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(imx8mp_hsio_blk_ctrl_suspend,
> > + imx8mp_hsio_blk_ctrl_resume)
> > +};
> > +
> > +static const struct of_device_id imx8mp_hsio_blk_ctrl_of_match[] = {
> > + {
> > + .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> > + }, {
> > + /* Sentinel */
> > + }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> > +
> > +static struct platform_driver imx8mp_hsio_blk_ctrl_driver = {
> > + .probe = imx8mp_hsio_blk_ctrl_probe,
> > + .remove = imx8mp_hsio_blk_ctrl_remove,
> > + .driver = {
> > + .name = "imx8mp-hsio-blk-ctrl",
> > + .pm = &imx8mp_hsio_blk_ctrl_pm_ops,
> > + .of_match_table = imx8mp_hsio_blk_ctrl_of_match,
> > + },
> > +};
> > +module_platform_driver(imx8mp_hsio_blk_ctrl_driver);
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/9] soc: imx: add i.MX8MP HSIO blk-ctrl
2022-02-07 19:25 ` [PATCH v2 6/9] soc: imx: add " Lucas Stach
2022-02-18 21:26 ` Laurent Pinchart
@ 2022-02-21 11:29 ` Laurent Pinchart
2022-02-25 10:52 ` Lucas Stach
1 sibling, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 11:29 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:44PM +0100, Lucas Stach wrote:
> The i.MX8MP added some blk-ctrl peripherals that don't follow the regular
> structure of the blk-ctrls in the previous SoCs. Add a new file for those
> with currently only the HSIO blk-ctrl being supported. Others will be added
> later on.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/soc/imx/Makefile | 1 +
> drivers/soc/imx/imx8mp-blk-ctrl.c | 444 ++++++++++++++++++++++++++++++
> 2 files changed, 445 insertions(+)
> create mode 100644 drivers/soc/imx/imx8mp-blk-ctrl.c
>
> diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> index 8a707077914c..63cd29f6d4d2 100644
> --- a/drivers/soc/imx/Makefile
> +++ b/drivers/soc/imx/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> +obj-$(CONFIG_SOC_IMX8M) += imx8mp-blk-ctrl.o
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> new file mode 100644
> index 000000000000..7f4e1a151d2b
> --- /dev/null
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/clk.h>
clk.h should go first in alphabetical order.
> +
> +#include <dt-bindings/power/imx8mp-power.h>
> +
> +#define GPR_REG0 0x0
> +#define PCIE_CLOCK_MODULE_EN BIT(0)
> +#define USB_CLOCK_MODULE_EN BIT(1)
> +
> +struct imx8mp_hsio_blk_ctrl_domain;
> +
> +struct imx8mp_hsio_blk_ctrl {
> + struct device *dev;
> + struct notifier_block power_nb;
> + struct device *bus_power_dev;
> + struct regmap *regmap;
> + struct imx8mp_hsio_blk_ctrl_domain *domains;
> + struct genpd_onecell_data onecell_data;
> +};
> +
> +struct imx8mp_hsio_blk_ctrl_domain_data {
> + const char *name;
> + const char *clk_name;
> + const char *gpc_name;
> +};
> +
> +struct imx8mp_hsio_blk_ctrl_domain {
> + struct generic_pm_domain genpd;
> + struct clk *clk;
> + struct device *power_dev;
> + struct imx8mp_hsio_blk_ctrl *bc;
> + int id;
> +};
> +
> +static inline struct imx8mp_hsio_blk_ctrl_domain *
> +to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
> +{
> + return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> +{
> + struct imx8mp_hsio_blk_ctrl_domain *domain =
> + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> + int ret;
> +
> + /* make sure bus domain is awake */
> + ret = pm_runtime_get_sync(bc->bus_power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(bc->bus_power_dev);
> + dev_err(bc->dev, "failed to power up bus domain\n");
> + return ret;
> + }
ret = pm_runtime_resume_and_get(bc->bus_power_dev);
if (ret < 0) {
dev_err(bc->dev, "failed to power up bus domain\n");
return ret;
}
Same below and in two locations in imx8mp_hsio_blk_ctrl_suspend().
> +
> + /* enable upstream and blk-ctrl clocks */
> + ret = clk_prepare_enable(domain->clk);
> + if (ret) {
> + dev_err(bc->dev, "failed to enable clocks\n");
> + goto bus_put;
> + }
> +
> + switch (domain->id) {
> + case IMX8MP_HSIOBLK_PD_USB:
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case IMX8MP_HSIOBLK_PD_PCIE:
> + regmap_set_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> + break;
> + default:
> + break;
> + }
> +
> + /* power up upstream GPC domain */
> + ret = pm_runtime_get_sync(domain->power_dev);
> + if (ret < 0) {
This would be missing pm_runtime_put_noidle() if you didn't switch to
pm_runtime_resume_and_get().
> + dev_err(bc->dev, "failed to power up peripheral domain\n");
> + goto clk_disable;
> + }
> +
> + return 0;
> +
> +clk_disable:
> + clk_disable_unprepare(domain->clk);
> +bus_put:
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> +{
> + struct imx8mp_hsio_blk_ctrl_domain *domain =
> + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> +
> + /* disable clocks */
> + switch (domain->id) {
> + case IMX8MP_HSIOBLK_PD_USB:
> + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case IMX8MP_HSIOBLK_PD_PCIE:
> + regmap_clear_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> + break;
> + default:
> + break;
> + }
> +
> + clk_disable_unprepare(domain->clk);
> +
> + /* power down upstream GPC domain */
> + pm_runtime_put(domain->power_dev);
> +
> + /* allow bus domain to suspend */
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return 0;
> +}
> +
> +static struct generic_pm_domain *
> +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> +{
> + struct genpd_onecell_data *onecell_data = data;
> + unsigned int index = args->args[0];
> +
> + if (args->args_count != 1 ||
> + index >= onecell_data->num_domains)
> + return ERR_PTR(-EINVAL);
> +
> + return onecell_data->domains[index];
> +}
> +
> +static struct lock_class_key blk_ctrl_genpd_lock_class;
> +
> +static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> + [IMX8MP_HSIOBLK_PD_USB] = {
> + .name = "hsioblk-usb",
> + .clk_name = "usb",
> + .gpc_name = "usb",
> + },
> + [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
> + .name = "hsioblk-ubs-phy1",
> + .gpc_name = "usb-phy1",
> + },
> + [IMX8MP_HSIOBLK_PD_USB_PHY2] = {
> + .name = "hsioblk-ubs-phy2",
> + .gpc_name = "usb-phy2",
> + },
> + [IMX8MP_HSIOBLK_PD_PCIE] = {
> + .name = "hsioblk-pcie",
> + .clk_name = "pcie",
> + .gpc_name = "pcie",
> + },
> + [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
> + .name = "hsioblk-pcie-phy",
> + .gpc_name = "pcie-phy",
> + },
> +};
> +
> +static int imx8mp_hsio_power_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = container_of(nb, struct imx8mp_hsio_blk_ctrl,
> + power_nb);
> + struct clk *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clk;
> + int ret;
> +
> + switch (action) {
> + case GENPD_NOTIFY_ON:
> + /*
> + * enable USB clock for a moment for the power-on ADB handshake
> + * to proceed
> + */
> + ret = clk_prepare_enable(usb_clk);
> + if (ret)
> + return NOTIFY_BAD;
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> +
> + udelay(5);
> +
> + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + clk_disable_unprepare(usb_clk);
> + break;
> + case GENPD_NOTIFY_PRE_OFF:
> + /* enable USB clock for the power-down ADB handshake to work */
> + ret = clk_prepare_enable(usb_clk);
> + if (ret)
> + return NOTIFY_BAD;
> +
> + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> + break;
> + case GENPD_NOTIFY_OFF:
No need to clear USB_CLOCK_MODULE_EN here ?
> + clk_disable_unprepare(usb_clk);
> + break;
> + default:
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> +{
> + int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
> + struct device *dev = &pdev->dev;
> + struct imx8mp_hsio_blk_ctrl *bc;
> + void __iomem *base;
> + int i, ret;
> +
> + struct regmap_config regmap_config = {
This can be static const
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = 0x24,
> + };
> +
> + bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> + if (!bc)
> + return -ENOMEM;
> +
> + bc->dev = dev;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> + if (IS_ERR(bc->regmap))
> + return dev_err_probe(dev, PTR_ERR(bc->regmap),
> + "failed to init regmap\n");
> +
> + bc->domains = devm_kcalloc(dev, num_domains,
> + sizeof(struct imx8mp_hsio_blk_ctrl_domain),
sizeof(*bc->domains) ? Up to you.
> + GFP_KERNEL);
> + if (!bc->domains)
> + return -ENOMEM;
> +
> + bc->onecell_data.num_domains = num_domains;
> + bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> + bc->onecell_data.domains =
> + devm_kcalloc(dev, num_domains,
> + sizeof(struct generic_pm_domain *), GFP_KERNEL);
> + if (!bc->onecell_data.domains)
> + return -ENOMEM;
> +
> + bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> + if (IS_ERR(bc->bus_power_dev))
> + return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> + "failed to attach power domain\n");
s/attach power domain/attach bus power domain/
> +
> + for (i = 0; i < num_domains; i++) {
> + const struct imx8mp_hsio_blk_ctrl_domain_data *data =
> + &imx8mp_hsio_domain_data[i];
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + if (data->clk_name) {
> + domain->clk = devm_clk_get(dev, data->clk_name);
> + if (IS_ERR(domain->clk)) {
> + ret = PTR_ERR(domain->clk);
> + dev_err_probe(dev, ret, "failed to get clock\n");
"failed to get clock %s\n", data->clk_name
could help debugging issues. Same for the gpc_name below.
With those small issues fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + goto cleanup_pds;
> + }
> + }
> +
> + domain->power_dev =
> + dev_pm_domain_attach_by_name(dev, data->gpc_name);
> + if (IS_ERR(domain->power_dev)) {
> + dev_err_probe(dev, PTR_ERR(domain->power_dev),
> + "failed to attach power domain\n");
> + ret = PTR_ERR(domain->power_dev);
> + goto cleanup_pds;
> + }
> +
> + domain->genpd.name = data->name;
> + domain->genpd.power_on = imx8mp_hsio_blk_ctrl_power_on;
> + domain->genpd.power_off = imx8mp_hsio_blk_ctrl_power_off;
> + domain->bc = bc;
> + domain->id = i;
> +
> + ret = pm_genpd_init(&domain->genpd, NULL, true);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to init power domain\n");
> + dev_pm_domain_detach(domain->power_dev, true);
> + goto cleanup_pds;
> + }
> +
> + /*
> + * We use runtime PM to trigger power on/off of the upstream GPC
> + * domain, as a strict hierarchical parent/child power domain
> + * setup doesn't allow us to meet the sequencing requirements.
> + * This means we have nested locking of genpd locks, without the
> + * nesting being visible at the genpd level, so we need a
> + * separate lock class to make lockdep aware of the fact that
> + * this are separate domain locks that can be nested without a
> + * self-deadlock.
> + */
> + lockdep_set_class(&domain->genpd.mlock,
> + &blk_ctrl_genpd_lock_class);
> +
> + bc->onecell_data.domains[i] = &domain->genpd;
> + }
> +
> + ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add power domain provider\n");
> + goto cleanup_pds;
> + }
> +
> + bc->power_nb.notifier_call = imx8mp_hsio_power_notifier;
> + ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> + if (ret) {
> + dev_err_probe(dev, ret, "failed to add power notifier\n");
> + goto cleanup_provider;
> + }
> +
> + dev_set_drvdata(dev, bc);
> +
> + return 0;
> +
> +cleanup_provider:
> + of_genpd_del_provider(dev->of_node);
> +cleanup_pds:
> + for (i--; i >= 0; i--) {
> + pm_genpd_remove(&bc->domains[i].genpd);
> + dev_pm_domain_detach(bc->domains[i].power_dev, true);
> + }
> +
> + dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_remove(struct platform_device *pdev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> + int i;
> +
> + of_genpd_del_provider(pdev->dev.of_node);
> +
> + for (i = 0; bc->onecell_data.num_domains; i++) {
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + pm_genpd_remove(&domain->genpd);
> + dev_pm_domain_detach(domain->power_dev, true);
> + }
> +
> + dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> +
> + dev_pm_domain_detach(bc->bus_power_dev, true);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int imx8mp_hsio_blk_ctrl_suspend(struct device *dev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> + int ret, i;
> +
> + /*
> + * This may look strange, but is done so the generic PM_SLEEP code
> + * can power down our domains and more importantly power them up again
> + * after resume, without tripping over our usage of runtime PM to
> + * control the upstream GPC domains. Things happen in the right order
> + * in the system suspend/resume paths due to the device parent/child
> + * hierarchy.
> + */
> + ret = pm_runtime_get_sync(bc->bus_power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(bc->bus_power_dev);
> + return ret;
> + }
> +
> + for (i = 0; i < bc->onecell_data.num_domains; i++) {
> + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> +
> + ret = pm_runtime_get_sync(domain->power_dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(domain->power_dev);
> + goto out_fail;
> + }
> + }
> +
> + return 0;
> +
> +out_fail:
> + for (i--; i >= 0; i--)
> + pm_runtime_put(bc->domains[i].power_dev);
> +
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return ret;
> +}
> +
> +static int imx8mp_hsio_blk_ctrl_resume(struct device *dev)
> +{
> + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> + int i;
> +
> + for (i = 0; i < bc->onecell_data.num_domains; i++)
> + pm_runtime_put(bc->domains[i].power_dev);
> +
> + pm_runtime_put(bc->bus_power_dev);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops imx8mp_hsio_blk_ctrl_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(imx8mp_hsio_blk_ctrl_suspend,
> + imx8mp_hsio_blk_ctrl_resume)
> +};
> +
> +static const struct of_device_id imx8mp_hsio_blk_ctrl_of_match[] = {
> + {
> + .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> + }, {
> + /* Sentinel */
> + }
> +};
> +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> +
> +static struct platform_driver imx8mp_hsio_blk_ctrl_driver = {
> + .probe = imx8mp_hsio_blk_ctrl_probe,
> + .remove = imx8mp_hsio_blk_ctrl_remove,
> + .driver = {
> + .name = "imx8mp-hsio-blk-ctrl",
> + .pm = &imx8mp_hsio_blk_ctrl_pm_ops,
> + .of_match_table = imx8mp_hsio_blk_ctrl_of_match,
> + },
> +};
> +module_platform_driver(imx8mp_hsio_blk_ctrl_driver);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 6/9] soc: imx: add i.MX8MP HSIO blk-ctrl
2022-02-21 11:29 ` Laurent Pinchart
@ 2022-02-25 10:52 ` Lucas Stach
0 siblings, 0 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-25 10:52 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Am Montag, dem 21.02.2022 um 13:29 +0200 schrieb Laurent Pinchart:
> Hi Lucas,
>
> Thank you for the patch.
>
> On Mon, Feb 07, 2022 at 08:25:44PM +0100, Lucas Stach wrote:
> > The i.MX8MP added some blk-ctrl peripherals that don't follow the regular
> > structure of the blk-ctrls in the previous SoCs. Add a new file for those
> > with currently only the HSIO blk-ctrl being supported. Others will be added
> > later on.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > drivers/soc/imx/Makefile | 1 +
> > drivers/soc/imx/imx8mp-blk-ctrl.c | 444 ++++++++++++++++++++++++++++++
> > 2 files changed, 445 insertions(+)
> > create mode 100644 drivers/soc/imx/imx8mp-blk-ctrl.c
> >
> > diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
> > index 8a707077914c..63cd29f6d4d2 100644
> > --- a/drivers/soc/imx/Makefile
> > +++ b/drivers/soc/imx/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
> > obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
> > obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o
> > obj-$(CONFIG_SOC_IMX8M) += imx8m-blk-ctrl.o
> > +obj-$(CONFIG_SOC_IMX8M) += imx8mp-blk-ctrl.o
> > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > new file mode 100644
> > index 000000000000..7f4e1a151d2b
> > --- /dev/null
> > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > @@ -0,0 +1,444 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * Copyright 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/regmap.h>
> > +#include <linux/clk.h>
>
> clk.h should go first in alphabetical order.
>
> > +
> > +#include <dt-bindings/power/imx8mp-power.h>
> > +
> > +#define GPR_REG0 0x0
> > +#define PCIE_CLOCK_MODULE_EN BIT(0)
> > +#define USB_CLOCK_MODULE_EN BIT(1)
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain;
> > +
> > +struct imx8mp_hsio_blk_ctrl {
> > + struct device *dev;
> > + struct notifier_block power_nb;
> > + struct device *bus_power_dev;
> > + struct regmap *regmap;
> > + struct imx8mp_hsio_blk_ctrl_domain *domains;
> > + struct genpd_onecell_data onecell_data;
> > +};
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain_data {
> > + const char *name;
> > + const char *clk_name;
> > + const char *gpc_name;
> > +};
> > +
> > +struct imx8mp_hsio_blk_ctrl_domain {
> > + struct generic_pm_domain genpd;
> > + struct clk *clk;
> > + struct device *power_dev;
> > + struct imx8mp_hsio_blk_ctrl *bc;
> > + int id;
> > +};
> > +
> > +static inline struct imx8mp_hsio_blk_ctrl_domain *
> > +to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
> > +{
> > + return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> > +{
> > + struct imx8mp_hsio_blk_ctrl_domain *domain =
> > + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> > + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> > + int ret;
> > +
> > + /* make sure bus domain is awake */
> > + ret = pm_runtime_get_sync(bc->bus_power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(bc->bus_power_dev);
> > + dev_err(bc->dev, "failed to power up bus domain\n");
> > + return ret;
> > + }
>
> ret = pm_runtime_resume_and_get(bc->bus_power_dev);
> if (ret < 0) {
> dev_err(bc->dev, "failed to power up bus domain\n");
> return ret;
> }
>
> Same below and in two locations in imx8mp_hsio_blk_ctrl_suspend().
>
> > +
> > + /* enable upstream and blk-ctrl clocks */
> > + ret = clk_prepare_enable(domain->clk);
> > + if (ret) {
> > + dev_err(bc->dev, "failed to enable clocks\n");
> > + goto bus_put;
> > + }
> > +
> > + switch (domain->id) {
> > + case IMX8MP_HSIOBLK_PD_USB:
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case IMX8MP_HSIOBLK_PD_PCIE:
> > + regmap_set_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + /* power up upstream GPC domain */
> > + ret = pm_runtime_get_sync(domain->power_dev);
> > + if (ret < 0) {
>
> This would be missing pm_runtime_put_noidle() if you didn't switch to
> pm_runtime_resume_and_get().
>
> > + dev_err(bc->dev, "failed to power up peripheral domain\n");
> > + goto clk_disable;
> > + }
> > +
> > + return 0;
> > +
> > +clk_disable:
> > + clk_disable_unprepare(domain->clk);
> > +bus_put:
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_power_off(struct generic_pm_domain *genpd)
> > +{
> > + struct imx8mp_hsio_blk_ctrl_domain *domain =
> > + to_imx8mp_hsio_blk_ctrl_domain(genpd);
> > + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> > +
> > + /* disable clocks */
> > + switch (domain->id) {
> > + case IMX8MP_HSIOBLK_PD_USB:
> > + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case IMX8MP_HSIOBLK_PD_PCIE:
> > + regmap_clear_bits(bc->regmap, GPR_REG0, PCIE_CLOCK_MODULE_EN);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + clk_disable_unprepare(domain->clk);
> > +
> > + /* power down upstream GPC domain */
> > + pm_runtime_put(domain->power_dev);
> > +
> > + /* allow bus domain to suspend */
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return 0;
> > +}
> > +
> > +static struct generic_pm_domain *
> > +imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
> > +{
> > + struct genpd_onecell_data *onecell_data = data;
> > + unsigned int index = args->args[0];
> > +
> > + if (args->args_count != 1 ||
> > + index >= onecell_data->num_domains)
> > + return ERR_PTR(-EINVAL);
> > +
> > + return onecell_data->domains[index];
> > +}
> > +
> > +static struct lock_class_key blk_ctrl_genpd_lock_class;
> > +
> > +static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> > + [IMX8MP_HSIOBLK_PD_USB] = {
> > + .name = "hsioblk-usb",
> > + .clk_name = "usb",
> > + .gpc_name = "usb",
> > + },
> > + [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
> > + .name = "hsioblk-ubs-phy1",
> > + .gpc_name = "usb-phy1",
> > + },
> > + [IMX8MP_HSIOBLK_PD_USB_PHY2] = {
> > + .name = "hsioblk-ubs-phy2",
> > + .gpc_name = "usb-phy2",
> > + },
> > + [IMX8MP_HSIOBLK_PD_PCIE] = {
> > + .name = "hsioblk-pcie",
> > + .clk_name = "pcie",
> > + .gpc_name = "pcie",
> > + },
> > + [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
> > + .name = "hsioblk-pcie-phy",
> > + .gpc_name = "pcie-phy",
> > + },
> > +};
> > +
> > +static int imx8mp_hsio_power_notifier(struct notifier_block *nb,
> > + unsigned long action, void *data)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = container_of(nb, struct imx8mp_hsio_blk_ctrl,
> > + power_nb);
> > + struct clk *usb_clk = bc->domains[IMX8MP_HSIOBLK_PD_USB].clk;
> > + int ret;
> > +
> > + switch (action) {
> > + case GENPD_NOTIFY_ON:
> > + /*
> > + * enable USB clock for a moment for the power-on ADB handshake
> > + * to proceed
> > + */
> > + ret = clk_prepare_enable(usb_clk);
> > + if (ret)
> > + return NOTIFY_BAD;
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > +
> > + udelay(5);
> > +
> > + regmap_clear_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + clk_disable_unprepare(usb_clk);
> > + break;
> > + case GENPD_NOTIFY_PRE_OFF:
> > + /* enable USB clock for the power-down ADB handshake to work */
> > + ret = clk_prepare_enable(usb_clk);
> > + if (ret)
> > + return NOTIFY_BAD;
> > +
> > + regmap_set_bits(bc->regmap, GPR_REG0, USB_CLOCK_MODULE_EN);
> > + break;
> > + case GENPD_NOTIFY_OFF:
>
> No need to clear USB_CLOCK_MODULE_EN here ?
Nope, at this point power to the domain is already collapsed and the
driver must not touch the blk-ctrl anymore. The blk-ctrl is reset
anyway when power is restored in the next runtime PM cycle, so there is
no point in disabling this secondary clock gate.
Regards,
Lucas
>
> > + clk_disable_unprepare(usb_clk);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> > +{
> > + int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
> > + struct device *dev = &pdev->dev;
> > + struct imx8mp_hsio_blk_ctrl *bc;
> > + void __iomem *base;
> > + int i, ret;
> > +
> > + struct regmap_config regmap_config = {
>
> This can be static const
>
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x24,
> > + };
> > +
> > + bc = devm_kzalloc(dev, sizeof(*bc), GFP_KERNEL);
> > + if (!bc)
> > + return -ENOMEM;
> > +
> > + bc->dev = dev;
> > +
> > + base = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + bc->regmap = devm_regmap_init_mmio(dev, base, ®map_config);
> > + if (IS_ERR(bc->regmap))
> > + return dev_err_probe(dev, PTR_ERR(bc->regmap),
> > + "failed to init regmap\n");
> > +
> > + bc->domains = devm_kcalloc(dev, num_domains,
> > + sizeof(struct imx8mp_hsio_blk_ctrl_domain),
>
> sizeof(*bc->domains) ? Up to you.
>
> > + GFP_KERNEL);
> > + if (!bc->domains)
> > + return -ENOMEM;
> > +
> > + bc->onecell_data.num_domains = num_domains;
> > + bc->onecell_data.xlate = imx8m_blk_ctrl_xlate;
> > + bc->onecell_data.domains =
> > + devm_kcalloc(dev, num_domains,
> > + sizeof(struct generic_pm_domain *), GFP_KERNEL);
> > + if (!bc->onecell_data.domains)
> > + return -ENOMEM;
> > +
> > + bc->bus_power_dev = genpd_dev_pm_attach_by_name(dev, "bus");
> > + if (IS_ERR(bc->bus_power_dev))
> > + return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> > + "failed to attach power domain\n");
>
> s/attach power domain/attach bus power domain/
>
> > +
> > + for (i = 0; i < num_domains; i++) {
> > + const struct imx8mp_hsio_blk_ctrl_domain_data *data =
> > + &imx8mp_hsio_domain_data[i];
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + if (data->clk_name) {
> > + domain->clk = devm_clk_get(dev, data->clk_name);
> > + if (IS_ERR(domain->clk)) {
> > + ret = PTR_ERR(domain->clk);
> > + dev_err_probe(dev, ret, "failed to get clock\n");
>
> "failed to get clock %s\n", data->clk_name
>
> could help debugging issues. Same for the gpc_name below.
>
> With those small issues fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > + goto cleanup_pds;
> > + }
> > + }
> > +
> > + domain->power_dev =
> > + dev_pm_domain_attach_by_name(dev, data->gpc_name);
> > + if (IS_ERR(domain->power_dev)) {
> > + dev_err_probe(dev, PTR_ERR(domain->power_dev),
> > + "failed to attach power domain\n");
> > + ret = PTR_ERR(domain->power_dev);
> > + goto cleanup_pds;
> > + }
> > +
> > + domain->genpd.name = data->name;
> > + domain->genpd.power_on = imx8mp_hsio_blk_ctrl_power_on;
> > + domain->genpd.power_off = imx8mp_hsio_blk_ctrl_power_off;
> > + domain->bc = bc;
> > + domain->id = i;
> > +
> > + ret = pm_genpd_init(&domain->genpd, NULL, true);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to init power domain\n");
> > + dev_pm_domain_detach(domain->power_dev, true);
> > + goto cleanup_pds;
> > + }
> > +
> > + /*
> > + * We use runtime PM to trigger power on/off of the upstream GPC
> > + * domain, as a strict hierarchical parent/child power domain
> > + * setup doesn't allow us to meet the sequencing requirements.
> > + * This means we have nested locking of genpd locks, without the
> > + * nesting being visible at the genpd level, so we need a
> > + * separate lock class to make lockdep aware of the fact that
> > + * this are separate domain locks that can be nested without a
> > + * self-deadlock.
> > + */
> > + lockdep_set_class(&domain->genpd.mlock,
> > + &blk_ctrl_genpd_lock_class);
> > +
> > + bc->onecell_data.domains[i] = &domain->genpd;
> > + }
> > +
> > + ret = of_genpd_add_provider_onecell(dev->of_node, &bc->onecell_data);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to add power domain provider\n");
> > + goto cleanup_pds;
> > + }
> > +
> > + bc->power_nb.notifier_call = imx8mp_hsio_power_notifier;
> > + ret = dev_pm_genpd_add_notifier(bc->bus_power_dev, &bc->power_nb);
> > + if (ret) {
> > + dev_err_probe(dev, ret, "failed to add power notifier\n");
> > + goto cleanup_provider;
> > + }
> > +
> > + dev_set_drvdata(dev, bc);
> > +
> > + return 0;
> > +
> > +cleanup_provider:
> > + of_genpd_del_provider(dev->of_node);
> > +cleanup_pds:
> > + for (i--; i >= 0; i--) {
> > + pm_genpd_remove(&bc->domains[i].genpd);
> > + dev_pm_domain_detach(bc->domains[i].power_dev, true);
> > + }
> > +
> > + dev_pm_domain_detach(bc->bus_power_dev, true);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_remove(struct platform_device *pdev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(&pdev->dev);
> > + int i;
> > +
> > + of_genpd_del_provider(pdev->dev.of_node);
> > +
> > + for (i = 0; bc->onecell_data.num_domains; i++) {
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + pm_genpd_remove(&domain->genpd);
> > + dev_pm_domain_detach(domain->power_dev, true);
> > + }
> > +
> > + dev_pm_genpd_remove_notifier(bc->bus_power_dev);
> > +
> > + dev_pm_domain_detach(bc->bus_power_dev, true);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int imx8mp_hsio_blk_ctrl_suspend(struct device *dev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> > + int ret, i;
> > +
> > + /*
> > + * This may look strange, but is done so the generic PM_SLEEP code
> > + * can power down our domains and more importantly power them up again
> > + * after resume, without tripping over our usage of runtime PM to
> > + * control the upstream GPC domains. Things happen in the right order
> > + * in the system suspend/resume paths due to the device parent/child
> > + * hierarchy.
> > + */
> > + ret = pm_runtime_get_sync(bc->bus_power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(bc->bus_power_dev);
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < bc->onecell_data.num_domains; i++) {
> > + struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
> > +
> > + ret = pm_runtime_get_sync(domain->power_dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(domain->power_dev);
> > + goto out_fail;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +out_fail:
> > + for (i--; i >= 0; i--)
> > + pm_runtime_put(bc->domains[i].power_dev);
> > +
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return ret;
> > +}
> > +
> > +static int imx8mp_hsio_blk_ctrl_resume(struct device *dev)
> > +{
> > + struct imx8mp_hsio_blk_ctrl *bc = dev_get_drvdata(dev);
> > + int i;
> > +
> > + for (i = 0; i < bc->onecell_data.num_domains; i++)
> > + pm_runtime_put(bc->domains[i].power_dev);
> > +
> > + pm_runtime_put(bc->bus_power_dev);
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static const struct dev_pm_ops imx8mp_hsio_blk_ctrl_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(imx8mp_hsio_blk_ctrl_suspend,
> > + imx8mp_hsio_blk_ctrl_resume)
> > +};
> > +
> > +static const struct of_device_id imx8mp_hsio_blk_ctrl_of_match[] = {
> > + {
> > + .compatible = "fsl,imx8mp-hsio-blk-ctrl",
> > + }, {
> > + /* Sentinel */
> > + }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> > +
> > +static struct platform_driver imx8mp_hsio_blk_ctrl_driver = {
> > + .probe = imx8mp_hsio_blk_ctrl_probe,
> > + .remove = imx8mp_hsio_blk_ctrl_remove,
> > + .driver = {
> > + .name = "imx8mp-hsio-blk-ctrl",
> > + .pm = &imx8mp_hsio_blk_ctrl_pm_ops,
> > + .of_match_table = imx8mp_hsio_blk_ctrl_of_match,
> > + },
> > +};
> > +module_platform_driver(imx8mp_hsio_blk_ctrl_driver);
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (4 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 6/9] soc: imx: add " Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-19 7:40 ` Laurent Pinchart
2022-02-07 19:25 ` [PATCH v2 8/9] arm64: dts: imx8mp: add GPU power domains Lucas Stach
` (2 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
This adds the GPC and HSIO blk-ctrl nodes providing power control for
the high-speed (USB and PCIe) IOs.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
1 file changed, 57 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6b840c05dd77..dc488a147d0c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -4,6 +4,7 @@
*/
#include <dt-bindings/clock/imx8mp-clock.h>
+#include <dt-bindings/power/imx8mp-power.h>
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -475,6 +476,44 @@ src: reset-controller@30390000 {
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
#reset-cells = <1>;
};
+
+ gpc: gpc@303a0000 {
+ compatible = "fsl,imx8mp-gpc";
+ reg = <0x303a0000 0x10000>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ #interrupt-cells = <3>;
+
+ pgc {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pgc_pcie_phy: power-domain@1 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
+ };
+
+ pgc_usb1_phy: power-domain@2 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
+ };
+
+ pgc_usb2_phy: power-domain@3 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
+ };
+
+ pgc_hsiomix: power-domains@17 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
+ clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
+ <&clk IMX8MP_CLK_HSIO_ROOT>;
+ assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+ assigned-clock-rates = <500000000>;
+ };
+ };
+ };
};
aips2: bus@30400000 {
@@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
};
+ hsio_blk_ctrl: blk-ctrl@32f10000 {
+ compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
+ reg = <0x32f10000 0x24>;
+ clocks = <&clk IMX8MP_CLK_USB_ROOT>,
+ <&clk IMX8MP_CLK_PCIE_ROOT>;
+ clock-names = "usb", "pcie";
+ power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
+ <&pgc_usb1_phy>, <&pgc_usb2_phy>,
+ <&pgc_hsiomix>, <&pgc_pcie_phy>;
+ power-domain-names = "bus", "usb", "usb-phy1",
+ "usb-phy2", "pcie", "pcie-phy";
+ #power-domain-cells = <1>;
+ };
+
usb3_phy0: usb-phy@381f0040 {
compatible = "fsl,imx8mp-usb-phy";
reg = <0x381f0040 0x40>;
@@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
clock-names = "phy";
assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
#phy-cells = <0>;
status = "disabled";
};
@@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
<&clk IMX8MP_CLK_USB_ROOT>;
clock-names = "hsio", "suspend";
interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
#address-cells = <1>;
#size-cells = <1>;
dma-ranges = <0x40000000 0x40000000 0xc0000000>;
@@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
<&clk IMX8MP_CLK_USB_CORE_REF>,
<&clk IMX8MP_CLK_USB_ROOT>;
clock-names = "bus_early", "ref", "suspend";
- assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
- assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
- assigned-clock-rates = <500000000>;
interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
phys = <&usb3_phy0>, <&usb3_phy0>;
phy-names = "usb2-phy", "usb3-phy";
@@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
clock-names = "phy";
assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
#phy-cells = <0>;
};
@@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
<&clk IMX8MP_CLK_USB_ROOT>;
clock-names = "hsio", "suspend";
interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
+ power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
#address-cells = <1>;
#size-cells = <1>;
dma-ranges = <0x40000000 0x40000000 0xc0000000>;
@@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
<&clk IMX8MP_CLK_USB_CORE_REF>,
<&clk IMX8MP_CLK_USB_ROOT>;
clock-names = "bus_early", "ref", "suspend";
- assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
- assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
- assigned-clock-rates = <500000000>;
interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
phys = <&usb3_phy1>, <&usb3_phy1>;
phy-names = "usb2-phy", "usb3-phy";
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-07 19:25 ` [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains Lucas Stach
@ 2022-02-19 7:40 ` Laurent Pinchart
2022-02-20 5:56 ` Laurent Pinchart
2022-02-21 10:05 ` Lucas Stach
0 siblings, 2 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-19 7:40 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:45PM +0100, Lucas Stach wrote:
> This adds the GPC and HSIO blk-ctrl nodes providing power control for
> the high-speed (USB and PCIe) IOs.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
> 1 file changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 6b840c05dd77..dc488a147d0c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -4,6 +4,7 @@
> */
>
> #include <dt-bindings/clock/imx8mp-clock.h>
> +#include <dt-bindings/power/imx8mp-power.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -475,6 +476,44 @@ src: reset-controller@30390000 {
> interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> #reset-cells = <1>;
> };
> +
> + gpc: gpc@303a0000 {
> + compatible = "fsl,imx8mp-gpc";
> + reg = <0x303a0000 0x10000>;
According to the reference manual, the GPC occupies 4kB, not 64kB.
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + #interrupt-cells = <3>;
> +
> + pgc {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
We're working on support for the MEDIAMIX power domains, which we'll
rebase on top of this. In case the HSIO part still needs more work,
could you split this patch in two, with one patch that adds the PGC,
with an empty pgc nodde, and a second patch that adds the HSIO-related
power domains ? The first one could then be merged faster (it would be
great if it could get in v5.18).
> + pgc_pcie_phy: power-domain@1 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
> + };
> +
> + pgc_usb1_phy: power-domain@2 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
> + };
> +
> + pgc_usb2_phy: power-domain@3 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> + };
> +
> + pgc_hsiomix: power-domains@17 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
> + clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> + <&clk IMX8MP_CLK_HSIO_ROOT>;
> + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> + assigned-clock-rates = <500000000>;
> + };
> + };
> + };
> };
>
> aips2: bus@30400000 {
> @@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
> interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> };
>
> + hsio_blk_ctrl: blk-ctrl@32f10000 {
> + compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> + reg = <0x32f10000 0x24>;
> + clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> + <&clk IMX8MP_CLK_PCIE_ROOT>;
> + clock-names = "usb", "pcie";
> + power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
> + <&pgc_usb1_phy>, <&pgc_usb2_phy>,
> + <&pgc_hsiomix>, <&pgc_pcie_phy>;
Would it be useful to rework the driver to avoid specifying the same
parent power domain multiple times in DT ?
> + power-domain-names = "bus", "usb", "usb-phy1",
> + "usb-phy2", "pcie", "pcie-phy";
> + #power-domain-cells = <1>;
> + };
> +
> usb3_phy0: usb-phy@381f0040 {
> compatible = "fsl,imx8mp-usb-phy";
> reg = <0x381f0040 0x40>;
> @@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
> clock-names = "phy";
> assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
> #phy-cells = <0>;
> status = "disabled";
> };
> @@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
> <&clk IMX8MP_CLK_USB_ROOT>;
> clock-names = "hsio", "suspend";
> interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> #address-cells = <1>;
> #size-cells = <1>;
> dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> @@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
> <&clk IMX8MP_CLK_USB_CORE_REF>,
> <&clk IMX8MP_CLK_USB_ROOT>;
> clock-names = "bus_early", "ref", "suspend";
> - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> - assigned-clock-rates = <500000000>;
> interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> phys = <&usb3_phy0>, <&usb3_phy0>;
> phy-names = "usb2-phy", "usb3-phy";
> @@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
> clock-names = "phy";
> assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
> #phy-cells = <0>;
> };
>
> @@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
> <&clk IMX8MP_CLK_USB_ROOT>;
> clock-names = "hsio", "suspend";
> interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> #address-cells = <1>;
> #size-cells = <1>;
> dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> @@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
> <&clk IMX8MP_CLK_USB_CORE_REF>,
> <&clk IMX8MP_CLK_USB_ROOT>;
> clock-names = "bus_early", "ref", "suspend";
> - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> - assigned-clock-rates = <500000000>;
> interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> phys = <&usb3_phy1>, <&usb3_phy1>;
> phy-names = "usb2-phy", "usb3-phy";
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-19 7:40 ` Laurent Pinchart
@ 2022-02-20 5:56 ` Laurent Pinchart
2022-02-21 11:30 ` Laurent Pinchart
2022-02-21 10:05 ` Lucas Stach
1 sibling, 1 reply; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-20 5:56 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
One more comment.
On Sat, Feb 19, 2022 at 09:40:50AM +0200, Laurent Pinchart wrote:
> On Mon, Feb 07, 2022 at 08:25:45PM +0100, Lucas Stach wrote:
> > This adds the GPC and HSIO blk-ctrl nodes providing power control for
> > the high-speed (USB and PCIe) IOs.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
> > 1 file changed, 57 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 6b840c05dd77..dc488a147d0c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include <dt-bindings/clock/imx8mp-clock.h>
> > +#include <dt-bindings/power/imx8mp-power.h>
> > #include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/input/input.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -475,6 +476,44 @@ src: reset-controller@30390000 {
> > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > #reset-cells = <1>;
> > };
> > +
> > + gpc: gpc@303a0000 {
> > + compatible = "fsl,imx8mp-gpc";
> > + reg = <0x303a0000 0x10000>;
>
> According to the reference manual, the GPC occupies 4kB, not 64kB.
>
> > + interrupt-parent = <&gic>;
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > +
> > + pgc {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
>
> We're working on support for the MEDIAMIX power domains, which we'll
> rebase on top of this. In case the HSIO part still needs more work,
> could you split this patch in two, with one patch that adds the PGC,
> with an empty pgc nodde, and a second patch that adds the HSIO-related
> power domains ? The first one could then be merged faster (it would be
> great if it could get in v5.18).
>
> > + pgc_pcie_phy: power-domain@1 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
> > + };
> > +
> > + pgc_usb1_phy: power-domain@2 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
> > + };
> > +
> > + pgc_usb2_phy: power-domain@3 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> > + };
> > +
> > + pgc_hsiomix: power-domains@17 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
> > + clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > + <&clk IMX8MP_CLK_HSIO_ROOT>;
> > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > + assigned-clock-rates = <500000000>;
> > + };
> > + };
> > + };
> > };
> >
> > aips2: bus@30400000 {
> > @@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
> > interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >
> > + hsio_blk_ctrl: blk-ctrl@32f10000 {
> > + compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > + reg = <0x32f10000 0x24>;
> > + clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> > + <&clk IMX8MP_CLK_PCIE_ROOT>;
> > + clock-names = "usb", "pcie";
> > + power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
> > + <&pgc_usb1_phy>, <&pgc_usb2_phy>,
> > + <&pgc_hsiomix>, <&pgc_pcie_phy>;
>
> Would it be useful to rework the driver to avoid specifying the same
> parent power domain multiple times in DT ?
>
> > + power-domain-names = "bus", "usb", "usb-phy1",
> > + "usb-phy2", "pcie", "pcie-phy";
> > + #power-domain-cells = <1>;
> > + };
Shouldn't this be in an aips4 node ?
> > +
> > usb3_phy0: usb-phy@381f0040 {
> > compatible = "fsl,imx8mp-usb-phy";
> > reg = <0x381f0040 0x40>;
> > @@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
> > clock-names = "phy";
> > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
> > #phy-cells = <0>;
> > status = "disabled";
> > };
> > @@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "hsio", "suspend";
> > interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > @@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
> > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "bus_early", "ref", "suspend";
> > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > - assigned-clock-rates = <500000000>;
> > interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > phys = <&usb3_phy0>, <&usb3_phy0>;
> > phy-names = "usb2-phy", "usb3-phy";
> > @@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
> > clock-names = "phy";
> > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
> > #phy-cells = <0>;
> > };
> >
> > @@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "hsio", "suspend";
> > interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > @@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
> > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "bus_early", "ref", "suspend";
> > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > - assigned-clock-rates = <500000000>;
> > interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > phys = <&usb3_phy1>, <&usb3_phy1>;
> > phy-names = "usb2-phy", "usb3-phy";
> >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-20 5:56 ` Laurent Pinchart
@ 2022-02-21 11:30 ` Laurent Pinchart
0 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 11:30 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
On Sun, Feb 20, 2022 at 07:56:11AM +0200, Laurent Pinchart wrote:
> On Sat, Feb 19, 2022 at 09:40:50AM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 07, 2022 at 08:25:45PM +0100, Lucas Stach wrote:
> > > This adds the GPC and HSIO blk-ctrl nodes providing power control for
> > > the high-speed (USB and PCIe) IOs.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
> > > 1 file changed, 57 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index 6b840c05dd77..dc488a147d0c 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -4,6 +4,7 @@
> > > */
> > >
> > > #include <dt-bindings/clock/imx8mp-clock.h>
> > > +#include <dt-bindings/power/imx8mp-power.h>
> > > #include <dt-bindings/gpio/gpio.h>
> > > #include <dt-bindings/input/input.h>
> > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > @@ -475,6 +476,44 @@ src: reset-controller@30390000 {
> > > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > #reset-cells = <1>;
> > > };
> > > +
> > > + gpc: gpc@303a0000 {
> > > + compatible = "fsl,imx8mp-gpc";
> > > + reg = <0x303a0000 0x10000>;
> >
> > According to the reference manual, the GPC occupies 4kB, not 64kB.
> >
> > > + interrupt-parent = <&gic>;
> > > + interrupt-controller;
> > > + #interrupt-cells = <3>;
> > > +
> > > + pgc {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> >
> > We're working on support for the MEDIAMIX power domains, which we'll
> > rebase on top of this. In case the HSIO part still needs more work,
> > could you split this patch in two, with one patch that adds the PGC,
> > with an empty pgc nodde, and a second patch that adds the HSIO-related
> > power domains ? The first one could then be merged faster (it would be
> > great if it could get in v5.18).
> >
> > > + pgc_pcie_phy: power-domain@1 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
> > > + };
> > > +
> > > + pgc_usb1_phy: power-domain@2 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
> > > + };
> > > +
> > > + pgc_usb2_phy: power-domain@3 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> > > + };
> > > +
> > > + pgc_hsiomix: power-domains@17 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
> > > + clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > > + <&clk IMX8MP_CLK_HSIO_ROOT>;
> > > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > + assigned-clock-rates = <500000000>;
> > > + };
> > > + };
> > > + };
> > > };
> > >
> > > aips2: bus@30400000 {
> > > @@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
> > > interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> > > };
> > >
> > > + hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > + compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > > + reg = <0x32f10000 0x24>;
> > > + clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> > > + <&clk IMX8MP_CLK_PCIE_ROOT>;
> > > + clock-names = "usb", "pcie";
> > > + power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
> > > + <&pgc_usb1_phy>, <&pgc_usb2_phy>,
> > > + <&pgc_hsiomix>, <&pgc_pcie_phy>;
> >
> > Would it be useful to rework the driver to avoid specifying the same
> > parent power domain multiple times in DT ?
> >
> > > + power-domain-names = "bus", "usb", "usb-phy1",
> > > + "usb-phy2", "pcie", "pcie-phy";
> > > + #power-domain-cells = <1>;
> > > + };
>
> Shouldn't this be in an aips4 node ?
With the reg size fixed, and this device moved to an aips4 node,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > +
> > > usb3_phy0: usb-phy@381f0040 {
> > > compatible = "fsl,imx8mp-usb-phy";
> > > reg = <0x381f0040 0x40>;
> > > @@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
> > > clock-names = "phy";
> > > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
> > > #phy-cells = <0>;
> > > status = "disabled";
> > > };
> > > @@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "hsio", "suspend";
> > > interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > > @@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
> > > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "bus_early", "ref", "suspend";
> > > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > - assigned-clock-rates = <500000000>;
> > > interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > > phys = <&usb3_phy0>, <&usb3_phy0>;
> > > phy-names = "usb2-phy", "usb3-phy";
> > > @@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
> > > clock-names = "phy";
> > > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
> > > #phy-cells = <0>;
> > > };
> > >
> > > @@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "hsio", "suspend";
> > > interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > > @@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
> > > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "bus_early", "ref", "suspend";
> > > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > - assigned-clock-rates = <500000000>;
> > > interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > > phys = <&usb3_phy1>, <&usb3_phy1>;
> > > phy-names = "usb2-phy", "usb3-phy";
> > >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-19 7:40 ` Laurent Pinchart
2022-02-20 5:56 ` Laurent Pinchart
@ 2022-02-21 10:05 ` Lucas Stach
2022-02-21 11:01 ` Laurent Pinchart
1 sibling, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2022-02-21 10:05 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Laurent,
Am Samstag, dem 19.02.2022 um 09:40 +0200 schrieb Laurent Pinchart:
> Hi Lucas,
>
> Thank you for the patch.
>
> On Mon, Feb 07, 2022 at 08:25:45PM +0100, Lucas Stach wrote:
> > This adds the GPC and HSIO blk-ctrl nodes providing power control for
> > the high-speed (USB and PCIe) IOs.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
> > 1 file changed, 57 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 6b840c05dd77..dc488a147d0c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include <dt-bindings/clock/imx8mp-clock.h>
> > +#include <dt-bindings/power/imx8mp-power.h>
> > #include <dt-bindings/gpio/gpio.h>
> > #include <dt-bindings/input/input.h>
> > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > @@ -475,6 +476,44 @@ src: reset-controller@30390000 {
> > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > #reset-cells = <1>;
> > };
> > +
> > + gpc: gpc@303a0000 {
> > + compatible = "fsl,imx8mp-gpc";
> > + reg = <0x303a0000 0x10000>;
>
> According to the reference manual, the GPC occupies 4kB, not 64kB.
Right, will fix.
>
> > + interrupt-parent = <&gic>;
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > +
> > + pgc {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
>
> We're working on support for the MEDIAMIX power domains, which we'll
> rebase on top of this. In case the HSIO part still needs more work,
> could you split this patch in two, with one patch that adds the PGC,
> with an empty pgc nodde, and a second patch that adds the HSIO-related
> power domains ? The first one could then be merged faster (it would be
> great if it could get in v5.18).
I don't think the HSIO part is controversial. It seems to work well in
my testing and DT binding is already reviewed. Not sure if someone is
going to review it properly, but I hope that we can land it together
with the reset of this series.
>
> > + pgc_pcie_phy: power-domain@1 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
> > + };
> > +
> > + pgc_usb1_phy: power-domain@2 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
> > + };
> > +
> > + pgc_usb2_phy: power-domain@3 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> > + };
> > +
> > + pgc_hsiomix: power-domains@17 {
> > + #power-domain-cells = <0>;
> > + reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
> > + clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > + <&clk IMX8MP_CLK_HSIO_ROOT>;
> > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > + assigned-clock-rates = <500000000>;
> > + };
> > + };
> > + };
> > };
> >
> > aips2: bus@30400000 {
> > @@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
> > interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >
> > + hsio_blk_ctrl: blk-ctrl@32f10000 {
> > + compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > + reg = <0x32f10000 0x24>;
> > + clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> > + <&clk IMX8MP_CLK_PCIE_ROOT>;
> > + clock-names = "usb", "pcie";
> > + power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
> > + <&pgc_usb1_phy>, <&pgc_usb2_phy>,
> > + <&pgc_hsiomix>, <&pgc_pcie_phy>;
>
> Would it be useful to rework the driver to avoid specifying the same
> parent power domain multiple times in DT ?
I don't think so. That's the pattern we used for all the other blk-
ctrls, where each virtual power-domain in the blk-ctrl has its own
handle to a upstream GPC power domain. I don't see why we would want to
change this now.
Regards,
Lucas
>
> > + power-domain-names = "bus", "usb", "usb-phy1",
> > + "usb-phy2", "pcie", "pcie-phy";
> > + #power-domain-cells = <1>;
> > + };
> > +
> > usb3_phy0: usb-phy@381f0040 {
> > compatible = "fsl,imx8mp-usb-phy";
> > reg = <0x381f0040 0x40>;
> > @@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
> > clock-names = "phy";
> > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
> > #phy-cells = <0>;
> > status = "disabled";
> > };
> > @@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "hsio", "suspend";
> > interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > @@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
> > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "bus_early", "ref", "suspend";
> > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > - assigned-clock-rates = <500000000>;
> > interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > phys = <&usb3_phy0>, <&usb3_phy0>;
> > phy-names = "usb2-phy", "usb3-phy";
> > @@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
> > clock-names = "phy";
> > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
> > #phy-cells = <0>;
> > };
> >
> > @@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "hsio", "suspend";
> > interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > @@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
> > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > <&clk IMX8MP_CLK_USB_ROOT>;
> > clock-names = "bus_early", "ref", "suspend";
> > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > - assigned-clock-rates = <500000000>;
> > interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > phys = <&usb3_phy1>, <&usb3_phy1>;
> > phy-names = "usb2-phy", "usb3-phy";
> >
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains
2022-02-21 10:05 ` Lucas Stach
@ 2022-02-21 11:01 ` Laurent Pinchart
0 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 11:01 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
On Mon, Feb 21, 2022 at 11:05:00AM +0100, Lucas Stach wrote:
> Am Samstag, dem 19.02.2022 um 09:40 +0200 schrieb Laurent Pinchart:
> > On Mon, Feb 07, 2022 at 08:25:45PM +0100, Lucas Stach wrote:
> > > This adds the GPC and HSIO blk-ctrl nodes providing power control for
> > > the high-speed (USB and PCIe) IOs.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 ++++++++++++++++++++---
> > > 1 file changed, 57 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index 6b840c05dd77..dc488a147d0c 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -4,6 +4,7 @@
> > > */
> > >
> > > #include <dt-bindings/clock/imx8mp-clock.h>
> > > +#include <dt-bindings/power/imx8mp-power.h>
> > > #include <dt-bindings/gpio/gpio.h>
> > > #include <dt-bindings/input/input.h>
> > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > @@ -475,6 +476,44 @@ src: reset-controller@30390000 {
> > > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > #reset-cells = <1>;
> > > };
> > > +
> > > + gpc: gpc@303a0000 {
> > > + compatible = "fsl,imx8mp-gpc";
> > > + reg = <0x303a0000 0x10000>;
> >
> > According to the reference manual, the GPC occupies 4kB, not 64kB.
>
> Right, will fix.
>
> > > + interrupt-parent = <&gic>;
> > > + interrupt-controller;
> > > + #interrupt-cells = <3>;
> > > +
> > > + pgc {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> >
> > We're working on support for the MEDIAMIX power domains, which we'll
> > rebase on top of this. In case the HSIO part still needs more work,
> > could you split this patch in two, with one patch that adds the PGC,
> > with an empty pgc nodde, and a second patch that adds the HSIO-related
> > power domains ? The first one could then be merged faster (it would be
> > great if it could get in v5.18).
>
> I don't think the HSIO part is controversial. It seems to work well in
> my testing and DT binding is already reviewed. Not sure if someone is
> going to review it properly, but I hope that we can land it together
> with the reset of this series.
I'll give it a review.
> > > + pgc_pcie_phy: power-domain@1 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_PCIE_PHY>;
> > > + };
> > > +
> > > + pgc_usb1_phy: power-domain@2 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_USB1_PHY>;
> > > + };
> > > +
> > > + pgc_usb2_phy: power-domain@3 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> > > + };
> > > +
> > > + pgc_hsiomix: power-domains@17 {
> > > + #power-domain-cells = <0>;
> > > + reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
> > > + clocks = <&clk IMX8MP_CLK_HSIO_AXI>,
> > > + <&clk IMX8MP_CLK_HSIO_ROOT>;
> > > + assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > + assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > + assigned-clock-rates = <500000000>;
> > > + };
> > > + };
> > > + };
> > > };
> > >
> > > aips2: bus@30400000 {
> > > @@ -908,6 +947,20 @@ ddr-pmu@3d800000 {
> > > interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> > > };
> > >
> > > + hsio_blk_ctrl: blk-ctrl@32f10000 {
> > > + compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> > > + reg = <0x32f10000 0x24>;
> > > + clocks = <&clk IMX8MP_CLK_USB_ROOT>,
> > > + <&clk IMX8MP_CLK_PCIE_ROOT>;
> > > + clock-names = "usb", "pcie";
> > > + power-domains = <&pgc_hsiomix>, <&pgc_hsiomix>,
> > > + <&pgc_usb1_phy>, <&pgc_usb2_phy>,
> > > + <&pgc_hsiomix>, <&pgc_pcie_phy>;
> >
> > Would it be useful to rework the driver to avoid specifying the same
> > parent power domain multiple times in DT ?
>
> I don't think so. That's the pattern we used for all the other blk-
> ctrls, where each virtual power-domain in the blk-ctrl has its own
> handle to a upstream GPC power domain. I don't see why we would want to
> change this now.
It's a small optimization, but probably not really important.
> > > + power-domain-names = "bus", "usb", "usb-phy1",
> > > + "usb-phy2", "pcie", "pcie-phy";
> > > + #power-domain-cells = <1>;
> > > + };
> > > +
> > > usb3_phy0: usb-phy@381f0040 {
> > > compatible = "fsl,imx8mp-usb-phy";
> > > reg = <0x381f0040 0x40>;
> > > @@ -915,6 +968,7 @@ usb3_phy0: usb-phy@381f0040 {
> > > clock-names = "phy";
> > > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY1>;
> > > #phy-cells = <0>;
> > > status = "disabled";
> > > };
> > > @@ -926,6 +980,7 @@ usb3_0: usb@32f10100 {
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "hsio", "suspend";
> > > interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > > @@ -939,9 +994,6 @@ usb_dwc3_0: usb@38100000 {
> > > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "bus_early", "ref", "suspend";
> > > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > - assigned-clock-rates = <500000000>;
> > > interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> > > phys = <&usb3_phy0>, <&usb3_phy0>;
> > > phy-names = "usb2-phy", "usb3-phy";
> > > @@ -957,6 +1009,7 @@ usb3_phy1: usb-phy@382f0040 {
> > > clock-names = "phy";
> > > assigned-clocks = <&clk IMX8MP_CLK_USB_PHY_REF>;
> > > assigned-clock-parents = <&clk IMX8MP_CLK_24M>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB_PHY2>;
> > > #phy-cells = <0>;
> > > };
> > >
> > > @@ -967,6 +1020,7 @@ usb3_1: usb@32f10108 {
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "hsio", "suspend";
> > > interrupts = <GIC_SPI 149 IRQ_TYPE_LEVEL_HIGH>;
> > > + power-domains = <&hsio_blk_ctrl IMX8MP_HSIOBLK_PD_USB>;
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > dma-ranges = <0x40000000 0x40000000 0xc0000000>;
> > > @@ -980,9 +1034,6 @@ usb_dwc3_1: usb@38200000 {
> > > <&clk IMX8MP_CLK_USB_CORE_REF>,
> > > <&clk IMX8MP_CLK_USB_ROOT>;
> > > clock-names = "bus_early", "ref", "suspend";
> > > - assigned-clocks = <&clk IMX8MP_CLK_HSIO_AXI>;
> > > - assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
> > > - assigned-clock-rates = <500000000>;
> > > interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > > phys = <&usb3_phy1>, <&usb3_phy1>;
> > > phy-names = "usb2-phy", "usb3-phy";
> > >
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 8/9] arm64: dts: imx8mp: add GPU power domains
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (5 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 7/9] arm64: dts: imx8mp: add HSIO power-domains Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-21 11:34 ` Laurent Pinchart
2022-02-07 19:25 ` [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes Lucas Stach
2022-02-19 7:32 ` [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Laurent Pinchart
8 siblings, 1 reply; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
Add the power domains for the GPUs, which do not require any interaction with
a blk-ctrl, but are simply two PU domains nested inside a MIX domain.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 27 +++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index dc488a147d0c..9ed57171b9fc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -503,6 +503,33 @@ pgc_usb2_phy: power-domain@3 {
reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
};
+ pgc_gpu2d: power-domain@6 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_GPU2D>;
+ clocks = <&clk IMX8MP_CLK_GPU2D_ROOT>;
+ power-domains = <&pgc_gpumix>;
+ };
+
+ pgc_gpumix: power-domain@7 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_GPUMIX>;
+ clocks = <&clk IMX8MP_CLK_GPU_ROOT>,
+ <&clk IMX8MP_CLK_GPU_AHB>;
+ assigned-clocks = <&clk IMX8MP_CLK_GPU_AXI>,
+ <&clk IMX8MP_CLK_GPU_AHB>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
+ <&clk IMX8MP_SYS_PLL1_800M>;
+ assigned-clock-rates = <800000000>, <400000000>;
+ };
+
+ pgc_gpu3d: power-domain@9 {
+ #power-domain-cells = <0>;
+ reg = <IMX8MP_POWER_DOMAIN_GPU3D>;
+ clocks = <&clk IMX8MP_CLK_GPU3D_ROOT>,
+ <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>;
+ power-domains = <&pgc_gpumix>;
+ };
+
pgc_hsiomix: power-domains@17 {
#power-domain-cells = <0>;
reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH v2 8/9] arm64: dts: imx8mp: add GPU power domains
2022-02-07 19:25 ` [PATCH v2 8/9] arm64: dts: imx8mp: add GPU power domains Lucas Stach
@ 2022-02-21 11:34 ` Laurent Pinchart
0 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 11:34 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:46PM +0100, Lucas Stach wrote:
> Add the power domains for the GPUs, which do not require any interaction with
> a blk-ctrl, but are simply two PU domains nested inside a MIX domain.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 27 +++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index dc488a147d0c..9ed57171b9fc 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -503,6 +503,33 @@ pgc_usb2_phy: power-domain@3 {
> reg = <IMX8MP_POWER_DOMAIN_USB2_PHY>;
> };
>
> + pgc_gpu2d: power-domain@6 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_GPU2D>;
> + clocks = <&clk IMX8MP_CLK_GPU2D_ROOT>;
> + power-domains = <&pgc_gpumix>;
> + };
> +
> + pgc_gpumix: power-domain@7 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_GPUMIX>;
> + clocks = <&clk IMX8MP_CLK_GPU_ROOT>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + assigned-clocks = <&clk IMX8MP_CLK_GPU_AXI>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
> + <&clk IMX8MP_SYS_PLL1_800M>;
> + assigned-clock-rates = <800000000>, <400000000>;
> + };
> +
> + pgc_gpu3d: power-domain@9 {
> + #power-domain-cells = <0>;
> + reg = <IMX8MP_POWER_DOMAIN_GPU3D>;
> + clocks = <&clk IMX8MP_CLK_GPU3D_ROOT>,
> + <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>;
> + power-domains = <&pgc_gpumix>;
> + };
> +
> pgc_hsiomix: power-domains@17 {
> #power-domain-cells = <0>;
> reg = <IMX8MP_POWER_DOMAIN_HSIOMIX>;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (6 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 8/9] arm64: dts: imx8mp: add GPU power domains Lucas Stach
@ 2022-02-07 19:25 ` Lucas Stach
2022-02-08 15:06 ` (EXT) " Alexander Stein
2022-02-21 11:42 ` Laurent Pinchart
2022-02-19 7:32 ` [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Laurent Pinchart
8 siblings, 2 replies; 32+ messages in thread
From: Lucas Stach @ 2022-02-07 19:25 UTC (permalink / raw)
To: Shawn Guo, Rob Herring
Cc: Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
devicetree, linux-arm-kernel, patchwork-lst
Add the DT nodes for both the 3D and 2D GPU cores found on the i.MX8MP.
etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6204
etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341
[drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 31 +++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9ed57171b9fc..c89acb53be4a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -958,6 +958,37 @@ eqos: ethernet@30bf0000 {
};
};
+ gpu3d: gpu@38000000 {
+ compatible = "vivante,gc";
+ reg = <0x38000000 0x8000>;
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MP_CLK_GPU3D_ROOT>,
+ <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>,
+ <&clk IMX8MP_CLK_GPU_ROOT>,
+ <&clk IMX8MP_CLK_GPU_AHB>;
+ clock-names = "core", "shader", "bus", "reg";
+ assigned-clocks = <&clk IMX8MP_CLK_GPU3D_CORE>,
+ <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
+ <&clk IMX8MP_SYS_PLL1_800M>;
+ assigned-clock-rates = <800000000>, <800000000>;
+ power-domains = <&pgc_gpu3d>;
+ };
+
+ gpu2d: gpu@38008000 {
+ compatible = "vivante,gc";
+ reg = <0x38008000 0x8000>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MP_CLK_GPU2D_ROOT>,
+ <&clk IMX8MP_CLK_GPU_ROOT>,
+ <&clk IMX8MP_CLK_GPU_AHB>;
+ clock-names = "core", "bus", "reg";
+ assigned-clocks = <&clk IMX8MP_CLK_GPU2D_CORE>;
+ assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
+ assigned-clock-rates = <800000000>;
+ power-domains = <&pgc_gpu2d>;
+ };
+
gic: interrupt-controller@38800000 {
compatible = "arm,gic-v3";
reg = <0x38800000 0x10000>,
--
2.30.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: (EXT) [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes
2022-02-07 19:25 ` [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes Lucas Stach
@ 2022-02-08 15:06 ` Alexander Stein
2022-02-21 11:42 ` Laurent Pinchart
1 sibling, 0 replies; 32+ messages in thread
From: Alexander Stein @ 2022-02-08 15:06 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, linux-arm-kernel, Pengutronix Kernel Team,
Fabio Estevam, NXP Linux Team, devicetree, linux-arm-kernel,
patchwork-lst
Am Montag, 7. Februar 2022, 20:25:47 CET schrieb Lucas Stach:
> Add the DT nodes for both the 3D and 2D GPU cores found on the i.MX8MP.
>
> etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6204
> etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341
> [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0
Got this on my machine as well:
[ 11.538880] etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6204
[ 11.566889] etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341
[ 11.575804] [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0
I've yet to test some GPU load though.
Best regards,
Alexander
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 31 +++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index
> 9ed57171b9fc..c89acb53be4a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -958,6 +958,37 @@ eqos: ethernet@30bf0000 {
> };
> };
>
> + gpu3d: gpu@38000000 {
> + compatible = "vivante,gc";
> + reg = <0x38000000 0x8000>;
> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MP_CLK_GPU3D_ROOT>,
> + <&clk
IMX8MP_CLK_GPU3D_SHADER_CORE>,
> + <&clk IMX8MP_CLK_GPU_ROOT>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + clock-names = "core", "shader", "bus", "reg";
> + assigned-clocks = <&clk
IMX8MP_CLK_GPU3D_CORE>,
> + <&clk
IMX8MP_CLK_GPU3D_SHADER_CORE>;
> + assigned-clock-parents = <&clk
IMX8MP_SYS_PLL1_800M>,
> + <&clk
IMX8MP_SYS_PLL1_800M>;
> + assigned-clock-rates = <800000000>,
<800000000>;
> + power-domains = <&pgc_gpu3d>;
> + };
> +
> + gpu2d: gpu@38008000 {
> + compatible = "vivante,gc";
> + reg = <0x38008000 0x8000>;
> + interrupts = <GIC_SPI 25
IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MP_CLK_GPU2D_ROOT>,
> + <&clk IMX8MP_CLK_GPU_ROOT>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + clock-names = "core", "bus", "reg";
> + assigned-clocks = <&clk
IMX8MP_CLK_GPU2D_CORE>;
> + assigned-clock-parents = <&clk
IMX8MP_SYS_PLL1_800M>;
> + assigned-clock-rates = <800000000>;
> + power-domains = <&pgc_gpu2d>;
> + };
> +
> gic: interrupt-controller@38800000 {
> compatible = "arm,gic-v3";
> reg = <0x38800000 0x10000>,
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes
2022-02-07 19:25 ` [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes Lucas Stach
2022-02-08 15:06 ` (EXT) " Alexander Stein
@ 2022-02-21 11:42 ` Laurent Pinchart
1 sibling, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-21 11:42 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:47PM +0100, Lucas Stach wrote:
> Add the DT nodes for both the 3D and 2D GPU cores found on the i.MX8MP.
>
> etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6204
> etnaviv-gpu 38008000.gpu: model: GC520, revision: 5341
> [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 31 +++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 9ed57171b9fc..c89acb53be4a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -958,6 +958,37 @@ eqos: ethernet@30bf0000 {
> };
> };
>
> + gpu3d: gpu@38000000 {
> + compatible = "vivante,gc";
> + reg = <0x38000000 0x8000>;
> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MP_CLK_GPU3D_ROOT>,
> + <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>,
> + <&clk IMX8MP_CLK_GPU_ROOT>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + clock-names = "core", "shader", "bus", "reg";
> + assigned-clocks = <&clk IMX8MP_CLK_GPU3D_CORE>,
> + <&clk IMX8MP_CLK_GPU3D_SHADER_CORE>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>,
> + <&clk IMX8MP_SYS_PLL1_800M>;
> + assigned-clock-rates = <800000000>, <800000000>;
> + power-domains = <&pgc_gpu3d>;
> + };
> +
> + gpu2d: gpu@38008000 {
> + compatible = "vivante,gc";
> + reg = <0x38008000 0x8000>;
> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MP_CLK_GPU2D_ROOT>,
> + <&clk IMX8MP_CLK_GPU_ROOT>,
> + <&clk IMX8MP_CLK_GPU_AHB>;
> + clock-names = "core", "bus", "reg";
> + assigned-clocks = <&clk IMX8MP_CLK_GPU2D_CORE>;
> + assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
> + assigned-clock-rates = <800000000>;
> + power-domains = <&pgc_gpu2d>;
> + };
> +
> gic: interrupt-controller@38800000 {
> compatible = "arm,gic-v3";
> reg = <0x38800000 0x10000>,
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection
2022-02-07 19:25 [PATCH v2 1/9] soc: imx: gpcv2: add PGC control register indirection Lucas Stach
` (7 preceding siblings ...)
2022-02-07 19:25 ` [PATCH v2 9/9] arm64: dts: imx8mp: add GPU nodes Lucas Stach
@ 2022-02-19 7:32 ` Laurent Pinchart
8 siblings, 0 replies; 32+ messages in thread
From: Laurent Pinchart @ 2022-02-19 7:32 UTC (permalink / raw)
To: Lucas Stach
Cc: Shawn Guo, Rob Herring, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, devicetree, linux-arm-kernel, patchwork-lst
Hi Lucas,
Thank you for the patch.
On Mon, Feb 07, 2022 at 08:25:39PM +0100, Lucas Stach wrote:
> The PGC control registers in the shared (not per-PGC) region of the
> GPC address space have different offsets on i.MX8MP to make space for
> additional interrupt control registers.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/soc/imx/gpcv2.c | 43 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index 3e59d479d001..01f46b078df3 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -184,9 +184,17 @@
>
> #define GPC_PGC_CTRL_PCR BIT(0)
>
> +struct imx_pgc_regs {
> + u16 map;
> + u16 pup;
> + u16 pdn;
> + u16 hsk;
> +};
> +
> struct imx_pgc_domain {
> struct generic_pm_domain genpd;
> struct regmap *regmap;
> + const struct imx_pgc_regs *regs;
> struct regulator *regulator;
> struct reset_control *reset;
> struct clk_bulk_data *clks;
> @@ -210,6 +218,7 @@ struct imx_pgc_domain_data {
> const struct imx_pgc_domain *domains;
> size_t domains_num;
> const struct regmap_access_table *reg_access_table;
> + const struct imx_pgc_regs *pgc_regs;
> };
>
> static inline struct imx_pgc_domain *
> @@ -249,14 +258,14 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>
> if (domain->bits.pxx) {
> /* request the domain to power up */
> - regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PUP_REQ,
> + regmap_update_bits(domain->regmap, domain->regs->pup,
> domain->bits.pxx, domain->bits.pxx);
> /*
> * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> * for PUP_REQ/PDN_REQ bit to be cleared
> */
> ret = regmap_read_poll_timeout(domain->regmap,
> - GPC_PU_PGC_SW_PUP_REQ, reg_val,
> + domain->regs->pup, reg_val,
> !(reg_val & domain->bits.pxx),
> 0, USEC_PER_MSEC);
> if (ret) {
> @@ -278,11 +287,11 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
>
> /* request the ADB400 to power up */
> if (domain->bits.hskreq) {
> - regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> + regmap_update_bits(domain->regmap, domain->regs->hsk,
> domain->bits.hskreq, domain->bits.hskreq);
>
> /*
> - * ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK, reg_val,
> + * ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk, reg_val,
> * (reg_val & domain->bits.hskack), 0,
> * USEC_PER_MSEC);
> * Technically we need the commented code to wait handshake. But that needs
> @@ -329,10 +338,10 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
>
> /* request the ADB400 to power down */
> if (domain->bits.hskreq) {
> - regmap_clear_bits(domain->regmap, GPC_PU_PWRHSK,
> + regmap_clear_bits(domain->regmap, domain->regs->hsk,
> domain->bits.hskreq);
>
> - ret = regmap_read_poll_timeout(domain->regmap, GPC_PU_PWRHSK,
> + ret = regmap_read_poll_timeout(domain->regmap, domain->regs->hsk,
> reg_val,
> !(reg_val & domain->bits.hskack),
> 0, USEC_PER_MSEC);
> @@ -350,14 +359,14 @@ static int imx_pgc_power_down(struct generic_pm_domain *genpd)
> }
>
> /* request the domain to power down */
> - regmap_update_bits(domain->regmap, GPC_PU_PGC_SW_PDN_REQ,
> + regmap_update_bits(domain->regmap, domain->regs->pdn,
> domain->bits.pxx, domain->bits.pxx);
> /*
> * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> * for PUP_REQ/PDN_REQ bit to be cleared
> */
> ret = regmap_read_poll_timeout(domain->regmap,
> - GPC_PU_PGC_SW_PDN_REQ, reg_val,
> + domain->regs->pdn, reg_val,
> !(reg_val & domain->bits.pxx),
> 0, USEC_PER_MSEC);
> if (ret) {
> @@ -441,10 +450,18 @@ static const struct regmap_access_table imx7_access_table = {
> .n_yes_ranges = ARRAY_SIZE(imx7_yes_ranges),
> };
>
> +static const struct imx_pgc_regs imx7_pgc_regs = {
> + .map = GPC_PGC_CPU_MAPPING,
> + .pup = GPC_PU_PGC_SW_PUP_REQ,
> + .pdn = GPC_PU_PGC_SW_PDN_REQ,
> + .hsk = GPC_PU_PWRHSK,
> +};
> +
> static const struct imx_pgc_domain_data imx7_pgc_domain_data = {
> .domains = imx7_pgc_domains,
> .domains_num = ARRAY_SIZE(imx7_pgc_domains),
> .reg_access_table = &imx7_access_table,
> + .pgc_regs = &imx7_pgc_regs,
> };
>
> static const struct imx_pgc_domain imx8m_pgc_domains[] = {
> @@ -613,6 +630,7 @@ static const struct imx_pgc_domain_data imx8m_pgc_domain_data = {
> .domains = imx8m_pgc_domains,
> .domains_num = ARRAY_SIZE(imx8m_pgc_domains),
> .reg_access_table = &imx8m_access_table,
> + .pgc_regs = &imx7_pgc_regs,
> };
>
> static const struct imx_pgc_domain imx8mm_pgc_domains[] = {
> @@ -803,6 +821,7 @@ static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
> .domains = imx8mm_pgc_domains,
> .domains_num = ARRAY_SIZE(imx8mm_pgc_domains),
> .reg_access_table = &imx8mm_access_table,
> + .pgc_regs = &imx7_pgc_regs,
> };
>
> static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
> @@ -894,6 +913,7 @@ static const struct imx_pgc_domain_data imx8mn_pgc_domain_data = {
> .domains = imx8mn_pgc_domains,
> .domains_num = ARRAY_SIZE(imx8mn_pgc_domains),
> .reg_access_table = &imx8mn_access_table,
> + .pgc_regs = &imx7_pgc_regs,
> };
>
> static int imx_pgc_domain_probe(struct platform_device *pdev)
> @@ -926,7 +946,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
> pm_runtime_enable(domain->dev);
>
> if (domain->bits.map)
> - regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> + regmap_update_bits(domain->regmap, domain->regs->map,
> domain->bits.map, domain->bits.map);
>
> ret = pm_genpd_init(&domain->genpd, NULL, true);
> @@ -952,7 +972,7 @@ static int imx_pgc_domain_probe(struct platform_device *pdev)
> pm_genpd_remove(&domain->genpd);
> out_domain_unmap:
> if (domain->bits.map)
> - regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> + regmap_update_bits(domain->regmap, domain->regs->map,
> domain->bits.map, 0);
> pm_runtime_disable(domain->dev);
>
> @@ -967,7 +987,7 @@ static int imx_pgc_domain_remove(struct platform_device *pdev)
> pm_genpd_remove(&domain->genpd);
>
> if (domain->bits.map)
> - regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> + regmap_update_bits(domain->regmap, domain->regs->map,
> domain->bits.map, 0);
>
> pm_runtime_disable(domain->dev);
> @@ -1098,6 +1118,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
>
> domain = pd_pdev->dev.platform_data;
> domain->regmap = regmap;
> + domain->regs = domain_data->pgc_regs;
> domain->genpd.power_on = imx_pgc_power_up;
> domain->genpd.power_off = imx_pgc_power_down;
>
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 32+ messages in thread