2023-07-31 07:12:51

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update

From: Peng Fan <[email protected]>

V3:
return -EBUSY instead of return 0 in patch 4

V2:
Move drivers/firmware/imx/scu-pd.c to drivers/genpd/imx

This patchset is to upstream NXP downstream scu-pd driver patches.
patch is to relocate scu-pd to genpd
patch 2,3 is to support more PDs
patch 4 is to not power off console when no console suspend
patch 5 is to suppress bind
patch 6 is to make genpd align with HW state
patch 7 is to support LP mode in runtime suspend, OFF mode in system suspend.
patch 8 is to change init level to avoid uneccessary defer probe

V1:
This patchset is to upstream NXP downstream scu-pd driver patches.
patch 1,2 is to support more PDs
patch 3 is to not power off console when no console suspend
patch 4 is to suppress bind
patch 5 is to make genpd align with HW state
patch 6 is to support LP mode in runtime suspend, OFF mode in system suspend.
patch 7 is to change init level to avoid uneccessary defer probe

Dong Aisheng (1):
genpd: imx: scu-pd: change init level to subsys_initcall

Peng Fan (7):
genpd: imx: relocate scu-pd under genpd
genpd: imx: scu-pd: enlarge PD range
genpd: imx: scu-pd: add more PDs
genpd: imx: scu-pd: do not power off console if no_console_suspend
genpd: imx: scu-pd: Suppress bind attrs
genpd: imx: scu-pd: initialize is_off according to HW state
genpd: imx: scu-pd: add multi states support

drivers/firmware/imx/Makefile | 1 -
drivers/genpd/imx/Makefile | 1 +
drivers/{firmware => genpd}/imx/scu-pd.c | 193 +++++++++++++++++++++--
3 files changed, 183 insertions(+), 12 deletions(-)
rename drivers/{firmware => genpd}/imx/scu-pd.c (70%)

--
2.37.1



2023-07-31 07:13:49

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if no_console_suspend

From: Peng Fan <[email protected]>

Do not power off console if no_console_suspend

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index 08583a10ac62..d69da79d3130 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -52,6 +52,7 @@
*/

#include <dt-bindings/firmware/imx/rsrc.h>
+#include <linux/console.h>
#include <linux/firmware/imx/sci.h>
#include <linux/firmware/imx/svc/rm.h>
#include <linux/io.h>
@@ -324,6 +325,10 @@ static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
msg.resource = pd->rsrc;
msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON : IMX_SC_PM_PW_MODE_LP;

+ /* keep uart console power on for no_console_suspend */
+ if (imx_con_rsrc == pd->rsrc && !console_suspend_enabled && !power_on)
+ return -EBUSY;
+
ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
if (ret)
dev_err(&domain->dev, "failed to power %s resource %d ret %d\n",
--
2.37.1


2023-07-31 07:21:00

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 1/8] genpd: imx: relocate scu-pd under genpd

From: Peng Fan <[email protected]>

Move scu-pd driver under genpd directory where the driver
should be.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/firmware/imx/Makefile | 1 -
drivers/genpd/imx/Makefile | 1 +
drivers/{firmware => genpd}/imx/scu-pd.c | 0
3 files changed, 1 insertion(+), 1 deletion(-)
rename drivers/{firmware => genpd}/imx/scu-pd.c (100%)

diff --git a/drivers/firmware/imx/Makefile b/drivers/firmware/imx/Makefile
index b76acbade2a0..8f9f04a513a8 100644
--- a/drivers/firmware/imx/Makefile
+++ b/drivers/firmware/imx/Makefile
@@ -1,4 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_IMX_DSP) += imx-dsp.o
obj-$(CONFIG_IMX_SCU) += imx-scu.o misc.o imx-scu-irq.o rm.o imx-scu-soc.o
-obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
diff --git a/drivers/genpd/imx/Makefile b/drivers/genpd/imx/Makefile
index 5f012717a666..52d2629014a7 100644
--- a/drivers/genpd/imx/Makefile
+++ b/drivers/genpd/imx/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
+obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o
obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8m-blk-ctrl.o
obj-$(CONFIG_IMX8M_BLK_CTRL) += imx8mp-blk-ctrl.o
obj-$(CONFIG_SOC_IMX9) += imx93-pd.o
diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
similarity index 100%
rename from drivers/firmware/imx/scu-pd.c
rename to drivers/genpd/imx/scu-pd.c
--
2.37.1


2023-07-31 07:21:32

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 7/8] genpd: imx: scu-pd: add multi states support

From: Peng Fan <[email protected]>

Add multi states support, this is to support devices could run in LP mode
when runtime suspend, and OFF mode when system suspend.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 48 ++++++++++++++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index 2f693b67ddb4..30da101119eb 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -65,6 +65,12 @@
#include <linux/pm_domain.h>
#include <linux/slab.h>

