1. 20 11月, 2019 2 次提交
    • K
      ASoC: soc-pcm: remove soc_pcm_private_free() · 0ced7b05
      Kuninori Morimoto 提交于
      soc-topology adds extra dai_link by using snd_soc_add_dai_link(),
      and removes it by snd_soc_romove_dai_link().
      
      This snd_soc_add/remove_dai_link() and/or its related
      functions are unbalanced before, and now, these are balance-uped.
      But, it finds the random operation issue, and it is reported by
      Pierre-Louis.
      
      When card was released, topology will call snd_soc_remove_dai_link()
      via (A).
      
      	static void soc_cleanup_card_resources(struct snd_soc_card *card)
      	{
      		struct snd_soc_dai_link *link, *_link;
      
      		/* This should be called before snd_card_free() */
      	(A)	soc_remove_link_components(card);
      
      		/* free the ALSA card at first; this syncs with pending operations */
      		if (card->snd_card) {
      	(B)		snd_card_free(card->snd_card);
      			card->snd_card = NULL;
      		}
      
      		/* remove and free each DAI */
      	(X)	soc_remove_link_dais(card);
      
      		for_each_card_links_safe(card, link, _link)
      	(C)		snd_soc_remove_dai_link(card, link);
      
      		...
      	}
      
      At (A), topology calls snd_soc_remove_dai_link().
      Then topology rtd, and its related all data are freed.
      
      Next, (B) is called, and then, pcm->private_free = soc_pcm_private_free()
      is called.
      
      	static void soc_pcm_private_free(struct snd_pcm *pcm)
      	{
      		struct snd_soc_pcm_runtime *rtd = pcm->private_data;
      
      		/* need to sync the delayed work before releasing resources */
      		flush_delayed_work(&rtd->delayed_work);
      		snd_soc_pcm_component_free(rtd);
      	}
      
      Here, it gets rtd via pcm->private_data.
      But, topology related rtd are already freed at (A).
      Normal sound card has no damage, becase it frees rtd at (C).
      
      These are finalizing rtd related data.
      Thus, these should be called when rtd was freed, not sound card
      was freed. It is very natural and understandable.
      
      In other words, pcm->private_free = soc_pcm_private_free()
      is no longer needed.
      
      Extra issue is that there is zero chance to call
      soc_remove_dai() for topology related dai at (X).
      Because (A) removes rtd connection from card too, and,
      (X) is based on card connected rtd.
      
      This means, (X) need to be called before (C) (= for normal sound)
      and (A) (= for topology).
      
      Now, I want to focus this patch which is the reason why
      snd_card_free() = (B) is located there.
      
      	commit 4efda5f2
      	("ASoC: Fix use-after-free at card unregistration")
      
      Original snd_card_free() was called last of this function.
      But moved to top to avoid use-after-free issue.
      The issue was happen at soc_pcm_free() which was pcm->private_free,
      today it is updated/renamed to soc_pcm_private_free().
      
      In other words, (B) need to be called before (C) (= for normal sound)
      and (A) (= for topology), because it needs (not yet freed) rtd.
      But, (A) need to be called before (B),
      because it needs card->snd_card pointer.
      
      If we call flush_delayed_work() and snd_soc_pcm_component_free()
      (= same as soc_pcm_private_free()) when rtd was freed (= (C), (A)),
      there is no reason to call snd_card_free() at top of this function.
      It can be called end of this function, again.
      
      But, in such case, it will likely break unbind again, as Takashi-san
      reported. When unbind is performed in a busy state, the code may
      release still-in-use resources.
      At least we need to call snd_card_disconnect_sync() at the first place.
      
      The final code will be...
      
      	static void soc_cleanup_card_resources(struct snd_soc_card *card)
      	{
      		struct snd_soc_dai_link *link, *_link;
      
      		if (card->snd_card)
      	(Z)		snd_card_disconnect_sync(card->snd_card);
      
      	(X)	soc_remove_link_dais(card);
      	(A)	soc_remove_link_components(card);
      
      		for_each_card_links_safe(card, link, _link)
      	(C)		snd_soc_remove_dai_link(card, link);
      
      		...
      		if (card->snd_card) {
      	(B)		snd_card_free(card->snd_card);
      			card->snd_card = NULL;
      		}
      	}
      
      To avoid release still-in-use resources,
      call snd_card_disconnect_sync() at (Z).
      
      (X) is needed for both non-topology and topology.
      
          topology removes rtd via (A), and
      non topology removes rtd via (C).
      
      snd_card_free() is no longer related to use-after-free issue.
      Thus, locating (B) is no problem.
      
      Fixes: df95a16d ("ASoC: soc-core: fix RIP warning on card removal")
      Fixes: bc7a9091 ("ASoC: soc-core: add soc_unbind_dai_link()")
      Reported-by: NPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
      Signed-off-by: NKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Tested-by: NPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
      Link: https://lore.kernel.org/r/87o8xax88g.wl-kuninori.morimoto.gx@renesas.comSigned-off-by: NMark Brown <broonie@kernel.org>
      0ced7b05
    • K
      ASoC: soc-component: tidyup snd_soc_pcm_component_new/free() parameter · b2b2afbb
      Kuninori Morimoto 提交于
      This patch uses rtd instead of pcm at snd_soc_pcm_component_new/free()
      parameter.
      This is prepare for dai_link remove bug fix on topology.
      Signed-off-by: NKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Link: https://lore.kernel.org/r/87pnhqx89j.wl-kuninori.morimoto.gx@renesas.comSigned-off-by: NMark Brown <broonie@kernel.org>
      b2b2afbb
  2. 08 11月, 2019 1 次提交
  3. 06 11月, 2019 1 次提交
    • R
      ASoC: pcm: update FE/BE trigger order based on the command · acbf2774
      Ranjani Sridharan 提交于
      Currently, the trigger orders SND_SOC_DPCM_TRIGGER_PRE/POST
      determine the order in which FE DAI and BE DAI are triggered.
      In the case of SND_SOC_DPCM_TRIGGER_PRE, the FE DAI is
      triggered before the BE DAI and in the case of
      SND_SOC_DPCM_TRIGGER_POST, the BE DAI is triggered before
      the FE DAI. And this order remains the same irrespective of the
      trigger command.
      
      In the case of the SOF driver, during playback, the FW
      expects the BE DAI to be triggered before the FE DAI during
      the START trigger. The BE DAI trigger handles the starting of
      Link DMA and so it must be started before the FE DAI is started
      to prevent xruns during pause/release. This can be addressed
      by setting the trigger order for the FE dai link to
      SND_SOC_DPCM_TRIGGER_POST. But during the STOP trigger,
      the FW expects the FE DAI to be triggered before the BE DAI.
      Retaining the same order during the START and STOP commands,
      results in FW error as the DAI component in the FW is still
      active.
      
      The issue can be fixed by mirroring the trigger order of
      FE and BE DAI's during the START and STOP trigger. So, with the
      trigger order set to SND_SOC_DPCM_TRIGGER_PRE, the FE DAI will be
      trigger first during SNDRV_PCM_TRIGGER_START/STOP/RESUME
      and the BE DAI will be triggered first during the
      STOP/SUSPEND/PAUSE commands. Conversely, with the trigger order
      set to SND_SOC_DPCM_TRIGGER_POST, the BE DAI will be triggered
      first during the SNDRV_PCM_TRIGGER_START/STOP/RESUME commands
      and the FE DAI will be triggered first during the
      SNDRV_PCM_TRIGGER_STOP/SUSPEND/PAUSE commands.
      Signed-off-by: NRanjani Sridharan <ranjani.sridharan@linux.intel.com>
      Signed-off-by: NPierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
      Link: https://lore.kernel.org/r/20191104224812.3393-2-ranjani.sridharan@linux.intel.comSigned-off-by: NMark Brown <broonie@kernel.org>
      acbf2774
  4. 24 10月, 2019 1 次提交
  5. 23 10月, 2019 1 次提交
  6. 08 10月, 2019 2 次提交
    • K
      ASoC: soc-component: remove snd_pcm_ops from component driver · e9067bb5
      Kuninori Morimoto 提交于
      No driver is using snd_pcm_ops on component driver.
      This patch removes it.
      Signed-off-by: NKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Link: https://lore.kernel.org/r/8736gb90by.wl-kuninori.morimoto.gx@renesas.comSigned-off-by: NMark Brown <broonie@kernel.org>
      e9067bb5
    • K
      ASoC: soc-core: merge snd_pcm_ops member to component driver · e2cb4a14
      Kuninori Morimoto 提交于
      Current snd_soc_component_driver has snd_pcm_ops, and each driver can
      have callback via it (1).
      But, it is mainly created for ALSA, thus, it doesn't have "component"
      as parameter for ALSA SoC (1)(2).
      Thus, each callback can't know it is called for which component.
      Thus, each callback currently is getting "component" by using
      snd_soc_rtdcom_lookup() with driver name (3).
      
      	--- ALSA SoC  ---
      	...
      	if (component->driver->ops &&
      	    component->driver->ops->open)
      (1)		return component->driver->ops->open(substream);
      	...
      
      	--- driver ---
      (2)	static int xxx_open(struct snd_pcm_substream *substream)
      	{
      		struct snd_soc_pcm_runtime *rtd = substream->private_data;
      (3)		struct snd_soc_component *component = snd_soc_rtdcom_lookup(..);
      		...
      	}
      
      It works today, but, will not work in the future if we support multi
      CPU/Codec/Platform, because 1 rtd might have multiple components which
      have same driver name.
      
      To solve this issue, each callback needs to be called with component.
      We already have many component driver callback.
      This patch copies each snd_pcm_ops member under component driver,
      and having "component" as parameter.
      
      	--- ALSA SoC  ---
      	...
      	if (component->driver->open)
      =>		return component->driver->open(component, substream);
      	...
      
      	--- driver ---
      =>	static int xxx_open(struct snd_soc_component *component,
      			    struct snd_pcm_substream *substream)
      	{
      		...
      	}
      
      *Note*
      
      Only Intel skl-pcm has .get_time_info implementation, but ALSA SoC
      framework doesn't call it so far.
      To keep its implementation, this patch keeps .get_time_info,
      but it is still not called.
      Intel guy need to support it in the future.
      Signed-off-by: NKuninori Morimoto <kuninori.morimoto.gx@renesas.com>
      Link: https://lore.kernel.org/r/87tv8raf3r.wl-kuninori.morimoto.gx@renesas.comSigned-off-by: NMark Brown <broonie@kernel.org>
      e2cb4a14
  7. 07 10月, 2019 1 次提交
  8. 01 10月, 2019 1 次提交
  9. 24 9月, 2019 1 次提交
  10. 15 8月, 2019 1 次提交
  11. 09 8月, 2019 1 次提交
  12. 05 8月, 2019 17 次提交
  13. 01 8月, 2019 2 次提交
  14. 24 7月, 2019 8 次提交