| Message ID | 20190822190425.23001-13-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Intel: Clenaup SST initialization | expand |
On 8/22/19 2:04 PM, Cezary Rojewski wrote: > Skylake is sst_dsp descendant. Rather than bypassing framework's flow, > embrace it. sst_dsp_free invokes sst specific handler internally so > nothing is missed. for clarity you should also add the free_irq is also handled internally in sst_dsp_free(), otherwise this looks like a mistake to the reviewer that's not even half-way through the series... > > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> > --- > sound/soc/intel/skylake/cnl-sst-dsp.c | 1 - > sound/soc/intel/skylake/skl-sst-dsp.c | 1 - > sound/soc/intel/skylake/skl-sst.c | 2 +- > 3 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/sound/soc/intel/skylake/cnl-sst-dsp.c b/sound/soc/intel/skylake/cnl-sst-dsp.c > index 189c1c7086e3..48b465939ef8 100644 > --- a/sound/soc/intel/skylake/cnl-sst-dsp.c > +++ b/sound/soc/intel/skylake/cnl-sst-dsp.c > @@ -215,7 +215,6 @@ void cnl_dsp_free(struct sst_dsp *dsp) > sst_ipc_fini(&skl->ipc); > cnl_ipc_int_disable(dsp); > > - free_irq(dsp->irq, dsp); > cnl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); > } > EXPORT_SYMBOL_GPL(cnl_dsp_free); > diff --git a/sound/soc/intel/skylake/skl-sst-dsp.c b/sound/soc/intel/skylake/skl-sst-dsp.c > index 348e69226e46..1c4ecbcd7db7 100644 > --- a/sound/soc/intel/skylake/skl-sst-dsp.c > +++ b/sound/soc/intel/skylake/skl-sst-dsp.c > @@ -468,7 +468,6 @@ void skl_dsp_free(struct sst_dsp *dsp) > sst_ipc_fini(&skl->ipc); > skl_ipc_int_disable(dsp); > > - free_irq(dsp->irq, dsp); > skl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); > } > EXPORT_SYMBOL_GPL(skl_dsp_free); > diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c > index 72ee3d8a1d7d..e55523826346 100644 > --- a/sound/soc/intel/skylake/skl-sst.c > +++ b/sound/soc/intel/skylake/skl-sst.c > @@ -622,7 +622,7 @@ void skl_sst_dsp_cleanup(struct skl_dev *skl) > skl_clear_module_table(dsp); > > skl_freeup_uuid_list(skl); > - dsp->ops->free(dsp); > + sst_dsp_free(dsp); > > if (skl->boot_complete && dsp->cl_dev.bufsize) { > dsp->cl_dev.ops.cl_cleanup_controller(dsp); >
On 2019-08-23 21:07, Pierre-Louis Bossart wrote: > > > On 8/22/19 2:04 PM, Cezary Rojewski wrote: >> Skylake is sst_dsp descendant. Rather than bypassing framework's flow, >> embrace it. sst_dsp_free invokes sst specific handler internally so >> nothing is missed. > > for clarity you should also add the free_irq is also handled internally > in sst_dsp_free(), otherwise this looks like a mistake to the reviewer > that's not even half-way through the series... > Noted, thanks. Indeed this is done to reuse sst_dsp_free while preventing double free_irq from occurring. >> >> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> >> --- >> sound/soc/intel/skylake/cnl-sst-dsp.c | 1 - >> sound/soc/intel/skylake/skl-sst-dsp.c | 1 - >> sound/soc/intel/skylake/skl-sst.c | 2 +- >> 3 files changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/sound/soc/intel/skylake/cnl-sst-dsp.c >> b/sound/soc/intel/skylake/cnl-sst-dsp.c >> index 189c1c7086e3..48b465939ef8 100644 >> --- a/sound/soc/intel/skylake/cnl-sst-dsp.c >> +++ b/sound/soc/intel/skylake/cnl-sst-dsp.c >> @@ -215,7 +215,6 @@ void cnl_dsp_free(struct sst_dsp *dsp) >> sst_ipc_fini(&skl->ipc); >> cnl_ipc_int_disable(dsp); >> - free_irq(dsp->irq, dsp); >> cnl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); >> } >> EXPORT_SYMBOL_GPL(cnl_dsp_free); >> diff --git a/sound/soc/intel/skylake/skl-sst-dsp.c >> b/sound/soc/intel/skylake/skl-sst-dsp.c >> index 348e69226e46..1c4ecbcd7db7 100644 >> --- a/sound/soc/intel/skylake/skl-sst-dsp.c >> +++ b/sound/soc/intel/skylake/skl-sst-dsp.c >> @@ -468,7 +468,6 @@ void skl_dsp_free(struct sst_dsp *dsp) >> sst_ipc_fini(&skl->ipc); >> skl_ipc_int_disable(dsp); >> - free_irq(dsp->irq, dsp); >> skl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); >> } >> EXPORT_SYMBOL_GPL(skl_dsp_free); >> diff --git a/sound/soc/intel/skylake/skl-sst.c >> b/sound/soc/intel/skylake/skl-sst.c >> index 72ee3d8a1d7d..e55523826346 100644 >> --- a/sound/soc/intel/skylake/skl-sst.c >> +++ b/sound/soc/intel/skylake/skl-sst.c >> @@ -622,7 +622,7 @@ void skl_sst_dsp_cleanup(struct skl_dev *skl) >> skl_clear_module_table(dsp); >> skl_freeup_uuid_list(skl); >> - dsp->ops->free(dsp); >> + sst_dsp_free(dsp); >> if (skl->boot_complete && dsp->cl_dev.bufsize) { >> dsp->cl_dev.ops.cl_cleanup_controller(dsp); >>
diff --git a/sound/soc/intel/skylake/cnl-sst-dsp.c b/sound/soc/intel/skylake/cnl-sst-dsp.c index 189c1c7086e3..48b465939ef8 100644 --- a/sound/soc/intel/skylake/cnl-sst-dsp.c +++ b/sound/soc/intel/skylake/cnl-sst-dsp.c @@ -215,7 +215,6 @@ void cnl_dsp_free(struct sst_dsp *dsp) sst_ipc_fini(&skl->ipc); cnl_ipc_int_disable(dsp); - free_irq(dsp->irq, dsp); cnl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); } EXPORT_SYMBOL_GPL(cnl_dsp_free); diff --git a/sound/soc/intel/skylake/skl-sst-dsp.c b/sound/soc/intel/skylake/skl-sst-dsp.c index 348e69226e46..1c4ecbcd7db7 100644 --- a/sound/soc/intel/skylake/skl-sst-dsp.c +++ b/sound/soc/intel/skylake/skl-sst-dsp.c @@ -468,7 +468,6 @@ void skl_dsp_free(struct sst_dsp *dsp) sst_ipc_fini(&skl->ipc); skl_ipc_int_disable(dsp); - free_irq(dsp->irq, dsp); skl_dsp_disable_core(dsp, SKL_DSP_CORE0_MASK); } EXPORT_SYMBOL_GPL(skl_dsp_free); diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c index 72ee3d8a1d7d..e55523826346 100644 --- a/sound/soc/intel/skylake/skl-sst.c +++ b/sound/soc/intel/skylake/skl-sst.c @@ -622,7 +622,7 @@ void skl_sst_dsp_cleanup(struct skl_dev *skl) skl_clear_module_table(dsp); skl_freeup_uuid_list(skl); - dsp->ops->free(dsp); + sst_dsp_free(dsp); if (skl->boot_complete && dsp->cl_dev.bufsize) { dsp->cl_dev.ops.cl_cleanup_controller(dsp);
Skylake is sst_dsp descendant. Rather than bypassing framework's flow, embrace it. sst_dsp_free invokes sst specific handler internally so nothing is missed. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> --- sound/soc/intel/skylake/cnl-sst-dsp.c | 1 - sound/soc/intel/skylake/skl-sst-dsp.c | 1 - sound/soc/intel/skylake/skl-sst.c | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-)