+enum {
+ PD_STATE_LP,
+ PD_STATE_OFF,
+ PD_STATE_MAX
+};
+
/* SCU Power Mode Protocol definition */
struct imx_sc_msg_req_set_resource_power_mode {
struct imx_sc_rpc_msg hdr;
@@ -368,7 +374,8 @@ static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
hdr->size = 2;

msg.resource = pd->rsrc;
- msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON : IMX_SC_PM_PW_MODE_LP;
+ msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON : pd->pd.state_idx ?
+ IMX_SC_PM_PW_MODE_OFF : IMX_SC_PM_PW_MODE_LP;

/* keep uart console power on for no_console_suspend */
if (imx_con_rsrc == pd->rsrc && !console_suspend_enabled && !power_on)
@@ -412,11 +419,33 @@ static struct generic_pm_domain *imx_scu_pd_xlate(struct of_phandle_args *spec,
return domain;
}

+static bool imx_sc_pd_suspend_ok(struct device *dev)
+{
+ /* Always true */
+ return true;
+}
+
+static bool imx_sc_pd_power_down_ok(struct dev_pm_domain *pd)
+{
+ struct generic_pm_domain *genpd = pd_to_genpd(pd);
+
+ /* For runtime suspend, choose LP mode */
+ genpd->state_idx = 0;
+
+ return true;
+}
+
+struct dev_power_governor imx_sc_pd_qos_governor = {
+ .suspend_ok = imx_sc_pd_suspend_ok,
+ .power_down_ok = imx_sc_pd_power_down_ok,
+};
+
static struct imx_sc_pm_domain *
imx_scu_add_pm_domain(struct device *dev, int idx,
const struct imx_sc_pd_range *pd_ranges)
{
struct imx_sc_pm_domain *sc_pd;
+ struct genpd_power_state *states;
bool is_off;
int mode, ret;

@@ -427,9 +456,22 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
if (!sc_pd)
return ERR_PTR(-ENOMEM);

+ states = devm_kcalloc(dev, PD_STATE_MAX, sizeof(*states), GFP_KERNEL);
+ if (!states) {
+ devm_kfree(dev, sc_pd);
+ return ERR_PTR(-ENOMEM);
+ }
+
sc_pd->rsrc = pd_ranges->rsrc + idx;
sc_pd->pd.power_off = imx_sc_pd_power_off;
sc_pd->pd.power_on = imx_sc_pd_power_on;
+ states[PD_STATE_LP].power_off_latency_ns = 25000;
+ states[PD_STATE_LP].power_on_latency_ns = 25000;
+ states[PD_STATE_OFF].power_off_latency_ns = 2500000;
+ states[PD_STATE_OFF].power_on_latency_ns = 2500000;
+
+ sc_pd->pd.states = states;
+ sc_pd->pd.state_count = PD_STATE_MAX;

if (pd_ranges->postfix)
snprintf(sc_pd->name, sizeof(sc_pd->name),
@@ -455,14 +497,16 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
sc_pd->name, sc_pd->rsrc);

devm_kfree(dev, sc_pd);
+ devm_kfree(dev, states);
return NULL;
}

- ret = pm_genpd_init(&sc_pd->pd, NULL, is_off);
+ ret = pm_genpd_init(&sc_pd->pd, &imx_sc_pd_qos_governor, is_off);
if (ret) {
dev_warn(dev, "failed to init pd %s rsrc id %d",
sc_pd->name, sc_pd->rsrc);
devm_kfree(dev, sc_pd);
+ devm_kfree(dev, states);
return NULL;
}

--
2.37.1


2023-07-31 07:21:59

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 3/8] genpd: imx: scu-pd: add more PDs

From: Peng Fan <[email protected]>

