diff mbox series

[v4] clk: imx: imx8mp: Add pm_runtime support for power saving

Message ID 1711026842-7268-1-git-send-email-shengjiu.wang@nxp.com (mailing list archive)
State New, archived
Headers show
Series [v4] clk: imx: imx8mp: Add pm_runtime support for power saving | expand

Commit Message

Shengjiu Wang March 21, 2024, 1:14 p.m. UTC 
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 <shengjiu.wang@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
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(-)

Comments

Marc Kleine-Budde March 21, 2024, 2:11 p.m. UTC | #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 <shengjiu.wang@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> ---
> 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 <mkl@pengutronix.de>

Marc
Abel Vesa April 22, 2024, 10:22 a.m. UTC | #2 
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,
Francesco Dolcini April 24, 2024, 4:47 p.m. UTC | #3 
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 <shengjiu.wang@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>

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/66293535.170a0220.21fe.a2e7@mx.google.com/

Francesco
Shengjiu Wang April 25, 2024, 6:36 a.m. UTC | #4 
On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <francesco@dolcini.it> 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 <shengjiu.wang@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
>
> 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/66293535.170a0220.21fe.a2e7@mx.google.com/
>

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
>
Marco Felsch April 25, 2024, 8:32 a.m. UTC | #5 
On 24-04-25, Shengjiu Wang wrote:
> On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <francesco@dolcini.it> 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 <shengjiu.wang@nxp.com>
> > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> >
> > 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/66293535.170a0220.21fe.a2e7@mx.google.com/
> >
> 
> 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
> >
> 
>
Shengjiu Wang April 25, 2024, 8:43 a.m. UTC | #6 
On Thu, Apr 25, 2024 at 4:32 PM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> On 24-04-25, Shengjiu Wang wrote:
> > On Thu, Apr 25, 2024 at 12:47 AM Francesco Dolcini <francesco@dolcini.it> 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 <shengjiu.wang@nxp.com>
> > > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > >
> > > 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/66293535.170a0220.21fe.a2e7@mx.google.com/
> > >
> >
> > 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
> > >
> >
> >
Mark Brown May 27, 2024, 12:24 p.m. UTC | #7 
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 <shengjiu.wang@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> 
> 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?
Shengjiu Wang May 27, 2024, 12:36 p.m. UTC | #8 
On Mon, May 27, 2024 at 8:24 PM Mark Brown <broonie@kernel.org> 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 <shengjiu.wang@nxp.com>
> > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> >
> > 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
Adam Ford June 6, 2024, 6:08 p.m. UTC | #9 
On Mon, May 27, 2024 at 7:37 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>
> On Mon, May 27, 2024 at 8:24 PM Mark Brown <broonie@kernel.org> 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 <shengjiu.wang@nxp.com>
> > > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > >
> > > 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
>
diff mbox series

Patch

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,
 	},
 };