Re: [PATCH 2/2] ASoC: fsl_sai: Export transmitter MCLK as clock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 11, 2022 at 10:41 AM Marek Vasut <marex@xxxxxxx> wrote:

> For SAI to generate MCLK on external SoC pad, the transmitter must be
> enabled as well. With transmitter disabled, no clock are generated.
> Enable the transmitter using the TERE bit.
>
> Rather than doing ad-hoc enablement of the TERE bit, convert the mclk
> handling into a clock provider and adjust the SAI driver points which
> modify the TERE bit to instead enable and disable the clock. Register
> two clock, mclk_tx and mclk_rx, each representing TCSR and RCSR TERE
> bit respectively. Expose only the mclk_tx, since it does not really
> matter which TERE bit is enabled for the generation of output MCLK,
> but it does matter which TERE bit is enabled for internal operation
> of the SAI.
>
> As a nice bonus, this simplifies the MCLK handling in the driver itself.
>
> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> ---
> Cc: Fabio Estevam <festevam@xxxxxxxxx>
> Cc: Jaroslav Kysela <perex@xxxxxxxx>
> Cc: Liam Girdwood <lgirdwood@xxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>
> Cc: Nicolin Chen <nicoleotsuka@xxxxxxxxx>
> Cc: Shengjiu Wang <shengjiu.wang@xxxxxxxxx>
> Cc: Takashi Iwai <tiwai@xxxxxxxx>
> Cc: Xiubo Li <Xiubo.Lee@xxxxxxxxx>
> To: alsa-devel@xxxxxxxxxxxxxxxx
> ---
>  sound/soc/fsl/fsl_sai.c | 226 ++++++++++++++++++++++++++++++----------
>  sound/soc/fsl/fsl_sai.h |   3 +
>  2 files changed, 176 insertions(+), 53 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index ade5a6b44b66a..944450ba19b62 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -185,34 +185,9 @@ static int fsl_sai_set_dai_bclk_ratio(struct
> snd_soc_dai *dai,
>         return 0;
>  }
>
> -static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
> -               int clk_id, unsigned int freq, bool tx)
> +static int fsl_sai_set_dai_sysclk_tr(struct fsl_sai *sai, int clk_id,
> bool tx)
>  {
> -       struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> -       unsigned int ofs = sai->soc_data->reg_offset;
> -       u32 val_cr2 = 0;
> -
> -       switch (clk_id) {
> -       case FSL_SAI_CLK_BUS:
> -               val_cr2 |= FSL_SAI_CR2_MSEL_BUS;
> -               break;
> -       case FSL_SAI_CLK_MAST1:
> -               val_cr2 |= FSL_SAI_CR2_MSEL_MCLK1;
> -               break;
> -       case FSL_SAI_CLK_MAST2:
> -               val_cr2 |= FSL_SAI_CR2_MSEL_MCLK2;
> -               break;
> -       case FSL_SAI_CLK_MAST3:
> -               val_cr2 |= FSL_SAI_CR2_MSEL_MCLK3;
> -               break;
> -       default:
> -               return -EINVAL;
> -       }
> -
> -       regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
> -                          FSL_SAI_CR2_MSEL_MASK, val_cr2);
> -
> -       return 0;
> +       return clk_set_parent(sai->mclk_clk_out[tx],
> sai->mclk_clk[clk_id]);
>  }
>
>  static int fsl_sai_set_mclk_rate(struct snd_soc_dai *dai, int clk_id,
> unsigned int freq)
> @@ -257,13 +232,13 @@ static int fsl_sai_set_dai_sysclk(struct snd_soc_dai
> *cpu_dai,
>                 }
>         }
>
> -       ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq, true);
> +       ret = fsl_sai_set_dai_sysclk_tr(sai, clk_id, true);
>         if (ret) {
>                 dev_err(cpu_dai->dev, "Cannot set tx sysclk: %d\n", ret);
>                 return ret;
>         }
>
> -       ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq, false);
> +       ret = fsl_sai_set_dai_sysclk_tr(sai, clk_id, false);
>         if (ret)
>                 dev_err(cpu_dai->dev, "Cannot set rx sysclk: %d\n", ret);
>
> @@ -487,8 +462,7 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai,
> bool tx, u32 freq)
>         else
>                 return 0;
>
> -       regmap_update_bits(sai->regmap, reg, FSL_SAI_CR2_MSEL_MASK,
> -                          FSL_SAI_CR2_MSEL(sai->mclk_id[tx]));
> +       fsl_sai_set_dai_sysclk_tr(sai, sai->mclk_id[tx], tx);
>
>         if (savediv == 1)
>                 regmap_update_bits(sai->regmap, reg,
> @@ -707,32 +681,41 @@ static int fsl_sai_hw_free(struct snd_pcm_substream
> *substream,
>  static void fsl_sai_reset(struct fsl_sai *sai, bool rx, bool tx)
>  {
>         unsigned int ofs = sai->soc_data->reg_offset;
> +       bool tx_keep_tere = false, rx_keep_tere = false;
> +       u32 tcsr = 0, rcsr = 0;
> +
> +       if (sai->mclk_clk_out[0])
> +               rx_keep_tere = clk_hw_is_enabled(&(sai->mclk_clk_hw[0]));
> +       if (sai->mclk_clk_out[1])
> +               tx_keep_tere = clk_hw_is_enabled(&(sai->mclk_clk_hw[1]));
> +
> +       if (rx_keep_tere) {
> +               regmap_read(sai->regmap, FSL_SAI_RCSR(ofs), &rcsr);
> +               rcsr &= FSL_SAI_CSR_TERE;
> +       }
> +
> +       if (tx_keep_tere) {
> +               regmap_read(sai->regmap, FSL_SAI_TCSR(ofs), &tcsr);
> +               tcsr &= FSL_SAI_CSR_TERE;
> +       }
>
>         if (tx)
> -               regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
> FSL_SAI_CSR_SR);
> +               regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
> FSL_SAI_CSR_SR | tcsr);
>         if (rx)
> -               regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
> FSL_SAI_CSR_SR);
> +               regmap_write(sai->regmap, FSL_SAI_RCSR(ofs),
> FSL_SAI_CSR_SR | rcsr);
>         usleep_range(1000, 2000);
>