Add more PDs for i.MX8QM and i.MX8DXL, including
dma-ch, esai, gpu1, v2x-mu, seco-mu, hdmi, img and etc.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 65 ++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index 5a28f5af592a..08583a10ac62 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -121,12 +121,16 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "audio-pll1", IMX_SC_R_AUDIO_PLL_1, 1, false, 0 },
{ "audio-clk-0", IMX_SC_R_AUDIO_CLK_0, 1, false, 0 },
{ "audio-clk-1", IMX_SC_R_AUDIO_CLK_1, 1, false, 0 },
+ { "mclk-out-0", IMX_SC_R_MCLK_OUT_0, 1, false, 0 },
+ { "mclk-out-1", IMX_SC_R_MCLK_OUT_1, 1, false, 0 },
{ "dma0-ch", IMX_SC_R_DMA_0_CH0, 32, true, 0 },
{ "dma1-ch", IMX_SC_R_DMA_1_CH0, 16, true, 0 },
{ "dma2-ch", IMX_SC_R_DMA_2_CH0, 32, true, 0 },
+ { "dma3-ch", IMX_SC_R_DMA_3_CH0, 32, true, 0 },
{ "asrc0", IMX_SC_R_ASRC_0, 1, false, 0 },
{ "asrc1", IMX_SC_R_ASRC_1, 1, false, 0 },
{ "esai0", IMX_SC_R_ESAI_0, 1, false, 0 },
+ { "esai1", IMX_SC_R_ESAI_1, 1, false, 0 },
{ "spdif0", IMX_SC_R_SPDIF_0, 1, false, 0 },
{ "spdif1", IMX_SC_R_SPDIF_1, 1, false, 0 },
{ "sai", IMX_SC_R_SAI_0, 3, true, 0 },
@@ -146,8 +150,10 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "lpi2c", IMX_SC_R_I2C_0, 5, true, 0 },
{ "adc", IMX_SC_R_ADC_0, 2, true, 0 },
{ "lcd", IMX_SC_R_LCD_0, 1, true, 0 },
+ { "lcd-pll", IMX_SC_R_ELCDIF_PLL, 1, true, 0 },
{ "lcd0-pwm", IMX_SC_R_LCD_0_PWM_0, 1, true, 0 },
{ "lpuart", IMX_SC_R_UART_0, 5, true, 0 },
+ { "sim", IMX_SC_R_EMVSIM_0, 2, true, 0 },
{ "lpspi", IMX_SC_R_SPI_0, 4, true, 0 },
{ "irqstr_dsp", IMX_SC_R_IRQSTR_DSP, 1, false, 0 },

@@ -163,10 +169,15 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {

/* GPU SS */
{ "gpu0-pid", IMX_SC_R_GPU_0_PID0, 4, true, 0 },
+ { "gpu1-pid", IMX_SC_R_GPU_1_PID0, 4, true, 0 },
+

/* HSIO SS */
+ { "pcie-a", IMX_SC_R_PCIE_A, 1, false, 0 },
+ { "serdes-0", IMX_SC_R_SERDES_0, 1, false, 0 },
{ "pcie-b", IMX_SC_R_PCIE_B, 1, false, 0 },
{ "serdes-1", IMX_SC_R_SERDES_1, 1, false, 0 },
+ { "sata-0", IMX_SC_R_SATA_0, 1, false, 0 },
{ "hsio-gpio", IMX_SC_R_HSIO_GPIO, 1, false, 0 },

/* MIPI SS */
@@ -186,11 +197,20 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "lvds1-pwm", IMX_SC_R_LVDS_1_PWM_0, 1, false, 0 },
{ "lvds1-lpi2c", IMX_SC_R_LVDS_1_I2C_0, 2, true, 0 },

+ { "mipi1", IMX_SC_R_MIPI_1, 1, 0 },
+ { "mipi1-pwm0", IMX_SC_R_MIPI_1_PWM_0, 1, 0 },
+ { "mipi1-i2c", IMX_SC_R_MIPI_1_I2C_0, 2, 1 },
+ { "lvds1", IMX_SC_R_LVDS_1, 1, 0 },
+
/* DC SS */
{ "dc0", IMX_SC_R_DC_0, 1, false, 0 },
{ "dc0-pll", IMX_SC_R_DC_0_PLL_0, 2, true, 0 },
{ "dc0-video", IMX_SC_R_DC_0_VIDEO0, 2, true, 0 },

+ { "dc1", IMX_SC_R_DC_1, 1, false, 0 },
+ { "dc1-pll", IMX_SC_R_DC_1_PLL_0, 2, true, 0 },
+ { "dc1-video", IMX_SC_R_DC_1_VIDEO0, 2, true, 0 },
+
/* CM40 SS */
{ "cm40-i2c", IMX_SC_R_M4_0_I2C, 1, false, 0 },
{ "cm40-intmux", IMX_SC_R_M4_0_INTMUX, 1, false, 0 },
@@ -205,11 +225,56 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "cm41-mu-a1", IMX_SC_R_M4_1_MU_1A, 1, false, 0},
{ "cm41-lpuart", IMX_SC_R_M4_1_UART, 1, false, 0},

