| Message ID | 20221211024109.671538-2-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ASoC: fsl_sai: Factor out SAI reset | expand |
Hi Marek,
I love your patch! Yet something to improve:
[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on linus/master v6.1-rc8 next-20221208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Marek-Vasut/ASoC-fsl_sai-Factor-out-SAI-reset/20221211-104415
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
patch link: https://lore.kernel.org/r/20221211024109.671538-2-marex%40denx.de
patch subject: [PATCH 2/2] ASoC: fsl_sai: Export transmitter MCLK as clock
config: hexagon-randconfig-r045-20221211
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/5e123e6575149ec2edb1a16fe989cc8d75cc96d1
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Marek-Vasut/ASoC-fsl_sai-Factor-out-SAI-reset/20221211-104415
git checkout 5e123e6575149ec2edb1a16fe989cc8d75cc96d1
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: clk_hw_get_name
>>> referenced by fsl_sai.c:1267 (sound/soc/fsl/fsl_sai.c:1267)
>>> sound/soc/fsl/fsl_sai.o:(mclk_gate_is_enabled) in archive vmlinux.a
>>> referenced by fsl_sai.c:1267 (sound/soc/fsl/fsl_sai.c:1267)
>>> sound/soc/fsl/fsl_sai.o:(mclk_gate_is_enabled) in archive vmlinux.a
>>> referenced by fsl_sai.c:1267 (sound/soc/fsl/fsl_sai.c:1267)
>>> sound/soc/fsl/fsl_sai.o:(mclk_gate_get_parent) in archive vmlinux.a
>>> referenced 7 more times
--
>> ld.lld: error: undefined symbol: __clk_get_name
>>> referenced by fsl_sai.c:1406 (sound/soc/fsl/fsl_sai.c:1406)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1406 (sound/soc/fsl/fsl_sai.c:1406)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:0 (sound/soc/fsl/fsl_sai.c:0)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced 1 more times
--
>> ld.lld: error: undefined symbol: of_clk_hw_register
>>> referenced by fsl_sai.c:1522 (sound/soc/fsl/fsl_sai.c:1522)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1522 (sound/soc/fsl/fsl_sai.c:1522)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1529 (sound/soc/fsl/fsl_sai.c:1529)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced 1 more times
--
>> ld.lld: error: undefined symbol: clk_hw_get_clk
>>> referenced by fsl_sai.c:1533 (sound/soc/fsl/fsl_sai.c:1533)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1533 (sound/soc/fsl/fsl_sai.c:1533)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1539 (sound/soc/fsl/fsl_sai.c:1539)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced 1 more times
--
>> ld.lld: error: undefined symbol: of_clk_hw_simple_get
>>> referenced by fsl_sai.c:1579 (sound/soc/fsl/fsl_sai.c:1579)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1579 (sound/soc/fsl/fsl_sai.c:1579)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
--
>> ld.lld: error: undefined symbol: devm_of_clk_add_hw_provider
>>> referenced by fsl_sai.c:1579 (sound/soc/fsl/fsl_sai.c:1579)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1579 (sound/soc/fsl/fsl_sai.c:1579)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
--
>> ld.lld: error: undefined symbol: clk_hw_unregister
>>> referenced by fsl_sai.c:1593 (sound/soc/fsl/fsl_sai.c:1593)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1593 (sound/soc/fsl/fsl_sai.c:1593)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced by fsl_sai.c:1596 (sound/soc/fsl/fsl_sai.c:1596)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_probe) in archive vmlinux.a
>>> referenced 5 more times
--
>> ld.lld: error: undefined symbol: clk_hw_is_enabled
>>> referenced by fsl_sai.c:688 (sound/soc/fsl/fsl_sai.c:688)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_reset) in archive vmlinux.a
>>> referenced by fsl_sai.c:688 (sound/soc/fsl/fsl_sai.c:688)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_reset) in archive vmlinux.a
>>> referenced by fsl_sai.c:690 (sound/soc/fsl/fsl_sai.c:690)
>>> sound/soc/fsl/fsl_sai.o:(fsl_sai_reset) in archive vmlinux.a
>>> referenced 1 more times
On Sun, Dec 11, 2022 at 10:41 AM Marek Vasut <marex@denx.de> 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@denx.de> > --- > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Jaroslav Kysela <perex@perex.cz> > Cc: Liam Girdwood <lgirdwood@gmail.com> > Cc: Mark Brown <broonie@kernel.org> > Cc: Nicolin Chen <nicoleotsuka@gmail.com> > Cc: Shengjiu Wang <shengjiu.wang@gmail.com> > Cc: Takashi Iwai <tiwai@suse.com> > Cc: Xiubo Li <Xiubo.Lee@gmail.com> > To: alsa-devel@alsa-project.org > --- > 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 > >
On 12/19/22 10:18, Shengjiu Wang wrote: [...] >> + 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. On the hardware I have available, MX8MNano EVK is not one of them. > there is kernel dump for "scheduling while atomic" Is it still there if you replace the usleep_range() with udelay() ? > [ 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 [...] >> @@ -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. What kind of issue ? Details please. [...]
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); 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" }; + 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;
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@denx.de> --- Cc: Fabio Estevam <festevam@gmail.com> Cc: Jaroslav Kysela <perex@perex.cz> Cc: Liam Girdwood <lgirdwood@gmail.com> Cc: Mark Brown <broonie@kernel.org> Cc: Nicolin Chen <nicoleotsuka@gmail.com> Cc: Shengjiu Wang <shengjiu.wang@gmail.com> Cc: Takashi Iwai <tiwai@suse.com> Cc: Xiubo Li <Xiubo.Lee@gmail.com> To: alsa-devel@alsa-project.org --- sound/soc/fsl/fsl_sai.c | 226 ++++++++++++++++++++++++++++++---------- sound/soc/fsl/fsl_sai.h | 3 + 2 files changed, 176 insertions(+), 53 deletions(-)