Not sure if you have test your patch.
there is kernel dump for "scheduling while atomic"

[  248.778590] BUG: scheduling while atomic: swapper/0/0/0x00010003
[  248.784618] Modules linked in:
[  248.787677] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
6.1.0-rc6-00014-gf2b129b4c056-dirty #136
[  248.796292] Hardware name: NXP i.MX8MNano EVK board (DT)
[  248.801604] Call trace:
[  248.804051]  dump_backtrace.part.0+0xdc/0xf0
[  248.808337]  show_stack+0x18/0x40
[  248.811660]  dump_stack_lvl+0x64/0x80
[  248.815327]  dump_stack+0x18/0x34
[  248.818645]  __schedule_bug+0x60/0x80
[  248.822311]  __schedule+0x630/0x710
[  248.825808]  schedule+0x5c/0xd0
[  248.828955]  schedule_hrtimeout_range_clock+0xf4/0x134
[  248.834096]  schedule_hrtimeout_range+0x14/0x20
[  248.838629]  usleep_range_state+0x74/0xb0
[  248.842641]  fsl_sai_reset+0x13c/0x1cc
[  248.846397]  fsl_sai_config_disable+0x94/0xb0
[  248.850760]  fsl_sai_trigger+0x208/0x29c
[  248.854688]  snd_soc_pcm_dai_trigger+0xcc/0x26c
[  248.859223]  soc_pcm_trigger+0x164/0x1f4
[  248.863154]  snd_pcm_do_stop+0x68/0x90