+ /* CM41 SS */
+ { "cm41_i2c", IMX_SC_R_M4_1_I2C, 1, false, 0 },
+ { "cm41_intmux", IMX_SC_R_M4_1_INTMUX, 1, false, 0 },
+
+ /* DB SS */
+ { "perf", IMX_SC_R_PERF, 1, false, 0},
+
/* IMAGE SS */
{ "img-jpegdec-mp", IMX_SC_R_MJPEG_DEC_MP, 1, false, 0 },
{ "img-jpegdec-s0", IMX_SC_R_MJPEG_DEC_S0, 4, true, 0 },
{ "img-jpegenc-mp", IMX_SC_R_MJPEG_ENC_MP, 1, false, 0 },
{ "img-jpegenc-s0", IMX_SC_R_MJPEG_ENC_S0, 4, true, 0 },
+
+ /* SECO SS */
+ { "seco_mu", IMX_SC_R_SECO_MU_2, 3, true, 2},
+
+ /* V2X SS */
+ { "v2x_mu", IMX_SC_R_V2X_MU_0, 2, true, 0},
+ { "v2x_mu", IMX_SC_R_V2X_MU_2, 1, true, 2},
+ { "v2x_mu", IMX_SC_R_V2X_MU_3, 2, true, 3},
+ { "img-pdma", IMX_SC_R_ISI_CH0, 8, true, 0 },
+ { "img-csi0", IMX_SC_R_CSI_0, 1, false, 0 },
+ { "img-csi0-i2c0", IMX_SC_R_CSI_0_I2C_0, 1, false, 0 },
+ { "img-csi0-pwm0", IMX_SC_R_CSI_0_PWM_0, 1, false, 0 },
+ { "img-csi1", IMX_SC_R_CSI_1, 1, false, 0 },
+ { "img-csi1-i2c0", IMX_SC_R_CSI_1_I2C_0, 1, false, 0 },
+ { "img-csi1-pwm0", IMX_SC_R_CSI_1_PWM_0, 1, false, 0 },
+ { "img-parallel", IMX_SC_R_PI_0, 1, false, 0 },
+ { "img-parallel-i2c0", IMX_SC_R_PI_0_I2C_0, 1, false, 0 },
+ { "img-parallel-pwm0", IMX_SC_R_PI_0_PWM_0, 2, true, 0 },
+ { "img-parallel-pll", IMX_SC_R_PI_0_PLL, 1, false, 0 },
+
+ /* HDMI TX SS */
+ { "hdmi-tx", IMX_SC_R_HDMI, 1, false, 0},
+ { "hdmi-tx-i2s", IMX_SC_R_HDMI_I2S, 1, false, 0},
+ { "hdmi-tx-i2c0", IMX_SC_R_HDMI_I2C_0, 1, false, 0},
+ { "hdmi-tx-pll0", IMX_SC_R_HDMI_PLL_0, 1, false, 0},
+ { "hdmi-tx-pll1", IMX_SC_R_HDMI_PLL_1, 1, false, 0},
+
+ /* HDMI RX SS */
+ { "hdmi-rx", IMX_SC_R_HDMI_RX, 1, false, 0},
+ { "hdmi-rx-pwm", IMX_SC_R_HDMI_RX_PWM_0, 1, false, 0},
+ { "hdmi-rx-i2c0", IMX_SC_R_HDMI_RX_I2C_0, 1, false, 0},
+ { "hdmi-rx-bypass", IMX_SC_R_HDMI_RX_BYPASS, 1, false, 0},
+
+ /* SECURITY SS */
+ { "sec-jr", IMX_SC_R_CAAM_JR2, 2, true, 2},
+
+ /* BOARD SS */
+ { "board", IMX_SC_R_BOARD_R0, 8, true, 0},
};

static const struct imx_sc_pd_soc imx8qxp_scu_pd = {
--
2.37.1


2023-07-31 07:24:11

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 2/8] genpd: imx: scu-pd: enlarge PD range

From: Peng Fan <[email protected]>

There are 5 LPI2C, 5 LPUART and 32 DMA0 Channel resources per imx_rsrc.h,
and they are in i.MX8QM, so enlarge the PD range for them.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index 84b673427073..5a28f5af592a 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -121,9 +121,9 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
{ "audio-pll1", IMX_SC_R_AUDIO_PLL_1, 1, false, 0 },
{ "audio-clk-0", IMX_SC_R_AUDIO_CLK_0, 1, false, 0 },
{ "audio-clk-1", IMX_SC_R_AUDIO_CLK_1, 1, false, 0 },
- { "dma0-ch", IMX_SC_R_DMA_0_CH0, 16, true, 0 },
+ { "dma0-ch", IMX_SC_R_DMA_0_CH0, 32, true, 0 },
{ "dma1-ch", IMX_SC_R_DMA_1_CH0, 16, true, 0 },
- { "dma2-ch", IMX_SC_R_DMA_2_CH0, 5, true, 0 },
+ { "dma2-ch", IMX_SC_R_DMA_2_CH0, 32, true, 0 },
{ "asrc0", IMX_SC_R_ASRC_0, 1, false, 0 },
{ "asrc1", IMX_SC_R_ASRC_1, 1, false, 0 },
{ "esai0", IMX_SC_R_ESAI_0, 1, false, 0 },
@@ -143,11 +143,11 @@ static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
/* DMA SS */
{ "can", IMX_SC_R_CAN_0, 3, true, 0 },
{ "ftm", IMX_SC_R_FTM_0, 2, true, 0 },
- { "lpi2c", IMX_SC_R_I2C_0, 4, true, 0 },
+ { "lpi2c", IMX_SC_R_I2C_0, 5, true, 0 },
{ "adc", IMX_SC_R_ADC_0, 2, true, 0 },
{ "lcd", IMX_SC_R_LCD_0, 1, true, 0 },
{ "lcd0-pwm", IMX_SC_R_LCD_0_PWM_0, 1, true, 0 },
- { "lpuart", IMX_SC_R_UART_0, 4, true, 0 },
+ { "lpuart", IMX_SC_R_UART_0, 5, true, 0 },
{ "lpspi", IMX_SC_R_SPI_0, 4, true, 0 },
{ "irqstr_dsp", IMX_SC_R_IRQSTR_DSP, 1, false, 0 },

