Add pm_runtime support for power saving. In pm runtime suspend
state the registers will be reseted, so add registers save
in pm runtime suspend and restore them in pm runtime resume.
Signed-off-by: Shengjiu Wang <[email protected]>
Reviewed-by: Peng Fan <[email protected]>
---
changes in v4:
- use struct clk_hw_onecell_data clk_data in priv struct
changes in v3:
- remove REGS_NUM, use the ARRAY_SIZE
- merge clk_imx8mp_audiomix_drvdata and clk_hw_onecell_data together.
changes in v2:
- move pm_runtime_enable before the clk register
drivers/clk/imx/clk-imx8mp-audiomix.c | 157 ++++++++++++++++++++++----
1 file changed, 136 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/imx/clk-imx8mp-audiomix.c b/drivers/clk/imx/clk-imx8mp-audiomix.c
index 55ed211a5e0b..574a032309c1 100644
--- a/drivers/clk/imx/clk-imx8mp-audiomix.c
+++ b/drivers/clk/imx/clk-imx8mp-audiomix.c
@@ -7,10 +7,12 @@
#include <linux/clk-provider.h>
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <dt-bindings/clock/imx8mp-clock.h>
@@ -18,6 +20,7 @@
#define CLKEN0 0x000
#define CLKEN1 0x004
+#define EARC 0x200
#define SAI1_MCLK_SEL 0x300
#define SAI2_MCLK_SEL 0x304
#define SAI3_MCLK_SEL 0x308
@@ -26,6 +29,11 @@
#define SAI7_MCLK_SEL 0x314
#define PDM_SEL 0x318
#define SAI_PLL_GNRL_CTL 0x400
+#define SAI_PLL_FDIVL_CTL0 0x404
+#define SAI_PLL_FDIVL_CTL1 0x408
+#define SAI_PLL_SSCG_CTL 0x40C
+#define SAI_PLL_MNIT_CTL 0x410
+#define IPG_LP_CTRL 0x504
#define SAIn_MCLK1_PARENT(n) \
static const struct clk_parent_data \
@@ -182,26 +190,82 @@ static struct clk_imx8mp_audiomix_sel sels[] = {
CLK_SAIn(7)
};
+static const u16 audiomix_regs[] = {
+ CLKEN0,
+ CLKEN1,
+ EARC,
+ SAI1_MCLK_SEL,
+ SAI2_MCLK_SEL,
+ SAI3_MCLK_SEL,
+ SAI5_MCLK_SEL,
+ SAI6_MCLK_SEL,
+ SAI7_MCLK_SEL,
+ PDM_SEL,
+ SAI_PLL_GNRL_CTL,
+ SAI_PLL_FDIVL_CTL0,
+ SAI_PLL_FDIVL_CTL1,
+ SAI_PLL_SSCG_CTL,
+ SAI_PLL_MNIT_CTL,
+ IPG_LP_CTRL,
+};
+
+struct clk_imx8mp_audiomix_priv {
+ void __iomem *base;
+ u32 regs_save[ARRAY_SIZE(audiomix_regs)];
+
+ /* Must be last */
+ struct clk_hw_onecell_data clk_data;
+};
+
+static void clk_imx8mp_audiomix_save_restore(struct device *dev, bool save)
+{
+ struct clk_imx8mp_audiomix_priv *priv = dev_get_drvdata(dev);
+ void __iomem *base = priv->base;
+ int i;
+
+ if (save) {
+ for (i = 0; i < ARRAY_SIZE(audiomix_regs); i++)
+ priv->regs_save[i] = readl(base + audiomix_regs[i]);
+ } else {
+ for (i = 0; i < ARRAY_SIZE(audiomix_regs); i++)
+ writel(priv->regs_save[i], base + audiomix_regs[i]);
+ }
+}
+
static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
{
- struct clk_hw_onecell_data *priv;
+ struct clk_imx8mp_audiomix_priv *priv;
+ struct clk_hw_onecell_data *clk_hw_data;
struct device *dev = &pdev->dev;
void __iomem *base;
struct clk_hw *hw;
- int i;
+ int i, ret;
priv = devm_kzalloc(dev,
- struct_size(priv, hws, IMX8MP_CLK_AUDIOMIX_END),
+ struct_size(priv, clk_data.hws, IMX8MP_CLK_AUDIOMIX_END),
GFP_KERNEL);
if (!priv)
return -ENOMEM;
- priv->num = IMX8MP_CLK_AUDIOMIX_END;
+ clk_hw_data = &priv->clk_data;
+ clk_hw_data->num = IMX8MP_CLK_AUDIOMIX_END;
base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(base))
return PTR_ERR(base);
+ priv->base = base;
+ dev_set_drvdata(dev, priv);
+
+ /*
+ * pm_runtime_enable needs to be called before clk register.
+ * That is to make core->rpm_enabled to be true for clock
+ * usage.
+ */
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
for (i = 0; i < ARRAY_SIZE(sels); i++) {
if (sels[i].num_parents == 1) {
hw = devm_clk_hw_register_gate_parent_data(dev,
@@ -216,10 +280,12 @@ static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
0, NULL, NULL);
}
- if (IS_ERR(hw))
- return PTR_ERR(hw);
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
- priv->hws[sels[i].clkid] = hw;
+ clk_hw_data->hws[sels[i].clkid] = hw;
}
/* SAI PLL */
@@ -228,39 +294,86 @@ static int clk_imx8mp_audiomix_probe(struct platform_device *pdev)
ARRAY_SIZE(clk_imx8mp_audiomix_pll_parents),
CLK_SET_RATE_NO_REPARENT, base + SAI_PLL_GNRL_CTL,
0, 2, 0, NULL, NULL);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_REF_SEL] = hw;
hw = imx_dev_clk_hw_pll14xx(dev, "sai_pll", "sai_pll_ref_sel",
base + 0x400, &imx_1443x_pll);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL] = hw;
hw = devm_clk_hw_register_mux_parent_data_table(dev,
"sai_pll_bypass", clk_imx8mp_audiomix_pll_bypass_sels,
ARRAY_SIZE(clk_imx8mp_audiomix_pll_bypass_sels),
CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
base + SAI_PLL_GNRL_CTL, 16, 1, 0, NULL, NULL);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS] = hw;
hw = devm_clk_hw_register_gate(dev, "sai_pll_out", "sai_pll_bypass",
0, base + SAI_PLL_GNRL_CTL, 13,
0, NULL);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
- priv->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+ clk_hw_data->hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_OUT] = hw;
hw = devm_clk_hw_register_fixed_factor(dev, "sai_pll_out_div2",
"sai_pll_out", 0, 1, 2);
- if (IS_ERR(hw))
- return PTR_ERR(hw);
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_clk_register;
+ }
+
+ ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
+ clk_hw_data);
+ if (ret)
+ goto err_clk_register;
+
+ pm_runtime_put_sync(dev);
+ return 0;
+
+err_clk_register:
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ return ret;
+}
+
+static int clk_imx8mp_audiomix_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static int clk_imx8mp_audiomix_runtime_suspend(struct device *dev)
+{
+ clk_imx8mp_audiomix_save_restore(dev, true);
- return devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
- priv);
+ return 0;
}
+static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
+{
+ clk_imx8mp_audiomix_save_restore(dev, false);
+
+ return 0;
+}
+
+static const struct dev_pm_ops clk_imx8mp_audiomix_pm_ops = {
+ SET_RUNTIME_PM_OPS(clk_imx8mp_audiomix_runtime_suspend,
+ clk_imx8mp_audiomix_runtime_resume, NULL)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+};
+
static const struct of_device_id clk_imx8mp_audiomix_of_match[] = {
{ .compatible = "fsl,imx8mp-audio-blk-ctrl" },
{ /* sentinel */ }
@@ -269,9 +382,11 @@ MODULE_DEVICE_TABLE(of, clk_imx8mp_audiomix_of_match);
static struct platform_driver clk_imx8mp_audiomix_driver = {
.probe = clk_imx8mp_audiomix_probe,
+ .remove = clk_imx8mp_audiomix_remove,
.driver = {
.name = "imx8mp-audio-blk-ctrl",
.of_match_table = clk_imx8mp_audiomix_of_match,
+ .pm = &clk_imx8mp_audiomix_pm_ops,
},
};
--
2.34.1
On 21.03.2024 21:14:02, Shengjiu Wang wrote:
> Add pm_runtime support for power saving. In pm runtime suspend
> state the registers will be reseted, so add registers save
> in pm runtime suspend and restore them in pm runtime resume.
>
> Signed-off-by: Shengjiu Wang <[email protected]>
> Reviewed-by: Peng Fan <[email protected]>
> ---
> changes in v4:
> - use struct clk_hw_onecell_data clk_data in priv struct
Well done! Looks good to me!
For the data structures:
Reviewed-by: Marc Kleine-Budde <[email protected]>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
On Thu, 21 Mar 2024 21:14:02 +0800, Shengjiu Wang wrote:
> Add pm_runtime support for power saving. In pm runtime suspend
> state the registers will be reseted, so add registers save
> in pm runtime suspend and restore them in pm runtime resume.
>
>
Applied, thanks!
[1/1] clk: imx: imx8mp: Add pm_runtime support for power saving
commit: 1496dd413b2e0974a040fa93a2ddc51cc9847fd8
Best regards,
--
Abel Vesa <[email protected]>
On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> Add pm_runtime support for power saving. In pm runtime suspend
> state the registers will be reseted, so add registers save
> in pm runtime suspend and restore them in pm runtime resume.
>
> Signed-off-by: Shengjiu Wang <[email protected]>
> Reviewed-by: Peng Fan <[email protected]>
Is this introducing a regression?
800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
804 13:50:19.747157 <4>[ 16.535431] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
805 13:50:19.758468 <4>[ 16.535442] pc : clk_imx8mp_audiomix_runtime_resume+0x24/0x48
806 13:50:19.759372 <4>[ 16.535456] lr : pm_generic_runtime_resume+0x2c/0x44
807 13:50:19.759587 <4>[ 16.535465] sp : ffff800082b8bb90
808 13:50:19.774512 <4>[ 16.535468] x29: ffff800082b8bb90 x28: 0000000000000000 x27: 0000000000000000
809 13:50:19.775367 <4>[ 16.535482] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
810 13:50:19.790567 <4>[ 16.535495] x23: ffff0000c0f7a4e0 x22: ffff0000c26922f8 x21: 0000000000000000
811 13:50:19.791308 <4>[ 16.535508] x20: ffff0000c2692000 x19: ffff0000c0e30c10 x18: 0000000000000000
812 13:50:19.794834 <4>[ 16.535521] x17: 000000007e4712cb x16: ffff80008296f800 x15: 0000000000000030
813 13:50:19.807341 <4>[ 16.535532] x14: ffff0000c00b8080 x13: 00000000000003f9 x12: 0000000000000000
814 13:50:19.810740 <4>[ 16.535545] x11: 0000000000000000 x10: 0000000000000aa0 x9 : ffff800082b8bb20
815 13:50:19.822528 <4>[ 16.535559] x8 : ffff0000c00b8b00 x7 : 0000000000000000 x6 : ffff0000c00b8000
816 13:50:19.827173 <4>[ 16.535570] x5 : ffff8000836b0000 x4 : ffff0000c2f3a488 x3 : ffff8000813660d0
817 13:50:19.838446 <4>[ 16.535583] x2 : 0000000000000004 x1 : 0000000000000001 x0 : 00000000ff777777
818 13:50:19.839321 <0>[ 16.535597] Kernel panic - not syncing: Asynchronous SError Interrupt
819 13:50:19.839983 Matched prompt #9: Kernel panic - not syncing
820 13:50:19.840155 Setting prompt string to ['end Kernel panic[^\\r]*\\r', '/ #', 'Login timed out', 'Login incorrect']
821 13:50:19.854524 <4>[ 16.535601] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
822 13:50:19.855261 <4>[ 16.535609] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
823 13:50:19.858660 <4>[ 16.535613] Call trace:
824 13:50:19.870455 <4>[ 16.535616] dump_backtrace+0x94/0xec
825 13:50:19.870763 <4>[ 16.535626] show_stack+0x18/0x24
826 13:50:19.871258 <4>[ 16.535635] dump_stack_lvl+0x38/0x90
827 13:50:19.874714 <4>[ 16.535647] dump_stack+0x18/0x24
828 13:50:19.874964 <4>[ 16.535656] panic+0x388/0x3c8
829 13:50:19.886551 <4>[ 16.535667] nmi_panic+0x48/0x94
830 13:50:19.888318 <4>[ 16.535679] arm64_serror_panic+0x6c/0x78
831 13:50:19.888531 <4>[ 16.535688] do_serror+0x3c/0x78
832 13:50:19.892592 <4>[ 16.535693] el1h_64_error_handler+0x30/0x48
833 13:50:19.902540 <4>[ 16.535703] el1h_64_error+0x64/0x68
834 13:50:19.903437 <4>[ 16.535709] clk_imx8mp_audiomix_runtime_resume+0x24/0x48
835 13:50:19.907712 <4>[ 16.535719] __genpd_runtime_resume+0x30/0xa8
836 13:50:19.918505 <4>[ 16.535729] genpd_runtime_resume+0xb4/0x29c
837 13:50:19.918770 <4>[ 16.535741] __rpm_callback+0x48/0x198
838 13:50:19.919372 <4>[ 16.535749] rpm_callback+0x68/0x74
839 13:50:19.922715 <4>[ 16.535754] rpm_resume+0x3cc/0x680
840 13:50:19.934495 <4>[ 16.535762] __pm_runtime_resume+0x4c/0x90
841 13:50:19.934784 <4>[ 16.535769] clk_pm_runtime_get_all+0x58/0x164
842 13:50:19.935344 <4>[ 16.535780] clk_disable_unused+0x2c/0x178
843 13:50:19.938873 <4>[ 16.535793] do_one_initcall+0x6c/0x1b0
844 13:50:19.950539 <4>[ 16.535799] kernel_init_freeable+0x1c8/0x290
845 13:50:19.951360 <4>[ 16.535812] kernel_init+0x20/0x1dc
846 13:50:19.951585 <4>[ 16.535821] ret_from_fork+0x10/0x20
847 13:50:19.954803 <2>[ 16.535831] SMP: stopping secondary CPUs
848 13:50:19.966688 <0>[ 16.535838] Kernel Offset: disabled
849 13:50:19.967221 <0>[ 16.535841] CPU features: 0x0,00000040,00100000,4200421b
850 13:50:19.967360 <0>[ 16.535845] Memory Limit: none
851 13:50:19.985117 <0>[ 16.788060] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
from
https://storage.kernelci.org/next/master/next-20240424/arm64/defconfig/gcc-10/lab-broonie/baseline-imx8mp-verdin-nonwifi-dahlia.html
https://lore.kernel.org/all/[email protected]/
Francesco
On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <[email protected]> wrote:
>
> On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > Add pm_runtime support for power saving. In pm runtime suspend
> > state the registers will be reseted, so add registers save
> > in pm runtime suspend and restore them in pm runtime resume.
> >
> > Signed-off-by: Shengjiu Wang <[email protected]>
> > Reviewed-by: Peng Fan <[email protected]>
>
> Is this introducing a regression?
>
> 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> 804 13:50:19.747157 <4>[ 16.535431] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> 805 13:50:19.758468 <4>[ 16.535442] pc : clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> 806 13:50:19.759372 <4>[ 16.535456] lr : pm_generic_runtime_resume+0x2c/0x44
> 807 13:50:19.759587 <4>[ 16.535465] sp : ffff800082b8bb90
> 808 13:50:19.774512 <4>[ 16.535468] x29: ffff800082b8bb90 x28: 0000000000000000 x27: 0000000000000000
> 809 13:50:19.775367 <4>[ 16.535482] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
> 810 13:50:19.790567 <4>[ 16.535495] x23: ffff0000c0f7a4e0 x22: ffff0000c26922f8 x21: 0000000000000000
> 811 13:50:19.791308 <4>[ 16.535508] x20: ffff0000c2692000 x19: ffff0000c0e30c10 x18: 0000000000000000
> 812 13:50:19.794834 <4>[ 16.535521] x17: 000000007e4712cb x16: ffff80008296f800 x15: 0000000000000030
> 813 13:50:19.807341 <4>[ 16.535532] x14: ffff0000c00b8080 x13: 00000000000003f9 x12: 0000000000000000
> 814 13:50:19.810740 <4>[ 16.535545] x11: 0000000000000000 x10: 0000000000000aa0 x9 : ffff800082b8bb20
> 815 13:50:19.822528 <4>[ 16.535559] x8 : ffff0000c00b8b00 x7 : 0000000000000000 x6 : ffff0000c00b8000
> 816 13:50:19.827173 <4>[ 16.535570] x5 : ffff8000836b0000 x4 : ffff0000c2f3a488 x3 : ffff8000813660d0
> 817 13:50:19.838446 <4>[ 16.535583] x2 : 0000000000000004 x1 : 0000000000000001 x0 : 00000000ff777777
> 818 13:50:19.839321 <0>[ 16.535597] Kernel panic - not syncing: Asynchronous SError Interrupt
> 819 13:50:19.839983 Matched prompt #9: Kernel panic - not syncing
> 820 13:50:19.840155 Setting prompt string to ['end Kernel panic[^\\r]*\\r', '/ #', 'Login timed out', 'Login incorrect']
> 821 13:50:19.854524 <4>[ 16.535601] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> 822 13:50:19.855261 <4>[ 16.535609] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> 823 13:50:19.858660 <4>[ 16.535613] Call trace:
> 824 13:50:19.870455 <4>[ 16.535616] dump_backtrace+0x94/0xec
> 825 13:50:19.870763 <4>[ 16.535626] show_stack+0x18/0x24
> 826 13:50:19.871258 <4>[ 16.535635] dump_stack_lvl+0x38/0x90
> 827 13:50:19.874714 <4>[ 16.535647] dump_stack+0x18/0x24
> 828 13:50:19.874964 <4>[ 16.535656] panic+0x388/0x3c8
> 829 13:50:19.886551 <4>[ 16.535667] nmi_panic+0x48/0x94
> 830 13:50:19.888318 <4>[ 16.535679] arm64_serror_panic+0x6c/0x78
> 831 13:50:19.888531 <4>[ 16.535688] do_serror+0x3c/0x78
> 832 13:50:19.892592 <4>[ 16.535693] el1h_64_error_handler+0x30/0x48
> 833 13:50:19.902540 <4>[ 16.535703] el1h_64_error+0x64/0x68
> 834 13:50:19.903437 <4>[ 16.535709] clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> 835 13:50:19.907712 <4>[ 16.535719] __genpd_runtime_resume+0x30/0xa8
> 836 13:50:19.918505 <4>[ 16.535729] genpd_runtime_resume+0xb4/0x29c
> 837 13:50:19.918770 <4>[ 16.535741] __rpm_callback+0x48/0x198
> 838 13:50:19.919372 <4>[ 16.535749] rpm_callback+0x68/0x74
> 839 13:50:19.922715 <4>[ 16.535754] rpm_resume+0x3cc/0x680
> 840 13:50:19.934495 <4>[ 16.535762] __pm_runtime_resume+0x4c/0x90
> 841 13:50:19.934784 <4>[ 16.535769] clk_pm_runtime_get_all+0x58/0x164
> 842 13:50:19.935344 <4>[ 16.535780] clk_disable_unused+0x2c/0x178
> 843 13:50:19.938873 <4>[ 16.535793] do_one_initcall+0x6c/0x1b0
> 844 13:50:19.950539 <4>[ 16.535799] kernel_init_freeable+0x1c8/0x290
> 845 13:50:19.951360 <4>[ 16.535812] kernel_init+0x20/0x1dc
> 846 13:50:19.951585 <4>[ 16.535821] ret_from_fork+0x10/0x20
> 847 13:50:19.954803 <2>[ 16.535831] SMP: stopping secondary CPUs
> 848 13:50:19.966688 <0>[ 16.535838] Kernel Offset: disabled
> 849 13:50:19.967221 <0>[ 16.535841] CPU features: 0x0,00000040,00100000,4200421b
> 850 13:50:19.967360 <0>[ 16.535845] Memory Limit: none
> 851 13:50:19.985117 <0>[ 16.788060] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
>
> from
>
> https://storage.kernelci.org/next/master/next-20240424/arm64/defconfig/gcc-10/lab-broonie/baseline-imx8mp-verdin-nonwifi-dahlia.html
> https://lore.kernel.org/all/[email protected]/
>
Sorry that I didn't use a clean community kernel for the test.
On my local side I added delay in drivers/pmdomain/imx/gpcv2.c
so there was no such issue.
But according to drivers/pmdomain/imx/gpcv2.c, seems that I need
to add delay in this driver, like this:
static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
{
+ /*
+ * According to the drivers/pmdomain/imx/gpcv2.c
+ * need to wait for reset to propagate
+ */
+ udelay(5);
+
I will submit a patch for it.
Thanks for reporting it
Best regards
Shengjiu Wang
> Francesco
>
On 24-04-25, Shengjiu Wang wrote:
> On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <[email protected]> wrote:
> >
> > On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > > Add pm_runtime support for power saving. In pm runtime suspend
> > > state the registers will be reseted, so add registers save
> > > in pm runtime suspend and restore them in pm runtime resume.
> > >
> > > Signed-off-by: Shengjiu Wang <[email protected]>
> > > Reviewed-by: Peng Fan <[email protected]>
> >
> > Is this introducing a regression?
> >
> > 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> > 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> > 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> > 804 13:50:19.747157 <4>[ 16.535431] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > 805 13:50:19.758468 <4>[ 16.535442] pc : clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> > 806 13:50:19.759372 <4>[ 16.535456] lr : pm_generic_runtime_resume+0x2c/0x44
> > 807 13:50:19.759587 <4>[ 16.535465] sp : ffff800082b8bb90
> > 808 13:50:19.774512 <4>[ 16.535468] x29: ffff800082b8bb90 x28: 0000000000000000 x27: 0000000000000000
> > 809 13:50:19.775367 <4>[ 16.535482] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
> > 810 13:50:19.790567 <4>[ 16.535495] x23: ffff0000c0f7a4e0 x22: ffff0000c26922f8 x21: 0000000000000000
> > 811 13:50:19.791308 <4>[ 16.535508] x20: ffff0000c2692000 x19: ffff0000c0e30c10 x18: 0000000000000000
> > 812 13:50:19.794834 <4>[ 16.535521] x17: 000000007e4712cb x16: ffff80008296f800 x15: 0000000000000030
> > 813 13:50:19.807341 <4>[ 16.535532] x14: ffff0000c00b8080 x13: 00000000000003f9 x12: 0000000000000000
> > 814 13:50:19.810740 <4>[ 16.535545] x11: 0000000000000000 x10: 0000000000000aa0 x9 : ffff800082b8bb20
> > 815 13:50:19.822528 <4>[ 16.535559] x8 : ffff0000c00b8b00 x7 : 0000000000000000 x6 : ffff0000c00b8000
> > 816 13:50:19.827173 <4>[ 16.535570] x5 : ffff8000836b0000 x4 : ffff0000c2f3a488 x3 : ffff8000813660d0
> > 817 13:50:19.838446 <4>[ 16.535583] x2 : 0000000000000004 x1 : 0000000000000001 x0 : 00000000ff777777
> > 818 13:50:19.839321 <0>[ 16.535597] Kernel panic - not syncing: Asynchronous SError Interrupt
> > 819 13:50:19.839983 Matched prompt #9: Kernel panic - not syncing
> > 820 13:50:19.840155 Setting prompt string to ['end Kernel panic[^\\r]*\\r', '/ #', 'Login timed out', 'Login incorrect']
> > 821 13:50:19.854524 <4>[ 16.535601] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > 822 13:50:19.855261 <4>[ 16.535609] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> > 823 13:50:19.858660 <4>[ 16.535613] Call trace:
> > 824 13:50:19.870455 <4>[ 16.535616] dump_backtrace+0x94/0xec
> > 825 13:50:19.870763 <4>[ 16.535626] show_stack+0x18/0x24
> > 826 13:50:19.871258 <4>[ 16.535635] dump_stack_lvl+0x38/0x90
> > 827 13:50:19.874714 <4>[ 16.535647] dump_stack+0x18/0x24
> > 828 13:50:19.874964 <4>[ 16.535656] panic+0x388/0x3c8
> > 829 13:50:19.886551 <4>[ 16.535667] nmi_panic+0x48/0x94
> > 830 13:50:19.888318 <4>[ 16.535679] arm64_serror_panic+0x6c/0x78
> > 831 13:50:19.888531 <4>[ 16.535688] do_serror+0x3c/0x78
> > 832 13:50:19.892592 <4>[ 16.535693] el1h_64_error_handler+0x30/0x48
> > 833 13:50:19.902540 <4>[ 16.535703] el1h_64_error+0x64/0x68
> > 834 13:50:19.903437 <4>[ 16.535709] clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> > 835 13:50:19.907712 <4>[ 16.535719] __genpd_runtime_resume+0x30/0xa8
> > 836 13:50:19.918505 <4>[ 16.535729] genpd_runtime_resume+0xb4/0x29c
> > 837 13:50:19.918770 <4>[ 16.535741] __rpm_callback+0x48/0x198
> > 838 13:50:19.919372 <4>[ 16.535749] rpm_callback+0x68/0x74
> > 839 13:50:19.922715 <4>[ 16.535754] rpm_resume+0x3cc/0x680
> > 840 13:50:19.934495 <4>[ 16.535762] __pm_runtime_resume+0x4c/0x90
> > 841 13:50:19.934784 <4>[ 16.535769] clk_pm_runtime_get_all+0x58/0x164
> > 842 13:50:19.935344 <4>[ 16.535780] clk_disable_unused+0x2c/0x178
> > 843 13:50:19.938873 <4>[ 16.535793] do_one_initcall+0x6c/0x1b0
> > 844 13:50:19.950539 <4>[ 16.535799] kernel_init_freeable+0x1c8/0x290
> > 845 13:50:19.951360 <4>[ 16.535812] kernel_init+0x20/0x1dc
> > 846 13:50:19.951585 <4>[ 16.535821] ret_from_fork+0x10/0x20
> > 847 13:50:19.954803 <2>[ 16.535831] SMP: stopping secondary CPUs
> > 848 13:50:19.966688 <0>[ 16.535838] Kernel Offset: disabled
> > 849 13:50:19.967221 <0>[ 16.535841] CPU features: 0x0,00000040,00100000,4200421b
> > 850 13:50:19.967360 <0>[ 16.535845] Memory Limit: none
> > 851 13:50:19.985117 <0>[ 16.788060] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
> >
> > from
> >
> > https://storage.kernelci.org/next/master/next-20240424/arm64/defconfig/gcc-10/lab-broonie/baseline-imx8mp-verdin-nonwifi-dahlia.html
> > https://lore.kernel.org/all/[email protected]/
> >
>
> Sorry that I didn't use a clean community kernel for the test.
:/ I have asked you if you have tested this feature since I was aware of
bugs regarding PM.
> On my local side I added delay in drivers/pmdomain/imx/gpcv2.c
> so there was no such issue.
>
> But according to drivers/pmdomain/imx/gpcv2.c, seems that I need
> to add delay in this driver, like this:
"Seems" shouldn't be really a "The root cause for this is".
Regards,
Marco
> static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
> {
> + /*
> + * According to the drivers/pmdomain/imx/gpcv2.c
> + * need to wait for reset to propagate
> + */
> + udelay(5);
> +
>
> I will submit a patch for it.
>
> Thanks for reporting it
>
> Best regards
> Shengjiu Wang
>
>
>
> > Francesco
> >
>
>
On Thu, Apr 25, 2024 at 4:32 PM Marco Felsch <[email protected]> wrote:
>
> On 24-04-25, Shengjiu Wang wrote:
> > On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <[email protected]> wrote:
> > >
> > > On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > > > Add pm_runtime support for power saving. In pm runtime suspend
> > > > state the registers will be reseted, so add registers save
> > > > in pm runtime suspend and restore them in pm runtime resume.
> > > >
> > > > Signed-off-by: Shengjiu Wang <[email protected]>
> > > > Reviewed-by: Peng Fan <[email protected]>
> > >
> > > Is this introducing a regression?
> > >
> > > 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> > > 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> > > 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > > 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> > > 804 13:50:19.747157 <4>[ 16.535431] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > 805 13:50:19.758468 <4>[ 16.535442] pc : clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> > > 806 13:50:19.759372 <4>[ 16.535456] lr : pm_generic_runtime_resume+0x2c/0x44
> > > 807 13:50:19.759587 <4>[ 16.535465] sp : ffff800082b8bb90
> > > 808 13:50:19.774512 <4>[ 16.535468] x29: ffff800082b8bb90 x28: 0000000000000000 x27: 0000000000000000
> > > 809 13:50:19.775367 <4>[ 16.535482] x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
> > > 810 13:50:19.790567 <4>[ 16.535495] x23: ffff0000c0f7a4e0 x22: ffff0000c26922f8 x21: 0000000000000000
> > > 811 13:50:19.791308 <4>[ 16.535508] x20: ffff0000c2692000 x19: ffff0000c0e30c10 x18: 0000000000000000
> > > 812 13:50:19.794834 <4>[ 16.535521] x17: 000000007e4712cb x16: ffff80008296f800 x15: 0000000000000030
> > > 813 13:50:19.807341 <4>[ 16.535532] x14: ffff0000c00b8080 x13: 00000000000003f9 x12: 0000000000000000
> > > 814 13:50:19.810740 <4>[ 16.535545] x11: 0000000000000000 x10: 0000000000000aa0 x9 : ffff800082b8bb20
> > > 815 13:50:19.822528 <4>[ 16.535559] x8 : ffff0000c00b8b00 x7 : 0000000000000000 x6 : ffff0000c00b8000
> > > 816 13:50:19.827173 <4>[ 16.535570] x5 : ffff8000836b0000 x4 : ffff0000c2f3a488 x3 : ffff8000813660d0
> > > 817 13:50:19.838446 <4>[ 16.535583] x2 : 0000000000000004 x1 : 0000000000000001 x0 : 00000000ff777777
> > > 818 13:50:19.839321 <0>[ 16.535597] Kernel panic - not syncing: Asynchronous SError Interrupt
> > > 819 13:50:19.839983 Matched prompt #9: Kernel panic - not syncing
> > > 820 13:50:19.840155 Setting prompt string to ['end Kernel panic[^\\r]*\\r', '/ #', 'Login timed out', 'Login incorrect']
> > > 821 13:50:19.854524 <4>[ 16.535601] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > > 822 13:50:19.855261 <4>[ 16.535609] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> > > 823 13:50:19.858660 <4>[ 16.535613] Call trace:
> > > 824 13:50:19.870455 <4>[ 16.535616] dump_backtrace+0x94/0xec
> > > 825 13:50:19.870763 <4>[ 16.535626] show_stack+0x18/0x24
> > > 826 13:50:19.871258 <4>[ 16.535635] dump_stack_lvl+0x38/0x90
> > > 827 13:50:19.874714 <4>[ 16.535647] dump_stack+0x18/0x24
> > > 828 13:50:19.874964 <4>[ 16.535656] panic+0x388/0x3c8
> > > 829 13:50:19.886551 <4>[ 16.535667] nmi_panic+0x48/0x94
> > > 830 13:50:19.888318 <4>[ 16.535679] arm64_serror_panic+0x6c/0x78
> > > 831 13:50:19.888531 <4>[ 16.535688] do_serror+0x3c/0x78
> > > 832 13:50:19.892592 <4>[ 16.535693] el1h_64_error_handler+0x30/0x48
> > > 833 13:50:19.902540 <4>[ 16.535703] el1h_64_error+0x64/0x68
> > > 834 13:50:19.903437 <4>[ 16.535709] clk_imx8mp_audiomix_runtime_resume+0x24/0x48
> > > 835 13:50:19.907712 <4>[ 16.535719] __genpd_runtime_resume+0x30/0xa8
> > > 836 13:50:19.918505 <4>[ 16.535729] genpd_runtime_resume+0xb4/0x29c
> > > 837 13:50:19.918770 <4>[ 16.535741] __rpm_callback+0x48/0x198
> > > 838 13:50:19.919372 <4>[ 16.535749] rpm_callback+0x68/0x74
> > > 839 13:50:19.922715 <4>[ 16.535754] rpm_resume+0x3cc/0x680
> > > 840 13:50:19.934495 <4>[ 16.535762] __pm_runtime_resume+0x4c/0x90
> > > 841 13:50:19.934784 <4>[ 16.535769] clk_pm_runtime_get_all+0x58/0x164
> > > 842 13:50:19.935344 <4>[ 16.535780] clk_disable_unused+0x2c/0x178
> > > 843 13:50:19.938873 <4>[ 16.535793] do_one_initcall+0x6c/0x1b0
> > > 844 13:50:19.950539 <4>[ 16.535799] kernel_init_freeable+0x1c8/0x290
> > > 845 13:50:19.951360 <4>[ 16.535812] kernel_init+0x20/0x1dc
> > > 846 13:50:19.951585 <4>[ 16.535821] ret_from_fork+0x10/0x20
> > > 847 13:50:19.954803 <2>[ 16.535831] SMP: stopping secondary CPUs
> > > 848 13:50:19.966688 <0>[ 16.535838] Kernel Offset: disabled
> > > 849 13:50:19.967221 <0>[ 16.535841] CPU features: 0x0,00000040,00100000,4200421b
> > > 850 13:50:19.967360 <0>[ 16.535845] Memory Limit: none
> > > 851 13:50:19.985117 <0>[ 16.788060] ---[ end Kernel panic - not syncing: Asynchronous SError Interrupt ]---
> > >
> > > from
> > >
> > > https://storage.kernelci.org/next/master/next-20240424/arm64/defconfig/gcc-10/lab-broonie/baseline-imx8mp-verdin-nonwifi-dahlia.html
> > > https://lore.kernel.org/all/[email protected]/
> > >
> >
> > Sorry that I didn't use a clean community kernel for the test.
>
> :/ I have asked you if you have tested this feature since I was aware of
> bugs regarding PM.
But the issue you encountered is the "clock-prepare lock" as I remember.
I think it is a different one.
>
> > On my local side I added delay in drivers/pmdomain/imx/gpcv2.c
> > so there was no such issue.
> >
> > But according to drivers/pmdomain/imx/gpcv2.c, seems that I need
> > to add delay in this driver, like this:
>
> "Seems" shouldn't be really a "The root cause for this is".
I should not use 'Seems':)
below is I find in drivers/pmdomain/imx/gpcv2.c. the commented code:
/* request the ADB400 to power up */
if (domain->bits.hskreq) {
regmap_update_bits(domain->regmap, domain->regs->hsk,
domain->bits.hskreq, domain->bits.hskreq);
/*
* 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
* the BLK-CTL module BUS clk-en bit being set.
*
* There is a separate BLK-CTL module and we will have
such a driver for it,
* that driver will set the BUS clk-en bit and
handshake will be triggered
* automatically there. Just add a delay and suppose
the handshake finish
* after that.
*/
}
Best regards
Shengjiu Wang
>
> Regards,
> Marco
>
> > static int clk_imx8mp_audiomix_runtime_resume(struct device *dev)
> > {
> > + /*
> > + * According to the drivers/pmdomain/imx/gpcv2.c
> > + * need to wait for reset to propagate
> > + */
> > + udelay(5);
> > +
> >
> > I will submit a patch for it.
> >
> > Thanks for reporting it
> >
> > Best regards
> > Shengjiu Wang
> >
> >
> >
> > > Francesco
> > >
> >
> >
On Wed, Apr 24, 2024 at 06:47:25PM +0200, Francesco Dolcini wrote:
> On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > Add pm_runtime support for power saving. In pm runtime suspend
> > state the registers will be reseted, so add registers save
> > in pm runtime suspend and restore them in pm runtime resume.
> >
> > Signed-off-by: Shengjiu Wang <[email protected]>
> > Reviewed-by: Peng Fan <[email protected]>
>
> Is this introducing a regression?
>
> 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
I am now seeing this failure in mainline on both the above board and
i.MX8MP-EVK. There was a fix mentioned in the thread but it's not
landed for -rc1, both boards crash as above. What's the plan for
getting this fixed, should the patch be reverted for now?
On Mon, May 27, 2024 at 8:24 PM Mark Brown <[email protected]> wrote:
>
> On Wed, Apr 24, 2024 at 06:47:25PM +0200, Francesco Dolcini wrote:
> > On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > > Add pm_runtime support for power saving. In pm runtime suspend
> > > state the registers will be reseted, so add registers save
> > > in pm runtime suspend and restore them in pm runtime resume.
> > >
> > > Signed-off-by: Shengjiu Wang <[email protected]>
> > > Reviewed-by: Peng Fan <[email protected]>
> >
> > Is this introducing a regression?
> >
> > 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> > 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> > 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
>
> I am now seeing this failure in mainline on both the above board and
> i.MX8MP-EVK. There was a fix mentioned in the thread but it's not
> landed for -rc1, both boards crash as above. What's the plan for
> getting this fixed, should the patch be reverted for now?
https://lore.kernel.org/all/CAPDyKFp4V8f0iyeRASSEu4YaCSz0m56=8ssBJ9ogSvqG1dzMZA@mail.gmail.com/
fixed is merged, but may not in v6.10-rc1. Should anybody help to cherry-pick?
Best regards
Shengjiu Wang
On Mon, May 27, 2024 at 7:37 AM Shengjiu Wang <[email protected]> wrote:
>
> On Mon, May 27, 2024 at 8:24 PM Mark Brown <[email protected]> wrote:
> >
> > On Wed, Apr 24, 2024 at 06:47:25PM +0200, Francesco Dolcini wrote:
> > > On Thu, Mar 21, 2024 at 09:14:02PM +0800, Shengjiu Wang wrote:
> > > > Add pm_runtime support for power saving. In pm runtime suspend
> > > > state the registers will be reseted, so add registers save
> > > > in pm runtime suspend and restore them in pm runtime resume.
> > > >
> > > > Signed-off-by: Shengjiu Wang <[email protected]>
> > > > Reviewed-by: Peng Fan <[email protected]>
> > >
> > > Is this introducing a regression?
> > >
> > > 800 13:50:19.713052 <6>[ 16.531134] clk: Disabling unused clocks
> > > 801 13:50:19.727524 <2>[ 16.535413] SError Interrupt on CPU2, code 0x00000000bf000002 -- SError
> > > 802 13:50:19.731400 <4>[ 16.535421] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc5-next-20240424 #1
> > > 803 13:50:19.742514 <4>[ 16.535428] Hardware name: Toradex Verdin iMX8M Plus on Dahlia Board (DT)
> >
> > I am now seeing this failure in mainline on both the above board and
> > i.MX8MP-EVK. There was a fix mentioned in the thread but it's not
> > landed for -rc1, both boards crash as above. What's the plan for
> > getting this fixed, should the patch be reverted for now?
>
> https://lore.kernel.org/all/CAPDyKFp4V8f0iyeRASSEu4YaCSz0m56=8ssBJ9ogSvqG1dzMZA@mail.gmail.com/
>
> fixed is merged, but may not in v6.10-rc1. Should anybody help to cherry-pick?
It appears that RC2 has this fix [1].
adam
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/drivers/pmdomain/imx/gpcv2.c?id=v6.10-rc2&id2=v6.10-rc1
>
> Best regards
> Shengjiu Wang
>