>         if (tx)
> -               regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), 0);
> +               regmap_write(sai->regmap, FSL_SAI_TCSR(ofs), tcsr);
>         if (rx)
> -               regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), 0);
> +               regmap_write(sai->regmap, FSL_SAI_RCSR(ofs), rcsr);
>  }
>
>  static void fsl_sai_config_disable(struct fsl_sai *sai, int dir)
>  {
>         unsigned int ofs = sai->soc_data->reg_offset;
>         bool tx = dir == TX;
> -       u32 xcsr, count = 100;
>
> -       regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> -                          FSL_SAI_CSR_TERE, 0);
> -
> -       /* TERE will remain set till the end of current frame */
> -       do {
> -               udelay(10);
> -               regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
> -       } while (--count && xcsr & FSL_SAI_CSR_TERE);
> +       clk_disable_unprepare(sai->mclk_clk_out[tx]);
>
>         regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
>                            FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
> @@ -780,8 +763,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
>                 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
>                                    FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
>
> -               regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> -                                  FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> +               clk_prepare_enable(sai->mclk_clk_out[tx]);
>                 /*
>                  * Enable the opposite direction for synchronous mode
>                  * 1. Tx sync with Rx: only set RE for Rx; set TE & RE for
> Tx
> @@ -794,8 +776,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream
> *substream, int cmd,
>                  * be safe to do, judging by years of testing results.
>                  */
>                 if (fsl_sai_dir_is_synced(sai, adir))
> -                       regmap_update_bits(sai->regmap,
> FSL_SAI_xCSR((!tx), ofs),
> -                                          FSL_SAI_CSR_TERE,
> FSL_SAI_CSR_TERE);
> +                       clk_prepare_enable(sai->mclk_clk_out[!tx]);
>
>                 regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
>                                    FSL_SAI_CSR_xIE_MASK, FSL_SAI_FLAGS);
> @@ -1281,6 +1262,86 @@ static int fsl_sai_read_dlcfg(struct fsl_sai *sai)
>         return 0;
>  }
>
> +static struct fsl_sai *mclk_get_clk_hw_to_sai(struct clk_hw *hw, bool *tx)
> +{
> +       *tx = !strcmp(clk_hw_get_name(hw), "mclk_tx");
> +
> +       return container_of(hw, struct fsl_sai, mclk_clk_hw[*tx]);
> +}
> +
> +static int mclk_gate_enable(struct clk_hw *hw)
> +{
> +       bool tx;
> +       struct fsl_sai *sai = mclk_get_clk_hw_to_sai(hw, &tx);
> +       unsigned int ofs = sai->soc_data->reg_offset;
> +
> +       pm_runtime_get_sync(&sai->pdev->dev);
> +
> +       /* Transmitter must be enabled to generate MCLK on pad */
> +       regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> +                          FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
> +
> +       return 0;
> +}
> +
> +static void mclk_gate_disable(struct clk_hw *hw)
> +{
> +       bool tx;
> +       struct fsl_sai *sai = mclk_get_clk_hw_to_sai(hw, &tx);
> +       unsigned int ofs = sai->soc_data->reg_offset;
> +       u32 xcsr, count = 100;
> +
> +       regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> FSL_SAI_CSR_TERE, 0);
> +
> +       /* TERE will remain set till the end of current frame */
> +       do {
> +               udelay(10);
> +               regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
> +       } while (--count && xcsr & FSL_SAI_CSR_TERE);
> +
> +       pm_runtime_put_sync(&sai->pdev->dev);
> +}
> +
> +static int mclk_gate_is_enabled(struct clk_hw *hw)
> +{
> +       bool tx;
> +       struct fsl_sai *sai = mclk_get_clk_hw_to_sai(hw, &tx);
> +       unsigned int ofs = sai->soc_data->reg_offset;
> +       u32 xcsr;
> +
> +       regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
> +
> +       return !!(xcsr & FSL_SAI_CSR_TERE);
> +}
> +
> +static u8 mclk_gate_get_parent(struct clk_hw *hw)
> +{
> +       bool tx;
> +       struct fsl_sai *sai = mclk_get_clk_hw_to_sai(hw, &tx);
> +
> +       return sai->mclk_id[tx];
> +}
> +
> +static int mclk_gate_set_parent(struct clk_hw *hw, u8 index)
> +{
> +       bool tx;
> +       struct fsl_sai *sai = mclk_get_clk_hw_to_sai(hw, &tx);
> +       unsigned int ofs = sai->soc_data->reg_offset;
> +
> +       regmap_update_bits(sai->regmap, FSL_SAI_xCR2(tx, ofs),
> +                          FSL_SAI_CR2_MSEL_MASK, FSL_SAI_CR2_MSEL(index));
> +
> +       return 0;
> +}
> +
> +const struct clk_ops mclk_gate_ops = {
> +       .enable         = mclk_gate_enable,
> +       .disable        = mclk_gate_disable,
> +       .is_enabled     = mclk_gate_is_enabled,
> +       .get_parent     = mclk_gate_get_parent,
> +       .set_parent     = mclk_gate_set_parent,
> +};
> +
>  static int fsl_sai_runtime_suspend(struct device *dev);
>  static int fsl_sai_runtime_resume(struct device *dev);
>
> @@ -1295,6 +1356,13 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>         int irq, ret, i;
>         int index;
>         u32 dmas[4];
> +       const char *mclk_parent_names[FSL_SAI_MCLK_MAX];
> +       const char *mclk_rxtx_names[2] = { "mclk_rx", "mclk_tx" };
>