--
2.37.1


2023-07-31 08:02:20

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 6/8] genpd: imx: scu-pd: initialize is_off according to HW state

From: Peng Fan <[email protected]>

The current code default set is_off to true except console resource,
this implies bootloader should power off all the resources it uses.

But this is not always true, let's check the HW state and set is_off.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 59 +++++++++++++++++++++++++++++++++++---
1 file changed, 55 insertions(+), 4 deletions(-)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index fa840ebe38c5..2f693b67ddb4 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -72,6 +72,22 @@ struct imx_sc_msg_req_set_resource_power_mode {
u8 mode;
} __packed __aligned(4);

+struct req_get_resource_mode {
+ u16 resource;
+};
+
+struct resp_get_resource_mode {
+ u8 mode;
+};
+
+struct imx_sc_msg_req_get_resource_power_mode {
+ struct imx_sc_rpc_msg hdr;
+ union {
+ struct req_get_resource_mode req;
+ struct resp_get_resource_mode resp;
+ } data;
+} __packed __aligned(4);
+
#define IMX_SCU_PD_NAME_SIZE 20
struct imx_sc_pm_domain {
struct generic_pm_domain pd;
@@ -96,6 +112,14 @@ struct imx_sc_pd_soc {

static int imx_con_rsrc;

+/* Align with the IMX_SC_PM_PW_MODE_[OFF,STBY,LP,ON] macros */
+static const char * const imx_sc_pm_mode[] = {
+ "IMX_SC_PM_PW_MODE_OFF",
+ "IMX_SC_PM_PW_MODE_STBY",
+ "IMX_SC_PM_PW_MODE_LP",
+ "IMX_SC_PM_PW_MODE_ON"
+};
+
static const struct imx_sc_pd_range imx8qxp_scu_pd_ranges[] = {
/* LSIO SS */
{ "pwm", IMX_SC_R_PWM_0, 8, true, 0 },
@@ -308,6 +332,27 @@ static void imx_sc_pd_get_console_rsrc(void)
imx_con_rsrc = specs.args[0];
}

+static int imx_sc_get_pd_power(struct device *dev, u32 rsrc)
+{
+ struct imx_sc_msg_req_get_resource_power_mode msg;
+ struct imx_sc_rpc_msg *hdr = &msg.hdr;
+ int ret;
+
+ hdr->ver = IMX_SC_RPC_VERSION;
+ hdr->svc = IMX_SC_RPC_SVC_PM;
+ hdr->func = IMX_SC_PM_FUNC_GET_RESOURCE_POWER_MODE;
+ hdr->size = 2;
+
+ msg.data.req.resource = rsrc;
+
+ ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
+ if (ret)
+ dev_err(dev, "failed to get power resource %d mode, ret %d\n",
+ rsrc, ret);
+
+ return msg.data.resp.mode;
+}
+
static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
{
struct imx_sc_msg_req_set_resource_power_mode msg;
@@ -372,8 +417,8 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
const struct imx_sc_pd_range *pd_ranges)
{
struct imx_sc_pm_domain *sc_pd;
- bool is_off = true;
- int ret;
+ bool is_off;
+ int mode, ret;

if (!imx_sc_rm_is_resource_owned(pm_ipc_handle, pd_ranges->rsrc + idx))
return NULL;
@@ -394,10 +439,16 @@ imx_scu_add_pm_domain(struct device *dev, int idx,
"%s", pd_ranges->name);

sc_pd->pd.name = sc_pd->name;
- if (imx_con_rsrc == sc_pd->rsrc) {
+ if (imx_con_rsrc == sc_pd->rsrc)
sc_pd->pd.flags = GENPD_FLAG_RPM_ALWAYS_ON;
+
+ mode = imx_sc_get_pd_power(dev, pd_ranges->rsrc + idx);
+ if (mode == IMX_SC_PM_PW_MODE_ON)
is_off = false;
- }
+ else
+ is_off = true;
+
+ dev_dbg(dev, "%s : %s\n", sc_pd->name, imx_sc_pm_mode[mode]);

if (sc_pd->rsrc >= IMX_SC_R_LAST) {
dev_warn(dev, "invalid pd %s rsrc id %d found",
--
2.37.1


2023-07-31 08:15:42

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V3 5/8] genpd: imx: scu-pd: Suppress bind attrs

From: Peng Fan <[email protected]>

This driver is registered as platform driver, but removing and binding
again would cause system not workable. So suppress bind attrs.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/genpd/imx/scu-pd.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
index d69da79d3130..fa840ebe38c5 100644
--- a/drivers/genpd/imx/scu-pd.c
+++ b/drivers/genpd/imx/scu-pd.c
@@ -488,6 +488,7 @@ static struct platform_driver imx_sc_pd_driver = {
.driver = {
.name = "imx-scu-pd",
.of_match_table = imx_sc_pd_match,
+ .suppress_bind_attrs = true,
},
.probe = imx_sc_pd_probe,
};
--
2.37.1


2023-08-08 18:51:50

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update

On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> V3:
> return -EBUSY instead of return 0 in patch 4
>
> V2:
> Move drivers/firmware/imx/scu-pd.c to drivers/genpd/imx
>
> This patchset is to upstream NXP downstream scu-pd driver patches.
> patch is to relocate scu-pd to genpd
> patch 2,3 is to support more PDs
> patch 4 is to not power off console when no console suspend
> patch 5 is to suppress bind
> patch 6 is to make genpd align with HW state
> patch 7 is to support LP mode in runtime suspend, OFF mode in system suspend.
> patch 8 is to change init level to avoid uneccessary defer probe
>
> V1:
> This patchset is to upstream NXP downstream scu-pd driver patches.
> patch 1,2 is to support more PDs
> patch 3 is to not power off console when no console suspend
> patch 4 is to suppress bind
> patch 5 is to make genpd align with HW state
> patch 6 is to support LP mode in runtime suspend, OFF mode in system suspend.
> patch 7 is to change init level to avoid uneccessary defer probe
>
> Dong Aisheng (1):
> genpd: imx: scu-pd: change init level to subsys_initcall
>
> Peng Fan (7):
> genpd: imx: relocate scu-pd under genpd
> genpd: imx: scu-pd: enlarge PD range
> genpd: imx: scu-pd: add more PDs
> genpd: imx: scu-pd: do not power off console if no_console_suspend
> genpd: imx: scu-pd: Suppress bind attrs
> genpd: imx: scu-pd: initialize is_off according to HW state
> genpd: imx: scu-pd: add multi states support
>
> drivers/firmware/imx/Makefile | 1 -
> drivers/genpd/imx/Makefile | 1 +
> drivers/{firmware => genpd}/imx/scu-pd.c | 193 +++++++++++++++++++++--
> 3 files changed, 183 insertions(+), 12 deletions(-)
> rename drivers/{firmware => genpd}/imx/scu-pd.c (70%)
>

Moving this to the new genpd subsystem makes sense to me.

Even if we can't get the whole series ready for v6.6, we can certainly
pick patch1. Either we can funnel this via my new genpd tree [1] or if
Shawn picks it up. If the latter, Shawn needs to merge my immutable
branch [2] before applying. I am fine either way.

Kind regards
Uffe

[1]
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git next

[2]
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git genpd_create_dir

2023-08-09 03:28:32

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update

> Subject: Re: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update
>
> On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <[email protected]>
> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > V3:
> > return -EBUSY instead of return 0 in patch 4
> >
> > V2:
> > Move drivers/firmware/imx/scu-pd.c to drivers/genpd/imx
> >
> > This patchset is to upstream NXP downstream scu-pd driver patches.
> > patch is to relocate scu-pd to genpd
> > patch 2,3 is to support more PDs
> > patch 4 is to not power off console when no console suspend patch 5 is
> > to suppress bind patch 6 is to make genpd align with HW state patch 7
> > is to support LP mode in runtime suspend, OFF mode in system suspend.
> > patch 8 is to change init level to avoid uneccessary defer probe
> >
> > V1:
> > This patchset is to upstream NXP downstream scu-pd driver patches.
> > patch 1,2 is to support more PDs
> > patch 3 is to not power off console when no console suspend patch 4 is
> > to suppress bind patch 5 is to make genpd align with HW state patch 6
> > is to support LP mode in runtime suspend, OFF mode in system suspend.
> > patch 7 is to change init level to avoid uneccessary defer probe
> >
> > Dong Aisheng (1):
> > genpd: imx: scu-pd: change init level to subsys_initcall
> >
> > Peng Fan (7):
> > genpd: imx: relocate scu-pd under genpd
> > genpd: imx: scu-pd: enlarge PD range
> > genpd: imx: scu-pd: add more PDs
> > genpd: imx: scu-pd: do not power off console if no_console_suspend
> > genpd: imx: scu-pd: Suppress bind attrs
> > genpd: imx: scu-pd: initialize is_off according to HW state
> > genpd: imx: scu-pd: add multi states support
> >
> > drivers/firmware/imx/Makefile | 1 -
> > drivers/genpd/imx/Makefile | 1 +
> > drivers/{firmware => genpd}/imx/scu-pd.c | 193
> > +++++++++++++++++++++--
> > 3 files changed, 183 insertions(+), 12 deletions(-) rename
> > drivers/{firmware => genpd}/imx/scu-pd.c (70%)
> >
>
> Moving this to the new genpd subsystem makes sense to me.
>
> Even if we can't get the whole series ready for v6.6, we can certainly pick
> patch1. Either we can funnel this via my new genpd tree [1] or if Shawn
> picks it up. If the latter, Shawn needs to merge my immutable branch [2]
> before applying. I am fine either way.

There is no rush to catch v6.6 for this patchset. It could go via your genpd
tree for v6.7 from my view. Anyway, up to you and Shawn to decide.

Thanks,
Peng.

>
> Kind regards
> Uffe
>
> [1]
> git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git next
>
> [2]
> git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm.git
> genpd_create_dir

2023-08-10 14:52:30

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if no_console_suspend

On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> Do not power off console if no_console_suspend

Perhaps extend this a bit to let the reader understand this is about
leaving the serial device's corresponding PM domain on.

>
> Signed-off-by: Peng Fan <[email protected]>

A comment below - that doesn't need to stop this from being applied though.

> ---
> drivers/genpd/imx/scu-pd.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
> index 08583a10ac62..d69da79d3130 100644
> --- a/drivers/genpd/imx/scu-pd.c
> +++ b/drivers/genpd/imx/scu-pd.c
> @@ -52,6 +52,7 @@
> */
>
> #include <dt-bindings/firmware/imx/rsrc.h>
> +#include <linux/console.h>
> #include <linux/firmware/imx/sci.h>
> #include <linux/firmware/imx/svc/rm.h>
> #include <linux/io.h>
> @@ -324,6 +325,10 @@ static int imx_sc_pd_power(struct generic_pm_domain *domain, bool power_on)
> msg.resource = pd->rsrc;
> msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON : IMX_SC_PM_PW_MODE_LP;
>
> + /* keep uart console power on for no_console_suspend */
> + if (imx_con_rsrc == pd->rsrc && !console_suspend_enabled && !power_on)

As I indicated above, I don't mind this, but I also think this is a
rather generic problem that you are trying to solve here.

In principle, I think it should be the serial driver's responsibility
to check the console_suspend_enabled flag. Based upon that, it should
inform upper layers (genpd) that its device may need to stay powered
on during system suspend. Quite similar to how we deal with system
wakeups. To make this work we could do the following instead of
$subject patch.

1. The serial driver should call device_set_wakeup_path() (the name of
that function is a bit confusing in this regard, but let's discuss
that separately) in its ->suspend() callback.
2. Set the GENPD_FLAG_ACTIVE_WAKEUP (again the name is a bit confusing
in this regard) for the corresponding genpd provider.

In this way, genpd will keep the PM domain powered on during system suspend.

> + return -EBUSY;
> +
> ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
> if (ret)
> dev_err(&domain->dev, "failed to power %s resource %d ret %d\n",
> --
> 2.37.1
>

Kind regards
Uffe

2023-08-11 15:00:02

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update

On Wed, Aug 09, 2023 at 01:32:28AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH V3 0/8] genpd: imx: relocate scu-pd and misc update
> >
> > On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <[email protected]>
> > wrote:
> > >
> > > From: Peng Fan <[email protected]>
> > >
> > > V3:
> > > return -EBUSY instead of return 0 in patch 4
> > >
> > > V2:
> > > Move drivers/firmware/imx/scu-pd.c to drivers/genpd/imx
> > >
> > > This patchset is to upstream NXP downstream scu-pd driver patches.
> > > patch is to relocate scu-pd to genpd
> > > patch 2,3 is to support more PDs
> > > patch 4 is to not power off console when no console suspend patch 5 is
> > > to suppress bind patch 6 is to make genpd align with HW state patch 7
> > > is to support LP mode in runtime suspend, OFF mode in system suspend.
> > > patch 8 is to change init level to avoid uneccessary defer probe
> > >
> > > V1:
> > > This patchset is to upstream NXP downstream scu-pd driver patches.
> > > patch 1,2 is to support more PDs
> > > patch 3 is to not power off console when no console suspend patch 4 is
> > > to suppress bind patch 5 is to make genpd align with HW state patch 6
> > > is to support LP mode in runtime suspend, OFF mode in system suspend.
> > > patch 7 is to change init level to avoid uneccessary defer probe
> > >
> > > Dong Aisheng (1):
> > > genpd: imx: scu-pd: change init level to subsys_initcall
> > >
> > > Peng Fan (7):
> > > genpd: imx: relocate scu-pd under genpd
> > > genpd: imx: scu-pd: enlarge PD range
> > > genpd: imx: scu-pd: add more PDs
> > > genpd: imx: scu-pd: do not power off console if no_console_suspend
> > > genpd: imx: scu-pd: Suppress bind attrs
> > > genpd: imx: scu-pd: initialize is_off according to HW state
> > > genpd: imx: scu-pd: add multi states support
> > >
> > > drivers/firmware/imx/Makefile | 1 -
> > > drivers/genpd/imx/Makefile | 1 +
> > > drivers/{firmware => genpd}/imx/scu-pd.c | 193
> > > +++++++++++++++++++++--
> > > 3 files changed, 183 insertions(+), 12 deletions(-) rename
> > > drivers/{firmware => genpd}/imx/scu-pd.c (70%)
> > >
> >
> > Moving this to the new genpd subsystem makes sense to me.
> >
> > Even if we can't get the whole series ready for v6.6, we can certainly pick
> > patch1. Either we can funnel this via my new genpd tree [1] or if Shawn
> > picks it up. If the latter, Shawn needs to merge my immutable branch [2]
> > before applying. I am fine either way.
>
> There is no rush to catch v6.6 for this patchset. It could go via your genpd
> tree for v6.7 from my view. Anyway, up to you and Shawn to decide.

I prefer to have this go via genpd tree.

Shawn

2023-08-14 12:30:59

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if no_console_suspend

> Subject: Re: [PATCH V3 4/8] genpd: imx: scu-pd: do not power off console if
> no_console_suspend
>
> On Mon, 31 Jul 2023 at 08:43, Peng Fan (OSS) <[email protected]>
> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > Do not power off console if no_console_suspend
>
> Perhaps extend this a bit to let the reader understand this is about leaving
> the serial device's corresponding PM domain on.

ok

>
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> A comment below - that doesn't need to stop this from being applied
> though.
>
> > ---
> > drivers/genpd/imx/scu-pd.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/genpd/imx/scu-pd.c b/drivers/genpd/imx/scu-pd.c
> > index 08583a10ac62..d69da79d3130 100644
> > --- a/drivers/genpd/imx/scu-pd.c
> > +++ b/drivers/genpd/imx/scu-pd.c
> > @@ -52,6 +52,7 @@
> > */
> >
> > #include <dt-bindings/firmware/imx/rsrc.h>
> > +#include <linux/console.h>
> > #include <linux/firmware/imx/sci.h>
> > #include <linux/firmware/imx/svc/rm.h> #include <linux/io.h> @@
> > -324,6 +325,10 @@ static int imx_sc_pd_power(struct
> generic_pm_domain *domain, bool power_on)
> > msg.resource = pd->rsrc;
> > msg.mode = power_on ? IMX_SC_PM_PW_MODE_ON :
> > IMX_SC_PM_PW_MODE_LP;
> >
> > + /* keep uart console power on for no_console_suspend */
> > + if (imx_con_rsrc == pd->rsrc && !console_suspend_enabled &&
> > + !power_on)
>
> As I indicated above, I don't mind this, but I also think this is a rather generic
> problem that you are trying to solve here.
>
> In principle, I think it should be the serial driver's responsibility to check the
> console_suspend_enabled flag. Based upon that, it should inform upper
> layers (genpd) that its device may need to stay powered on during system
> suspend. Quite similar to how we deal with system wakeups. To make this
> work we could do the following instead of $subject patch.
>
> 1. The serial driver should call device_set_wakeup_path() (the name of that
> function is a bit confusing in this regard, but let's discuss that separately) in
> its ->suspend() callback.
> 2. Set the GENPD_FLAG_ACTIVE_WAKEUP (again the name is a bit confusing
> in this regard) for the corresponding genpd provider.

Thanks for your suggestion, I will try it. For the current v3 4/8, I will keep it in
V4. For your suggested method, I will post in a separate patch after I finish
and verified.

Thanks,
Peng.

>
> In this way, genpd will keep the PM domain powered on during system
> suspend.
>
> > + return -EBUSY;
> > +
> > ret = imx_scu_call_rpc(pm_ipc_handle, &msg, true);
> > if (ret)
> > dev_err(&domain->dev, "failed to power %s resource %d
> > ret %d\n",
> > --
> > 2.37.1
> >
>
> Kind regards
> Uffe