please consider multi sai instance case, this name will duplicate
that cause sai probe issue.

best regards
wang shengjiu

+       struct clk_init_data mclk_init = {
> +               .ops            = &mclk_gate_ops,
> +               .parent_names   = mclk_parent_names,
> +               .num_parents    = FSL_SAI_MCLK_MAX,
> +       };
>
>         sai = devm_kzalloc(dev, sizeof(*sai), GFP_KERNEL);
>         if (!sai)
> @@ -1341,12 +1409,14 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>                                         i, PTR_ERR(sai->mclk_clk[i]));
>                         sai->mclk_clk[i] = NULL;
>                 }
> +               mclk_parent_names[i] = __clk_get_name(sai->mclk_clk[i]);
>         }
>
>         if (sai->soc_data->mclk0_is_mclk1)
>                 sai->mclk_clk[0] = sai->mclk_clk[1];
>         else
>                 sai->mclk_clk[0] = sai->bus_clk;
> +       mclk_parent_names[0] = __clk_get_name(sai->mclk_clk[0]);
>
>         fsl_asoc_get_pll_clocks(&pdev->dev, &sai->pll8k_clk,
>                                 &sai->pll11k_clk);
> @@ -1443,8 +1513,34 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>
>         /* Get sai version */
>         ret = fsl_sai_check_version(dev);
> -       if (ret < 0)
> +       if (ret < 0) {
>                 dev_warn(dev, "Error reading SAI version: %d\n", ret);
> +               goto err_pm_get_sync;
> +       }
> +
> +       mclk_init.name = mclk_rxtx_names[0];
> +       sai->mclk_clk_hw[0].init = &mclk_init;
> +       ret = of_clk_hw_register(np, &sai->mclk_clk_hw[0]);
> +       if (ret)
> +               goto err_pm_get_sync;
> +
> +       mclk_init.name = mclk_rxtx_names[1];
> +       sai->mclk_clk_hw[1].init = &mclk_init;
> +       ret = of_clk_hw_register(np, &sai->mclk_clk_hw[1]);
> +       if (ret)
> +               goto err_clk_unregister_rx;
> +
> +       sai->mclk_clk_out[0] = clk_hw_get_clk(&sai->mclk_clk_hw[0], NULL);
> +       if (IS_ERR(sai->mclk_clk_out[0])) {
> +               ret = PTR_ERR(sai->mclk_clk_out[0]);
> +               goto err_clk_unregister_tx;
> +       }
> +
> +       sai->mclk_clk_out[1] = clk_hw_get_clk(&sai->mclk_clk_hw[1], NULL);
> +       if (IS_ERR(sai->mclk_clk_out[1])) {
> +               ret = PTR_ERR(sai->mclk_clk_out[1]);
> +               goto err_clk_put_rx;
> +       }
>
>         /* Select MCLK direction */
>         if (of_find_property(np, "fsl,sai-mclk-direction-output", NULL) &&
> @@ -1455,7 +1551,7 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>
>         ret = pm_runtime_put_sync(dev);
>         if (ret < 0 && ret != -ENOSYS)
> -               goto err_pm_get_sync;
> +               goto err_clk_put_tx;
>
>         /*
>          * Register platform component before registering cpu dai for there
> @@ -1466,21 +1562,39 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>                 if (ret) {
>                         if (!IS_ENABLED(CONFIG_SND_SOC_IMX_PCM_DMA))
>                                 dev_err(dev, "Error: You must enable the
> imx-pcm-dma support!\n");
> -                       goto err_pm_get_sync;
> +                       goto err_clk_put_tx;
>                 }
>         } else {
>                 ret = devm_snd_dmaengine_pcm_register(dev, NULL, 0);
>                 if (ret)
> -                       goto err_pm_get_sync;
> +                       goto err_clk_put_tx;
>         }
>
>         ret = devm_snd_soc_register_component(dev, &fsl_component,
>                                               &sai->cpu_dai_drv, 1);
>         if (ret)
> -               goto err_pm_get_sync;
> +               goto err_clk_put_tx;
> +
> +       /* Expose only the TX clock */
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> +                                         &sai->mclk_clk_hw[1]);
> +       if (ret)
> +               goto err_clk_put_tx;
>
>         return ret;
>
> +err_clk_put_tx:
> +       clk_put(sai->mclk_clk_out[1]);
> +
> +err_clk_put_rx:
> +       clk_put(sai->mclk_clk_out[0]);
> +
> +err_clk_unregister_tx:
> +       clk_hw_unregister(&sai->mclk_clk_hw[1]);
> +
> +err_clk_unregister_rx:
> +       clk_hw_unregister(&sai->mclk_clk_hw[0]);
> +
>  err_pm_get_sync:
>         if (!pm_runtime_status_suspended(dev))
>                 fsl_sai_runtime_suspend(dev);
> @@ -1492,6 +1606,12 @@ static int fsl_sai_probe(struct platform_device
> *pdev)
>
>  static int fsl_sai_remove(struct platform_device *pdev)
>  {
> +       struct fsl_sai *sai = dev_get_drvdata(&pdev->dev);
> +
> +       clk_put(sai->mclk_clk_out[1]);
> +       clk_put(sai->mclk_clk_out[0]);
> +       clk_hw_unregister(&sai->mclk_clk_hw[1]);
> +       clk_hw_unregister(&sai->mclk_clk_hw[0]);
>         pm_runtime_disable(&pdev->dev);
>         if (!pm_runtime_status_suspended(&pdev->dev))
>                 fsl_sai_runtime_suspend(&pdev->dev);
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index 197748a888d5f..f2abf2d6de218 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -6,6 +6,7 @@
>  #ifndef __FSL_SAI_H
>  #define __FSL_SAI_H
>
> +#include <linux/clk-provider.h>
>  #include <linux/dma/imx-dma.h>
>  #include <sound/dmaengine_pcm.h>
>
> @@ -277,6 +278,8 @@ struct fsl_sai {
>         struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
>         struct clk *pll8k_clk;
>         struct clk *pll11k_clk;
> +       struct clk_hw mclk_clk_hw[2];   /* MCLK provider */
> +       struct clk *mclk_clk_out[2];    /* MCLK output */
>         struct resource *res;
>
>         bool is_consumer_mode;
> --
> 2.35.1
>
>



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux