Re: [PATCH] ARM: dts: imx6sx-sdb: Add headphone detection for sound card
Hi Shawn > > > I didn't put headphone detect GPIO in audmux group in imx6sl-evk > > patch, Still in hog group. > > Ok, sorry. You grouped it with MX6SL_PAD_AUD_MCLK__AUDIO_CLK_OUT, > which I also think should not be part of the hog group. > > > And I think headphone detect GPIO is not belong to audmux group, it > > should Be in hog group. > > The hog group is better suited when there is no driver that can be associated > with that particular pin. > > For the headphone GPIO detect, I think it makes sense to group it with the > other audio-related pinctrl pins. I would like to know your opinion, should I move headphone detect GPIO To audmux group? Best regards Wang shengjiu
Re: [PATCH] ARM: dts: imx6sx-sdb: Add headphone detection for sound card
Hi Shawn, Fabio > > > > > I would like to know your opinion, should I move headphone detect > > > GPIO To audmux group? > > > > What about adding a dedicated pinctrl_hp for the headphone detect pin > > like it is done at: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fshawnguo%2Flinux.g > it%2 > > Ftree%2Farch%2Farm64%2Fboot%2Fdts%2Ffreescale%2Fimx8mq- > librem5.dtsi%3F > > h%3Dfor- > next%26id%3D8f0216b006e5f553d28c4c1a991b5234693a49cb%23n130 > > > p;data=02%7C01%7Cshengjiu.wang%40nxp.com%7C672e70414ead4170b617 > 08d847c > > > 05117%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63733823860 > 3810133& > > > amp;sdata=69K7rVsYB35Iq%2FdE%2FSf2%2B862VlwOFRwSTYBML7OQxUE%3 > Dres > > erved=0 > > > > My point is that we should avoid adding a hog group when possible. > > I agree. Hog group should be used as the last sort, when there is no clear > client device owning the pins. > Ok, thanks. I have sent v2. That I refined three patches for this topic. ARM: dts: imx6sx-sdb: Add headphone detection for sound card ARM: dts: imx6sl-evk: Add headphone detection for sound card ARM: dts: imx6sll-evk: Add audio sound card node Best regards Wang shengjiu
Re: [PATCH V2 2/2] ASoC: fsl_esai: recover the channel swap after xrun
> > > > + > > > > + /* restore registers by regcache_sync */ > > > > + fsl_esai_register_restore(esai_priv); > > > > + > > > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, > > > > +ESAI_xCR_xPR_MASK, 0); > > > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, > > > > +ESAI_xCR_xPR_MASK, 0); > > > > > > And just for curious, can (or shall) we stuff this personal reset to > > > the reset() function? I found this one is a part of the reset > > > routine being mentioned in the RM -- it was done after ESAI reset is > done via ECR register. > > > > > > > There is a problem to do this, TPR/RPR need to be clear after > > configure the control register. (TCCR, TCR). So it seems not only one > > place (reset function) need to be changed. > > Do you know (or remember) why we suddenly involve this TPR/PRP? > The driver has no problem so far, even if we don't have them. > > The "personal reset" sounds like a feature that we would use to reset TX or > RX individually, while this hw_reset() does a full reset for both TX and RX. > So I wonder whether they're necessary. The hw_reset flow is suggested by design team, so involve TRP/RPP is from them, I don't know the detail. Best regards Wang shengjiu
Re: [PATCH] ASoC: imx-audmux: Add driver suspend and resume to support MEGA Fast
Hi Mark > > On Fri, Aug 16, 2019 at 01:03:14AM -0400, Shengjiu Wang wrote: > > > + for (i = 0; i < reg_max; i++) > > + regcache[i] = readl(audmux_base + i * 4); > > If only there were some framework which provided a register cache! Yes, next step I can refine this driver to use the regmap. Best regards Wang shengjiu
RE: [RFC/RFT PATCH v2] ASoC: fsl_esai: Revert "ETDR and TX0~5 registers are non volatile"
Hi
>
> Commit 8973112aa41b ("ASoC: fsl_esai: ETDR and TX0~5 registers are non
> volatile") removed TX data registers from the volatile_reg list and appended
> default values for them. However, being data registers of TX, they should
> not have been removed from the list because they should not be cached --
> see the following reason.
>
> When doing regcache_sync(), this operation might accidentally write some
> dirty data to these registers, in case that cached data happen to be
> different from the default ones, which might also result in a channel shift or
> swap situation, since the number of write-via-sync operations at ETDR
> would very unlikely match the channel number.
>
> So this patch reverts the original commit to keep TX data registers in
> volatile_reg list in order to prevent them from being written by
> regcache_sync().
>
> Note: this revert is not a complete revert as it keeps those macros of
> registers remaining in the default value list while the original commit also
> changed other entries in the list. And this patch isn't very necessary to Cc
> stable tree since there has been always a FIFO reset operation around the
> regcache_sync() call, even prior to this reverted commit.
>
> Signed-off-by: Nicolin Chen
> Cc: Shengjiu Wang
> ---
> Hi Mark,
> In case there's no objection against the patch, I'd still like to wait for a
> Tested-by from NXP folks before submitting it. Thanks!
bool regmap_volatile(struct regmap *map, unsigned int reg)
{
if (!map->format.format_write && !regmap_readable(map, reg))
return false;
Actually with this patch, the regcache_sync will write the 0 to ETDR, even
It is declared volatile, the reason is that in regmap_volatile(), the first
condition
(!map->format.format_write && !regmap_readable(map, reg)) is true.
So the regmap_volatile will return false.
And in regcache_reg_needs_sync(), because there is no default value
It will return true, then the ETDR need be synced, and be written 0.
Here is the code for regcache_default_sync()
static int regcache_default_sync(struct regmap *map, unsigned int min,
unsigned int max)
{
unsigned int reg;
for (reg = min; reg <= max; reg += map->reg_stride) {
unsigned int val;
int ret;
if (regmap_volatile(map, reg) ||
!regmap_writeable(map, reg))
continue;
ret = regcache_read(map, reg, );
if (ret)
return ret;
if (!regcache_reg_needs_sync(map, reg, val))
continue;
map->cache_bypass = true;
ret = _regmap_write(map, reg, val);
map->cache_bypass = false;
Best regards
Wang shengjiu
Re: [PATCH] ARM: dts: imx7d-sdb: Add notes for audio sound card
> > Configure the SAI device node, configure audio clock and pinctrl. > > > > Enable the audio sound card, which use the SAI1 and wm8960, and enable > > headphone detection. > > > > Signed-off-by: Shengjiu Wang > > s/notes/nodes in subject? Yes. Thanks for fixing it. Best regards Wang shengjiu
Re: [PATCH] ARM: dts: imx6sx-sdb: Add headphone detection for sound card
> > {
> > imx6x-sdb {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <_hog>;
>
> Instead of adding a hog group, please add the headphone detect GPIO under
> the audmux group like you did in the imx6sl-evk patch.
>
I didn't put headphone detect GPIO in audmux group in imx6sl-evk patch,
Still in hog group.
And I think headphone detect GPIO is not belong to audmux group, it should
Be in hog group.
Best regards
Wang shengjiu
Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
Hi > > On Fri, Sep 13, 2019 at 05:48:40AM +0000, S.j. Wang wrote: > > Hi > > > > > > > > On Tue, Sep 10, 2019 at 02:07:25AM +, S.j. Wang wrote: > > > > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote: > > > > > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE > > > > > > format should not be supported, it is word width is 20bit. > > > > > > > > > > I thought 3LE used 24-bit physical width. And the driver assigns > > > > > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit > > > > > would go for that 24-bit slot also. I don't clearly recall if I > > > > > had explicitly tested S20_3LE, but I feel it should work since I put > there... > > > > > > > > For S20_3LE, the width is 20bit, but the ASRC only support 24bit, > > > > if set the ASRMCR1n.IWD= 24bit, because the actual width is 20 > > > > bit, the volume is Lower than expected, it likes 24bit data right > > > > shift 4 > bit. > > > > So it is not supported. > > > > > > Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought > > > they're left aligned... > > > > > > If this is the case...shouldn't we have the same lower-volume > > > problem for all hardwares that support S20_3LE now? > > > > Actually some hardware/module when they do transmission from FIFO to > > shift register, they can select the start bit, for example from the > > 20th bit. but not all module have this capability. > > > > For ASRC, it haven't. IWD can only cover the data width, there is no > > Other bit for slot width. > > Okay..let's drop the S20_3LE then. But would it be possible for you to > elaborate the reasoning into the commit message also? Just for case when > people ask why we remove it simply. > > Thanks OK. Best regards Wang shengjiu
RE: [PATCH V2 3/4] ASoC: pcm_dmaengine: Extract snd_dmaengine_pcm_set_runtime_hwparams
Hi > > When set the runtime hardware parameters, we may need to query the > capability of DMA to complete the parameters. > > This patch is to Extract this operation from > dmaengine_pcm_set_runtime_hwparams function to a separate function > snd_dmaengine_pcm_set_runtime_hwparams, that other components > which need this feature can call this function. > > Signed-off-by: Shengjiu Wang > --- kbuild test robot report compile issue, will resend v3 after fixing. Best regards Wang shengjiu
Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
Hi > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote: > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format > > should not be supported, it is word width is 20bit. > > I thought 3LE used 24-bit physical width. And the driver assigns > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit would go > for that 24-bit slot also. I don't clearly recall if I had explicitly tested > S20_3LE, but I feel it should work since I put there... > > Thanks > Nicolin > For S20_3LE, the width is 20bit, but the ASRC only support 24bit, if set the ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the volume is Lower than expected, it likes 24bit data right shift 4 bit. So it is not supported. Best regards Wang shengjiu
RE: [EXT] Re: [PATCH 1/3] ASoC: fsl_asrc: Use in(out)put_format instead of in(out)put_word_width
Hi > > On Mon, Sep 09, 2019 at 06:33:19PM -0400, Shengjiu Wang wrote: > > snd_pcm_format_t is more formal than enum asrc_word_width, which > has > > two property, width and physical width, which is more accurate than > > enum asrc_word_width. So it is better to use in(out)put_format instead > > of in(out)put_word_width. > > Hmm...I don't really see the benefit of using snd_pcm_format_t here...I > mean, I know it's a generic one, and would understand if we use it as a > param for a common API. But this patch merely packs the "width" by > intentionally using this snd_pcm_format_t and then adds another > translation to unpack it.. I feel it's a bit overcomplicated. Or am I missing > something? > > And I feel it's not necessary to use ALSA common format in our own "struct > asrc_config" since it is more IP/register specific. > > Thanks > Nicolin > As you know, we have another M2M function internally, when user want to Set the format through M2M API, it is better to use snd_pcm_format_t instead the Width, for snd_pcm_format_t include two property, data with and physical width In driver some place need data width, some place need physical width. For example how to distinguish S24_LE and S24_3LE in driver, DMA setting needs The physical width, but ASRC need data width. Another purpose is that we have another new designed ASRC, which support more Formats, I would like it can share same API with this ASRC, using snd_pcm_format_t That we can use the common API, like snd_pcm_format_linear, snd_pcm_format_big_endian to get the property of the format, which is needed by driver. Best regards Wang shengjiu
RE: [EXT] Re: [PATCH 3/3] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
Hi
>
> On Mon, Sep 09, 2019 at 06:33:21PM -0400, Shengjiu Wang wrote:
> > There is error "aplay: pcm_write:2023: write error: Input/output error"
> > on i.MX8QM/i.MX8QXP platform for S24_3LE format.
> >
> > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit
> > sample, but we didn't add any constraint, that cause issues.
> >
> > So we need to query the caps of dma, then update the hw parameters
> > according to the caps.
>
> > @@ -285,8 +293,81 @@ static int fsl_asrc_dma_startup(struct
> > snd_pcm_substream *substream)
> >
> > runtime->private_data = pair;
> >
> > - snd_pcm_hw_constraint_integer(substream->runtime,
> > - SNDRV_PCM_HW_PARAM_PERIODS);
> > + ret = snd_pcm_hw_constraint_integer(substream->runtime,
> > + SNDRV_PCM_HW_PARAM_PERIODS);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to set pcm hw params periods\n");
> > + return ret;
> > + }
> > +
> > + dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
> > +
> > + /* Request a temp pair, which is release in the end */
> > + fsl_asrc_request_pair(1, pair);
>
> Not sure if it'd be practical, but a pair request could fail. Will probably
> need
> to check return value.
>
> And a quick feeling is that below code is mostly identical to what is in the
> soc-generic-dmaengine-pcm.c file. So I'm wondering if we could abstract a
> helper function somewhere in the ASoC core: Mark?
>
> Thanks
> Nicolin
>
Yes, it refers to the code in soc-generic-dmaengine-pcm.c, if there is a common
API, this is helpful.
Best regards
Wang shengjiu
Re: [PATCH V3 2/2] ASoC: fsl_esai: recover the channel swap after xrun
>
> Hi Shengjiu,
>
> Mostly looks good to me, just some small comments.
>
> On Mon, Jul 08, 2019 at 02:38:52PM +0800, shengjiu.w...@nxp.com wrote:
>
> > +static void fsl_esai_hw_reset(unsigned long arg) {
> > + struct fsl_esai *esai_priv = (struct fsl_esai *)arg;
> > + u32 saisr, tfcr, rfcr;
> > + bool tx = true, rx = false, enabled[2];
>
> Could we swap the lines of u32 and bool? It'd look better.
>
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> > +ESAI_xCR_xPR_MASK, ESAI_xCR_xPR);
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR,
> > +ESAI_xCR_xPR_MASK, ESAI_xCR_xPR);
>
> Let's add a line of comments for these two:
> /* Enforce ESAI personal resets for both TX and RX */
>
> > + /*
> > + * Restore registers by regcache_sync, and ignore
> > + * return value
> > + */
>
> Could fit into single-line?
>
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
> > +ESAI_xCR_xPR_MASK, 0);
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR,
> > +ESAI_xCR_xPR_MASK, 0);
> > +
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_PRRC,
> > +ESAI_PRRC_PDC_MASK, ESAI_PRRC_PDC(ESAI_GPIO));
> > + regmap_update_bits(esai_priv->regmap, REG_ESAI_PCRC,
> > +ESAI_PCRC_PC_MASK, ESAI_PCRC_PC(ESAI_GPIO));
>
> Could remove the blank line and add a line of comments:
> /* Remove ESAI personal resets by configuring PCRC and PRRC also */
>
> Btw, I still feel this personal reset can be stuffed into one of the wrapper
> functions. But let's keep this simple for now.
>
> > + regmap_read(esai_priv->regmap, REG_ESAI_SAISR, );
>
> Why do we read saisr here? All its bits would get cleared by the hardware
> reset. If it's a must to clear again, we should add a line of comments to
> emphasize it.
This line can be removed.
Best regards
Wang Shengjiu
Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
Hi > > + /* > > + * Add fifo reset here, because the regcache_sync will > > + * write one more data to ETDR. > > + * Which will cause channel shift. > > Sounds like a bug to me...should fix it first by marking the data registers as > volatile. > The ETDR is a writable register, it is not volatile. Even we change it to Volatile, I don't think we can't avoid this issue. for the regcache_sync Just to write this register, it is correct behavior. Best regards Wang shengjiu
Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
Hi > On Thu, May 23, 2019 at 09:53:42AM +0000, S.j. Wang wrote: > > > > + /* > > > > + * Add fifo reset here, because the regcache_sync will > > > > + * write one more data to ETDR. > > > > + * Which will cause channel shift. > > > > > > Sounds like a bug to me...should fix it first by marking the data > > > registers as volatile. > > > > > The ETDR is a writable register, it is not volatile. Even we change it > > to Volatile, I don't think we can't avoid this issue. for the > > regcache_sync Just to write this register, it is correct behavior. > > Is that so? Quoting the comments of regcache_sync(): > "* regcache_sync - Sync the register cache with the hardware. > * > * @map: map to configure. > * > * Any registers that should not be synced should be marked as > * volatile." > > If regcache_sync() does sync volatile registers too as you said, I don't mind > having this FIFO reset WAR for now, though I think this mismatch between > the comments and the actual behavior then should get people's attention. > > Thank you ETDR is not volatile, if we mark it is volatile, is it correct? Bets regards Wang shengjiu
RE: [EXT] Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
Hi > > On Tue, Sep 10, 2019 at 02:07:25AM +0000, S.j. Wang wrote: > > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote: > > > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format > > > > should not be supported, it is word width is 20bit. > > > > > > I thought 3LE used 24-bit physical width. And the driver assigns > > > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit > > > would go for that 24-bit slot also. I don't clearly recall if I had > > > explicitly tested S20_3LE, but I feel it should work since I put there... > > > > For S20_3LE, the width is 20bit, but the ASRC only support 24bit, if > > set the ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the > > volume is Lower than expected, it likes 24bit data right shift 4 bit. > > So it is not supported. > > Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought they're left > aligned... > > If this is the case...shouldn't we have the same lower-volume problem for > all hardwares that support S20_3LE now? Actually some hardware/module when they do transmission from FIFO to shift register, they can select the start bit, for example from the 20th bit. but not all module have this capability. For ASRC, it haven't. IWD can only cover the data width, there is no Other bit for slot width. Best regards Wang shengjiu
Re: [PATCH] ASoC: fsl_asrc: refine the setting of internal clock divider
Hi
>
> On Thu, Oct 17, 2019 at 02:21:08PM +0800, Shengjiu Wang wrote:
> > For P2P output, the output divider should align with the output sample
>
> I think we should avoid "P2P" (or "M2M") keyword in the mainline code as
> we know M2M will never get merged while somebody working with the
> mainline and caring about new feature might be confused.
Ok. But we still curious that is there a way to upstream m2m?
>
> > rate, if use ideal sample rate, there will be a lot of overload, which
> > would cause underrun.
>
> If I understand it correctly, setting to ideal ratio provides a faster
> converting
> speed but increases the load of the processor of ASRC. So we choose a
> slower converting speed here since real- time playback mode doesn't really
> need a faster conversion?
Yes. Slower speed is enough for real-time playback
>
> It makes sense to me, yet I feel that the delay at the beginning of the audio
> playback might be longer as a compromise. I am okay with this decision
> though...
>
> > The maximum divider of asrc clock is 1024, but there is no judgement
> > for this limitaion in driver, which may cause the divider setting not
> > correct.
> >
> > For non-ideal ratio mode, the clock rate should divide the sample rate
> > with no remainder, and the quotient should be less than 1024.
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/fsl_asrc.c | 40
> > +++-
> > 1 file changed, 31 insertions(+), 9 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c index
> > 0bf91a6f54b9..44d05ec28bd3 100644
> > --- a/sound/soc/fsl/fsl_asrc.c
> > +++ b/sound/soc/fsl/fsl_asrc.c
> > @@ -260,7 +260,7 @@ static int fsl_asrc_set_ideal_ratio(struct
> fsl_asrc_pair *pair,
> > * of struct asrc_config which includes in/output sample rate, width,
> channel
> > * and clock settings.
> > */
> > -static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
> > +static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair, bool p2p)
> > {
> > struct asrc_config *config = pair->config;
> > struct fsl_asrc *asrc_priv = pair->asrc_priv; @@ -268,7 +268,8
> > @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
> > enum asrc_word_width input_word_width;
> > enum asrc_word_width output_word_width;
> > u32 inrate, outrate, indiv, outdiv;
> > - u32 clk_index[2], div[2];
> > + u32 clk_index[2], div[2], rem[2];
> > + u64 clk_rate;
> > int in, out, channels;
> > int pre_proc, post_proc;
> > struct clk *clk;
> > @@ -351,7 +352,9 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
> *pair)
> > /* We only have output clock for ideal ratio mode */
> > clk = asrc_priv->asrck_clk[clk_index[ideal ? OUT : IN]];
> >
> > - div[IN] = clk_get_rate(clk) / inrate;
> > + clk_rate = clk_get_rate(clk);
>
> The fsl_asrc.c file has config.inclk being set to INCLK_NONE and this sets the
> "ideal" in this function to true. So, although we tend to not use ideal ratio
> setting for p2p cases, yet the input clock is still not physically connected,
> so
> we still use output clock for div[IN] calculation?
For p2p case, it can be ideal or non-ideal. For non-ideal, we still use
Output clock for div calculation.
>
> I am thinking something simplier: if we decided not to use ideal ratio for
> "P2P", instead of adding "bool p2p" with the confusing "ideal" in this
> function, could we just set config.inclk to the same clock as the output one
> for "P2P"? By doing so, "P2P" won't go through ideal ratio mode while still
> having a clock rate from the output clock for div[IN] calculation here.
Bool p2p is to force output rate to be sample rate, no impact to ideal
Ratio mode.
>
> > + rem[IN] = do_div(clk_rate, inrate);
> > + div[IN] = (u32)clk_rate;
> > if (div[IN] == 0) {
>
> Could we check div[IN] and rem[IN] here? Like:
> if (div[IN] == 0 || div[IN] > 1024) {
> pair_err();
> goto out;
> }
>
> if (!ideal && rem[IN]) {
> pair_err();
> goto out;
> }
>
> According to your commit log, I think the max-1024 limitation should be
> applied to all cases, not confined to "!ideal" cases right? And we should
> add some comments also, indicating it is limited by hardware.
For ideal mode, my test result is the divider not impact the output result.
Which means it is ok for ideal mode even divider is not correct...
>
> > pair_err("failed to support input sample rate %dHz by
> asrck_%x\n",
> > inrate, clk_index[ideal ? OUT : IN]); @@
> > -360,11 +363,20 @@ static int fsl_asrc_config_pair(struct
> > fsl_asrc_pair *pair)
> >
> > clk = asrc_priv->asrck_clk[clk_index[OUT]];
> >
> > - /* Use fixed output rate for Ideal Ratio mode (INCLK_NONE) */
> > - if (ideal)
> > - div[OUT] = clk_get_rate(clk) / IDEAL_RATIO_RATE;
[PATCH] ARM64: dts: imx8mm-evk: Assigned clocks for audio plls
Assign clocks and clock-rates for audio plls, that audio
drivers can utilize them.
Add dai-tdm-slot-num and dai-tdm-slot-width for sound-wm8524,
that sai driver can generate correct bit clock.
Fixes: 13f3b9fdef6c ("arm64: dts: imx8mm-evk: Enable audio codec wm8524")
Signed-off-by: Shengjiu Wang
---
arch/arm64/boot/dts/freescale/imx8mm-evk.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mm.dtsi| 8 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
index f7a15f3904c2..13137451b438 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dts
@@ -62,6 +62,8 @@
cpudai: simple-audio-card,cpu {
sound-dai = <>;
+ dai-tdm-slot-num = <2>;
+ dai-tdm-slot-width = <32>;
};
simple-audio-card,codec {
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 5f9d0da196e1..2139c0a9c495 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -479,14 +479,18 @@
< IMX8MM_CLK_AUDIO_AHB>,
<
IMX8MM_CLK_IPG_AUDIO_ROOT>,
< IMX8MM_SYS_PLL3>,
- < IMX8MM_VIDEO_PLL1>;
+ < IMX8MM_VIDEO_PLL1>,
+ < IMX8MM_AUDIO_PLL1>,
+ < IMX8MM_AUDIO_PLL2>;
assigned-clock-parents = <
IMX8MM_SYS_PLL3_OUT>,
<
IMX8MM_SYS_PLL1_800M>;
assigned-clock-rates = <0>,
<4>,
<4>,
<75000>,
- <59400>;
+ <59400>,
+ <393216000>,
+ <361267200>;
};
src: reset-controller@3039 {
--
2.21.0
Re: [PATCH V3 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
Hi
>
> One issue for error-out and some nit-pickings inline. Thanks.
>
> On Thu, Sep 19, 2019 at 08:11:42PM +0800, Shengjiu Wang wrote:
> > There is error "aplay: pcm_write:2023: write error: Input/output error"
> > on i.MX8QM/i.MX8QXP platform for S24_3LE format.
> >
> > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit
> > sample, but we didn't add any constraint, that cause issues.
> >
> > So we need to query the caps of dma, then update the hw parameters
> > according to the caps.
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/fsl_asrc.c | 4 +--
> > sound/soc/fsl/fsl_asrc.h | 3 +++
> > sound/soc/fsl/fsl_asrc_dma.c | 52
> > +++-
> > 3 files changed, 50 insertions(+), 9 deletions(-)
> >
> > @@ -276,6 +274,11 @@ static int fsl_asrc_dma_startup(struct
> snd_pcm_substream *substream)
> > struct device *dev = component->dev;
> > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> > struct fsl_asrc_pair *pair;
> > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> > + u8 dir = tx ? OUT : IN;
> > + struct dma_chan *tmp_chan;
> > + struct snd_dmaengine_dai_dma_data *dma_data;
>
> Nit: would it be possible to reorganize these a bit? Usually we put struct
> things together unless there is a dependency, similar to
> fsl_asrc_dma_hw_params().
>
> > @@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct
> > snd_pcm_substream *substream)
> >
> > runtime->private_data = pair;
> >
> > + /* Request a temp pair, which is release in the end */
>
> Nit: "which will be released later" or "and will release it later"? And could
> we use a work like "dummy"? Or at least I would love to see the comments
> explaining the parameter "1"
> in the function call below.
>
> > + ret = fsl_asrc_request_pair(1, pair);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to request asrc pair\n");
> > + return ret;
> > + }
> > +
> > + tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> > + if (!tmp_chan) {
> > + dev_err(dev, "can't get dma channel\n");
>
> Could we align with other error messages using "failed to"?
>
> > + ret = snd_soc_set_runtime_hwparams(substream,
> _imx_hardware);
> > + if (ret)
> > + return ret;
> > +
> [...]
> > + dma_release_channel(tmp_chan);
> > + fsl_asrc_release_pair(pair);
>
> I think we need an "out:" here for those error-out routines to goto.
> Otherwise, it'd be a pair leak?
>
> > +
>
> Could we drop this? There is a blank line below already :)
>
Will update them.
Best regards
Wang Shengjiu
Re: [PATCH V4 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
Hi
> On Tue, Sep 24, 2019 at 06:52:35PM +0800, Shengjiu Wang wrote:
> > There is error "aplay: pcm_write:2023: write error: Input/output error"
> > on i.MX8QM/i.MX8QXP platform for S24_3LE format.
> >
> > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit
> > sample, but we didn't add any constraint, that cause issues.
> >
> > So we need to query the caps of dma, then update the hw parameters
> > according to the caps.
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/fsl_asrc.c | 4 +--
> > sound/soc/fsl/fsl_asrc.h | 3 ++
> > sound/soc/fsl/fsl_asrc_dma.c | 59
> > +++-
> > 3 files changed, 56 insertions(+), 10 deletions(-)
> >
> > @@ -270,12 +268,17 @@ static int fsl_asrc_dma_hw_free(struct
> > snd_pcm_substream *substream)
> >
> > static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream)
> > {
> > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> > struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > struct snd_pcm_runtime *runtime = substream->runtime;
> > struct snd_soc_component *component =
> snd_soc_rtdcom_lookup(rtd,
> > DRV_NAME);
> > + struct snd_dmaengine_dai_dma_data *dma_data;
> > struct device *dev = component->dev;
> > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev);
> > struct fsl_asrc_pair *pair;
> > + struct dma_chan *tmp_chan = NULL;
> > + u8 dir = tx ? OUT : IN;
> > + int ret = 0;
> >
> > pair = kzalloc(sizeof(struct fsl_asrc_pair), GFP_KERNEL);
>
> Sorry, I didn't catch it previously. We would need to release this memory
> also for all error-out paths, as the code doesn't have any error-out routine,
> prior to applying this change.
>
> > if (!pair)
> > @@ -285,11 +288,51 @@ static int fsl_asrc_dma_startup(struct
> > snd_pcm_substream *substream)
>
> > + /* Request a dummy pair, which will be released later.
> > + * Request pair function needs channel num as input, for this
> > + * dummy pair, we just request "1" channel temporary.
> > + */
>
> "temporary" => "temporarily"
>
> > + ret = fsl_asrc_request_pair(1, pair);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to request asrc pair\n");
> > + return ret;
> > + }
> > +
> > + /* Request a dummy dma channel, which will be release later. */
>
> "release" => "released"
Ok, will update them.
Best regards
Wang shengjiu
Re: [PATCH V5 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
Hi
> Just a small concern...
>
> On Thu, Sep 26, 2019 at 09:29:51AM +0800, Shengjiu Wang wrote:
> > static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream)
> > {
> > +
> > + release_pair = false;
> > + ret = snd_soc_set_runtime_hwparams(substream,
> > + _imx_hardware);
>
> This set_runtime_hwparams() always returns 0 for now, but if one day it
> changes and it fails here, kfree() will be still ignored although the
> startup()
> gets error-out.
>
> We could avoid this if we continue to ignore the return value like the
> current code. Or we may check ret at kfree() also?
I like to ignore the return value.
Best regards
Wang shengjiu
Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
Hi > > > > > Sounds like a bug to me...should fix it first by marking the > > > > > data registers as volatile. > > > > > > > > > The ETDR is a writable register, it is not volatile. Even we > > > > change it to Volatile, I don't think we can't avoid this issue. > > > > for the regcache_sync Just to write this register, it is correct > > > > behavior. > > > > > > Is that so? Quoting the comments of regcache_sync(): > > > "* regcache_sync - Sync the register cache with the hardware. > > > * > > > * @map: map to configure. > > > * > > > * Any registers that should not be synced should be marked as > > > * volatile." > > > > > > If regcache_sync() does sync volatile registers too as you said, I > > > don't mind having this FIFO reset WAR for now, though I think this > > > mismatch between the comments and the actual behavior then should > get people's attention. > > > > > > Thank you > > > > ETDR is not volatile, if we mark it is volatile, is it correct? > > Well, you have a point -- it might not be ideally true, but it sounds like a > correct fix to me according to this comments. > > We can wait for Mark's comments or just send a patch to the mail list for > review. > > Thanks you I test this patch, we don't need to reset the FIFO, and regcache_sync didn't Write the ETDR even the EDTR is not volatile. This fault maybe caused by Legacy, in the beginning we add this patch in internal branch, there maybe Something cause this issue, but now can't reproduced. So I will remove the reset of FIFO. Best regards Wang Shengjiu
Re: [PATCH V2 2/2] ASoC: fsl_esai: recover the channel swap after xrun
> > > + > > + /* restore registers by regcache_sync */ > > + fsl_esai_register_restore(esai_priv); > > + > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, > > +ESAI_xCR_xPR_MASK, 0); > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, > > +ESAI_xCR_xPR_MASK, 0); > > And just for curious, can (or shall) we stuff this personal reset to the > reset() > function? I found this one is a part of the reset routine being mentioned in > the RM -- it was done after ESAI reset is done via ECR register. > There is a problem to do this, TPR/RPR need to be clear after configure the control register. (TCCR, TCR). So it seems not only one place (reset function) need to be changed. Best regards Wang shengjiu
Re: [PATCH 0/5] Add audio support for imx6sx platform
Hi > > On Thu, Jun 18, 2020 at 02:03:44PM +0800, Shengjiu Wang wrote: > > Add audio support for imx6sx platform. > > Enable ASRC, ESAI, SPDIF, MQS. > > > > Shengjiu Wang (5): > > ARM: dts: imx6sx: Enable ASRC device > > ARM: dts: imx6sx-sdb: Add MQS support > > Applied the series, except this one which doesn't apply to my branch. Thanks. I have resolved the conflict and resent this commit. [RESEND PATCH] ARM: dts: imx6sx-sdb: Add MQS support Best regards Wang shengjiu
Re: [PATCH V3 1/2] ASoC: fsl_asrc: replace the process_option table with function
Hi
>
> On Thu, Apr 18, 2019 at 08:50:48AM +0000, S.j. Wang wrote:
> > > And this is according to IMX6DQRM:
> > > Limited support for the case when output sampling rates is
> > > between 8kHz and 30kHz. The limitation is the supported ratio
> > > (Fsin/Fsout) range as between 1/24 to 8
> > >
> > > This should cover your 8.125 condition already, even if having an
> > > outrate range between [8KHz, 30KHz] check, since an outrate above
> > > 30KHz will not have an inrate bigger than 8.125 times of it, given
> > > the maximum input rate is 192KHz.
> > >
> > > So I think that we can just drop that 8.125 condition from your
> > > change and there's no need to error out any more.
> > >
> > No, if outrate=8kHz, inrate > 88.2kHz, these cases are not supported.
> > This is not covered by
> >
> > if ((outrate > 8000 && outrate < 3) &&
> > (outrate/inrate > 24 || inrate/outrate > 8)) {
>
> Good catch. The range should be [8KHz, 30KHz] vs. (8KHz, 32KHz) in the
> code. Then I think the fix should be at both lines:
>
> - if ((outrate > 8000 && outrate < 3) &&
> - (outrate/inrate > 24 || inrate/outrate > 8)) {
> + if ((outrate >= 8000 && outrate =< 3) &&
> + (outrate > 24 * inrate || inrate > 8 * outrate)) {
>
> Overall, I think we should fix this instead of adding an extra one, since it
> is
> very likely saying the same thing.
Actually if outrate < 8kHz, there will be issue too.
Best regards
Wang shengjiu
Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
Hi > > > On Sat, Apr 20, 2019 at 09:12:52AM +, Daniel Baluta wrote: > > From: Shengjiu Wang > > > > SAI has 4 clock sources, which can be selected using MSEL bit of SAI > > TCR2 register. > > I have a doubt at this statement. As far as I can understand, this MSEL is > probably used by its internal clock MUX, so it's not really proving that SAI > has 4 MCLK inputs. What I know is that SAI block itself only has 3 MCLK > inputs as we defined in DT. It's just internally connects bus clock or MCLK1 > to input0 of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding an > MCLK0 here doesn't sound a right way to me. Unless someone can justify > for it, I think we should just fix it from driver side. > > Thanks > Nicolin > The MSEL bit width is 2 bit, so there is 4 options, the MCLK0 maybe the same input as MCLK1 or bus clock as you said, so we think may be better to show this relation in DT, And this is DT's capability. Driver don't care about which clock connect to MCLK0, it only need to know there is 4 MCLK from DT. Best regards Wang shengjiu
[PATCH RESEND V6 1/3] ASoC: fsl_asrc: Fix the issue about unsupported rate
When the output sample rate is [8kHz, 30kHz], the limitation
of the supported ratio range is [1/24, 8]. In the driver
we use (8kHz, 30kHz) instead of [8kHz, 30kHz].
So this patch is to fix this issue and the potential rounding
issue with divider.
Fixes: fff6e03c7b65 ("ASoC: fsl_asrc: add support for 8-30kHz
output sample rate")
Cc:
Signed-off-by: Shengjiu Wang
Acked-by: Nicolin Chen
---
sound/soc/fsl/fsl_asrc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 0b937924d2e4..ea035c12a325 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -282,8 +282,8 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
return -EINVAL;
}
- if ((outrate > 8000 && outrate < 3) &&
- (outrate/inrate > 24 || inrate/outrate > 8)) {
+ if ((outrate >= 8000 && outrate <= 3) &&
+ (outrate > 24 * inrate || inrate > 8 * outrate)) {
pair_err("exceed supported ratio range [1/24, 8] for \
inrate/outrate: %d/%d\n", inrate, outrate);
return -EINVAL;
--
2.21.0
[PATCH RESEND V6 0/3] Support more sample rate in asrc
Support more sample rate in asrc Shengjiu Wang (3): ASoC: fsl_asrc: Fix the issue about unsupported rate ASoC: fsl_asrc: replace the process_option table with function ASoC: fsl_asrc: Unify the supported input and output rate Changes in RESEND V6 - change the Content-Transfer-Encoding to "quoted-printable", for - "base64" can't be applied Changes in v6 - add acked-by - fixed minor issue according to comments in v5 Changes in v5 - fix the [1/24, 8] - move fsl_asrc_sel_proc before setting Changes in v4 - add patch to Fix the [8kHz, 30kHz] open set issue. Changes in v3 - remove FSL_ASRC_RATES - refine fsl_asrc_sel_proc according to comments Changes in v2 - add more comments in code - add commit "Unify the supported input and output rate" sound/soc/fsl/fsl_asrc.c | 105 ++- 1 file changed, 71 insertions(+), 34 deletions(-) -- 2.21.0
[PATCH RESEND V6 3/3] ASoC: fsl_asrc: Unify the supported input and output rate
Unify the supported input and output rate, add the
12kHz/24kHz/128kHz to the support list
Signed-off-by: Shengjiu Wang
Acked-by: Nicolin Chen
---
sound/soc/fsl/fsl_asrc.c | 32 +++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index a8d6710f2541..cbbf6257f08a 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -27,13 +27,14 @@
dev_dbg(_priv->pdev->dev, "Pair %c: " fmt, 'A' + index,
##__VA_ARGS__)
/* Corresponding to process_option */
-static int supported_input_rate[] = {
- 5512, 8000, 11025, 16000, 22050, 32000, 44100, 48000, 64000, 88200,
- 96000, 176400, 192000,
+static unsigned int supported_asrc_rate[] = {
+ 5512, 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000,
+ 64000, 88200, 96000, 128000, 176400, 192000,
};
-static int supported_asrc_rate[] = {
- 8000, 11025, 16000, 22050, 32000, 44100, 48000, 64000, 88200, 96000,
176400, 192000,
+static struct snd_pcm_hw_constraint_list fsl_asrc_rate_constraints = {
+ .count = ARRAY_SIZE(supported_asrc_rate),
+ .list = supported_asrc_rate,
};
/**
@@ -293,11 +294,11 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
*pair)
ideal = config->inclk == INCLK_NONE;
/* Validate input and output sample rates */
- for (in = 0; in < ARRAY_SIZE(supported_input_rate); in++)
- if (inrate == supported_input_rate[in])
+ for (in = 0; in < ARRAY_SIZE(supported_asrc_rate); in++)
+ if (inrate == supported_asrc_rate[in])
break;
- if (in == ARRAY_SIZE(supported_input_rate)) {
+ if (in == ARRAY_SIZE(supported_asrc_rate)) {
pair_err("unsupported input sample rate: %dHz\n", inrate);
return -EINVAL;
}
@@ -311,7 +312,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
return -EINVAL;
}
- if ((outrate >= 8000 && outrate <= 3) &&
+ if ((outrate >= 5512 && outrate <= 3) &&
(outrate > 24 * inrate || inrate > 8 * outrate)) {
pair_err("exceed supported ratio range [1/24, 8] for \
inrate/outrate: %d/%d\n", inrate, outrate);
@@ -486,7 +487,9 @@ static int fsl_asrc_dai_startup(struct snd_pcm_substream
*substream,
snd_pcm_hw_constraint_step(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_CHANNELS, 2);
- return 0;
+
+ return snd_pcm_hw_constraint_list(substream->runtime, 0,
+ SNDRV_PCM_HW_PARAM_RATE, _asrc_rate_constraints);
}
static int fsl_asrc_dai_hw_params(struct snd_pcm_substream *substream,
@@ -599,7 +602,6 @@ static int fsl_asrc_dai_probe(struct snd_soc_dai *dai)
return 0;
}
-#define FSL_ASRC_RATES SNDRV_PCM_RATE_8000_192000
#define FSL_ASRC_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | \
SNDRV_PCM_FMTBIT_S16_LE | \
SNDRV_PCM_FMTBIT_S20_3LE)
@@ -610,14 +612,18 @@ static struct snd_soc_dai_driver fsl_asrc_dai = {
.stream_name = "ASRC-Playback",
.channels_min = 1,
.channels_max = 10,
- .rates = FSL_ASRC_RATES,
+ .rate_min = 5512,
+ .rate_max = 192000,
+ .rates = SNDRV_PCM_RATE_KNOT,
.formats = FSL_ASRC_FORMATS,
},
.capture = {
.stream_name = "ASRC-Capture",
.channels_min = 1,
.channels_max = 10,
- .rates = FSL_ASRC_RATES,
+ .rate_min = 5512,
+ .rate_max = 192000,
+ .rates = SNDRV_PCM_RATE_KNOT,
.formats = FSL_ASRC_FORMATS,
},
.ops = _asrc_dai_ops,
--
2.21.0
[PATCH RESEND V6 2/3] ASoC: fsl_asrc: replace the process_option table with function
When we want to support more sample rate, for example 12kHz/24kHz
we need update the process_option table, if we want to support more
sample rate next time, the table need to be updated again. which
is not flexible.
We got a function fsl_asrc_sel_proc to replace the table, which can
give the pre-processing and post-processing options according to
the sample rate.
Signed-off-by: Shengjiu Wang
Acked-by: Nicolin Chen
---
sound/soc/fsl/fsl_asrc.c | 71 +---
1 file changed, 51 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index ea035c12a325..a8d6710f2541 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -26,24 +26,6 @@
#define pair_dbg(fmt, ...) \
dev_dbg(_priv->pdev->dev, "Pair %c: " fmt, 'A' + index,
##__VA_ARGS__)
-/* Sample rates are aligned with that defined in pcm.h file */
-static const u8 process_option[][12][2] = {
- /* 8kHz 11.025kHz 16kHz 22.05kHz 32kHz 44.1kHz 48kHz 64kHz 88.2kHz
96kHz 176kHz 192kHz */
- {{0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0,
0}, {0, 0}, {0, 0}, {0, 0},}, /* 5512Hz */
- {{0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0,
0}, {0, 0}, {0, 0}, {0, 0},}, /* 8kHz */
- {{0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {0,
0}, {0, 0}, {0, 0}, {0, 0},}, /* 11025Hz */
- {{1, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0,
0}, {0, 0}, {0, 0}, {0, 0},}, /* 16kHz */
- {{1, 2}, {1, 2}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0, 0}, {0,
0}, {0, 0}, {0, 0}, {0, 0},}, /* 22050Hz */
- {{1, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0, 1}, {0,
1}, {0, 0}, {0, 0}, {0, 0},}, /* 32kHz */
- {{2, 2}, {2, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 1}, {0, 1}, {0, 1}, {0,
1}, {0, 1}, {0, 0}, {0, 0},}, /* 44.1kHz */
- {{2, 2}, {2, 2}, {2, 1}, {2, 1}, {0, 2}, {0, 2}, {0, 1}, {0, 1}, {0,
1}, {0, 1}, {0, 0}, {0, 0},}, /* 48kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 1}, {1, 2}, {0, 2}, {0, 2}, {0, 1}, {0,
1}, {0, 1}, {0, 1}, {0, 0},}, /* 64kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {1, 2}, {1, 2}, {1, 2}, {1, 1}, {1,
1}, {1, 1}, {1, 1}, {1, 1},}, /* 88.2kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {1, 2}, {1, 2}, {1, 2}, {1, 1}, {1,
1}, {1, 1}, {1, 1}, {1, 1},}, /* 96kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 1}, {2,
1}, {2, 1}, {2, 1}, {2, 1},}, /* 176kHz */
- {{2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 2}, {2, 1}, {2,
1}, {2, 1}, {2, 1}, {2, 1},}, /* 192kHz */
-};
-
/* Corresponding to process_option */
static int supported_input_rate[] = {
5512, 8000, 11025, 16000, 22050, 32000, 44100, 48000, 64000, 88200,
@@ -79,6 +61,52 @@ static unsigned char output_clk_map_imx53[] = {
static unsigned char *clk_map[2];
+/**
+ * Select the pre-processing and post-processing options
+ * Make sure to exclude following unsupported cases before
+ * calling this function:
+ * 1) inrate > 8.125 * outrate
+ * 2) inrate > 16.125 * outrate
+ *
+ * inrate: input sample rate
+ * outrate: output sample rate
+ * pre_proc: return value for pre-processing option
+ * post_proc: return value for post-processing option
+ */
+static void fsl_asrc_sel_proc(int inrate, int outrate,
+int *pre_proc, int *post_proc)
+{
+ bool post_proc_cond2;
+ bool post_proc_cond0;
+
+ /* select pre_proc between [0, 2] */
+ if (inrate * 8 > 33 * outrate)
+ *pre_proc = 2;
+ else if (inrate * 8 > 15 * outrate) {
+ if (inrate > 152000)
+ *pre_proc = 2;
+ else
+ *pre_proc = 1;
+ } else if (inrate < 76000)
+ *pre_proc = 0;
+ else if (inrate > 152000)
+ *pre_proc = 2;
+ else
+ *pre_proc = 1;
+
+ /* Condition for selection of post-processing */
+ post_proc_cond2 = (inrate * 15 > outrate * 16 && outrate < 56000) ||
+ (inrate > 56000 && outrate < 56000);
+ post_proc_cond0 = inrate * 23 < outrate * 8;
+
+ if (post_proc_cond2)
+ *post_proc = 2;
+ else if (post_proc_cond0)
+ *post_proc = 0;
+ else
+ *post_proc = 1;
+}
+
/**
* Request ASRC pair
*
@@ -239,6 +267,7 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)
u32 inrate, outrate, indiv, outdiv;
u32 clk_index[2], div[2];
int in, out, channels;
+ int pre_proc, post_proc;
struct clk *clk;
bool ideal;
@@ -377,11 +406,13 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair
*pair)
ASRCTR_IDRi_MASK(index) | ASRCTR_USRi_MASK(index),
ASRCTR_IDR(index) | ASRCTR_USR(index));
+
[PATCH] mfd: imx6sx: add MQS register definition for iomuxc gpr
Add macros to define masks and bits for imx6sx MQS registers Signed-off-by: Shengjiu Wang --- include/linux/mfd/syscon/imx6q-iomuxc-gpr.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h index c1b25f5e386d..f232c8130d00 100644 --- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h +++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h @@ -410,6 +410,15 @@ #define IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK (0x3 << 17) #define IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_EXT (0x3 << 13) +#define IMX6SX_GPR2_MQS_OVERSAMPLE_MASK(0x1 << 26) +#define IMX6SX_GPR2_MQS_OVERSAMPLE_SHIFT (26) +#define IMX6SX_GPR2_MQS_EN_MASK(0x1 << 25) +#define IMX6SX_GPR2_MQS_EN_SHIFT (25) +#define IMX6SX_GPR2_MQS_SW_RST_MASK(0x1 << 24) +#define IMX6SX_GPR2_MQS_SW_RST_SHIFT (24) +#define IMX6SX_GPR2_MQS_CLK_DIV_MASK (0xFF << 16) +#define IMX6SX_GPR2_MQS_CLK_DIV_SHIFT (16) + #define IMX6SX_GPR4_FEC_ENET1_STOP_REQ (0x1 << 3) #define IMX6SX_GPR4_FEC_ENET2_STOP_REQ (0x1 << 4) -- 1.9.1
[PATCH] ASoC: cs42xx8: Add reset gpio handling
Handle the reset GPIO and reset the device in
pm_runtime_resume
Signed-off-by: Shengjiu Wang
---
sound/soc/codecs/cs42xx8.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
index ebb9e0cf8364..fc28e6d26c6d 100644
--- a/sound/soc/codecs/cs42xx8.c
+++ b/sound/soc/codecs/cs42xx8.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -45,6 +46,7 @@ struct cs42xx8_priv {
bool slave_mode;
unsigned long sysclk;
u32 tx_channels;
+ int gpio_reset;
};
/* -127.5dB to 0dB with step of 0.5dB */
@@ -467,6 +469,17 @@ int cs42xx8_probe(struct device *dev, struct regmap
*regmap)
return -EINVAL;
}
+ cs42xx8->gpio_reset = of_get_named_gpio(dev->of_node, "gpio-reset", 0);
+ if (gpio_is_valid(cs42xx8->gpio_reset)) {
+ ret = devm_gpio_request_one(dev, cs42xx8->gpio_reset,
+ GPIOF_OUT_INIT_LOW, "cs42xx8 reset");
+ if (ret) {
+ dev_err(dev, "unable to get reset gpio\n");
+ return ret;
+ }
+ gpio_set_value_cansleep(cs42xx8->gpio_reset, 1);
+ }
+
cs42xx8->clk = devm_clk_get(dev, "mclk");
if (IS_ERR(cs42xx8->clk)) {
dev_err(dev, "failed to get the clock: %ld\n",
@@ -547,6 +560,11 @@ static int cs42xx8_runtime_resume(struct device *dev)
return ret;
}
+ if (gpio_is_valid(cs42xx8->gpio_reset)) {
+ gpio_set_value_cansleep(cs42xx8->gpio_reset, 0);
+ gpio_set_value_cansleep(cs42xx8->gpio_reset, 1);
+ }
+
ret = regulator_bulk_enable(ARRAY_SIZE(cs42xx8->supplies),
cs42xx8->supplies);
if (ret) {
@@ -559,6 +577,7 @@ static int cs42xx8_runtime_resume(struct device *dev)
regcache_cache_only(cs42xx8->regmap, false);
+ regcache_mark_dirty(cs42xx8->regmap);
ret = regcache_sync(cs42xx8->regmap);
if (ret) {
dev_err(dev, "failed to sync regmap: %d\n", ret);
--
1.9.1
Re: [PATCH] ASoC: cs42xx8: Add reset gpio handling
Hi Mark
>
> On Mon, Apr 29, 2019 at 10:46:03AM +0000, S.j. Wang wrote:
>
> > + cs42xx8->gpio_reset = of_get_named_gpio(dev->of_node, "gpio-
> reset", 0);
> > + if (gpio_is_valid(cs42xx8->gpio_reset)) {
> > + ret = devm_gpio_request_one(dev, cs42xx8->gpio_reset,
> > + GPIOF_OUT_INIT_LOW, "cs42xx8 reset");
>
> You should just be able to request the GPIO by name without going
> through of_get_named_gpio() using devm_gpio_get().
>
Will send v2.
> > @@ -559,6 +577,7 @@ static int cs42xx8_runtime_resume(struct device
> > *dev)
> >
> > regcache_cache_only(cs42xx8->regmap, false);
> >
> > + regcache_mark_dirty(cs42xx8->regmap);
> > ret = regcache_sync(cs42xx8->regmap);
> > if (ret) {
> > dev_err(dev, "failed to sync regmap: %d\n", ret);
>
> This looks like an unrelated bugfix.
Will separate it to another patch.
Best regards
Wang shengjiu
[PATCH] ASoC: cs42xx8: Add regcache mask dirty
Add regcache_mark_dirty before regcache_sync for power
of codec may be lost at suspend, then all the register
need to be reconfigured.
Fixes: 0c516b4ff85c ("ASoC: cs42xx8: Add codec driver
support for CS42448/CS42888")
Cc:
Signed-off-by: Shengjiu Wang
---
sound/soc/codecs/cs42xx8.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
index ebb9e0cf8364..28a4ac36c4f8 100644
--- a/sound/soc/codecs/cs42xx8.c
+++ b/sound/soc/codecs/cs42xx8.c
@@ -558,6 +558,7 @@ static int cs42xx8_runtime_resume(struct device *dev)
msleep(5);
regcache_cache_only(cs42xx8->regmap, false);
+ regcache_mark_dirty(cs42xx8->regmap);
ret = regcache_sync(cs42xx8->regmap);
if (ret) {
--
2.21.0
[PATCH V2] ASoC: cs42xx8: Add reset gpio handling
Handle the reset GPIO and reset the device every time we
start it.
Signed-off-by: Shengjiu Wang
---
Changes in v2
- use devm_gpiod_get_optional instead of of_get_named_gpio
sound/soc/codecs/cs42xx8.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
index 28a4ac36c4f8..b377cddaf2e6 100644
--- a/sound/soc/codecs/cs42xx8.c
+++ b/sound/soc/codecs/cs42xx8.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -45,6 +46,7 @@ struct cs42xx8_priv {
bool slave_mode;
unsigned long sysclk;
u32 tx_channels;
+ struct gpio_desc *gpiod_reset;
};
/* -127.5dB to 0dB with step of 0.5dB */
@@ -467,6 +469,13 @@ int cs42xx8_probe(struct device *dev, struct regmap
*regmap)
return -EINVAL;
}
+ cs42xx8->gpiod_reset = devm_gpiod_get_optional(dev, "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(cs42xx8->gpiod_reset))
+ return PTR_ERR(cs42xx8->gpiod_reset);
+
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 0);
+
cs42xx8->clk = devm_clk_get(dev, "mclk");
if (IS_ERR(cs42xx8->clk)) {
dev_err(dev, "failed to get the clock: %ld\n",
@@ -547,6 +556,8 @@ static int cs42xx8_runtime_resume(struct device *dev)
return ret;
}
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 0);
+
ret = regulator_bulk_enable(ARRAY_SIZE(cs42xx8->supplies),
cs42xx8->supplies);
if (ret) {
@@ -586,6 +597,8 @@ static int cs42xx8_runtime_suspend(struct device *dev)
regulator_bulk_disable(ARRAY_SIZE(cs42xx8->supplies),
cs42xx8->supplies);
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 1);
+
clk_disable_unprepare(cs42xx8->clk);
return 0;
--
2.21.0
Re: [PATCH V2] ASoC: cs42xx8: Add reset gpio handling
Hi > > On Thu, May 16, 2019 at 06:04:58AM +0000, S.j. Wang wrote: > > > + cs42xx8->gpiod_reset = devm_gpiod_get_optional(dev, "reset", > > + GPIOD_OUT_HIGH); > > + if (IS_ERR(cs42xx8->gpiod_reset)) > > + return PTR_ERR(cs42xx8->gpiod_reset); > > You also need a binding document update for this. ok, will send v3 best regards wang shengjiu
[PATCH V3] ASoC: cs42xx8: Add reset gpio handling
Handle the reset GPIO and reset the device every time we
start it.
Signed-off-by: Shengjiu Wang
---
Changes in v3
- update binding document.
Changes in v2
- use devm_gpiod_get_optional instead of of_get_named_gpio
Documentation/devicetree/bindings/sound/cs42xx8.txt | 6 ++
sound/soc/codecs/cs42xx8.c | 13 +
2 files changed, 19 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/cs42xx8.txt
b/Documentation/devicetree/bindings/sound/cs42xx8.txt
index 8619a156d038..ab8f54095269 100644
--- a/Documentation/devicetree/bindings/sound/cs42xx8.txt
+++ b/Documentation/devicetree/bindings/sound/cs42xx8.txt
@@ -14,6 +14,11 @@ Required properties:
- VA-supply, VD-supply, VLS-supply, VLC-supply: power supplies for the
device,
as covered in Documentation/devicetree/bindings/regulator/regulator.txt
+Optional properties:
+
+ - reset-gpio : a GPIO spec to define which pin is connected to the chip's
+!RESET pin
+
Example:
cs42888: codec@48 {
@@ -25,4 +30,5 @@ cs42888: codec@48 {
VD-supply = <_audio>;
VLS-supply = <_audio>;
VLC-supply = <_audio>;
+ reset-gpio = <_b 1 1>;
};
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
index 28a4ac36c4f8..b377cddaf2e6 100644
--- a/sound/soc/codecs/cs42xx8.c
+++ b/sound/soc/codecs/cs42xx8.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -45,6 +46,7 @@ struct cs42xx8_priv {
bool slave_mode;
unsigned long sysclk;
u32 tx_channels;
+ struct gpio_desc *gpiod_reset;
};
/* -127.5dB to 0dB with step of 0.5dB */
@@ -467,6 +469,13 @@ int cs42xx8_probe(struct device *dev, struct regmap
*regmap)
return -EINVAL;
}
+ cs42xx8->gpiod_reset = devm_gpiod_get_optional(dev, "reset",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(cs42xx8->gpiod_reset))
+ return PTR_ERR(cs42xx8->gpiod_reset);
+
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 0);
+
cs42xx8->clk = devm_clk_get(dev, "mclk");
if (IS_ERR(cs42xx8->clk)) {
dev_err(dev, "failed to get the clock: %ld\n",
@@ -547,6 +556,8 @@ static int cs42xx8_runtime_resume(struct device *dev)
return ret;
}
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 0);
+
ret = regulator_bulk_enable(ARRAY_SIZE(cs42xx8->supplies),
cs42xx8->supplies);
if (ret) {
@@ -586,6 +597,8 @@ static int cs42xx8_runtime_suspend(struct device *dev)
regulator_bulk_disable(ARRAY_SIZE(cs42xx8->supplies),
cs42xx8->supplies);
+ gpiod_set_value_cansleep(cs42xx8->gpiod_reset, 1);
+
clk_disable_unprepare(cs42xx8->clk);
return 0;
--
2.21.0
RE: [EXT] Re: [PATCH V2] ASoC: cs42xx8: Add reset gpio handling
Hi > > On Thu, May 16, 2019 at 11:09:27AM +0000, S.j. Wang wrote: > > > > You also need a binding document update for this. > > ok, will send v3 > > Separate patch please, I already applied this and binding docs should be > separate patches anyway. Ok, I already send v3. please forget the v3, I will resend v3 for binding doc only. Best regards Wang shengjiu
[PATCH RESEND V3] ASoC: cs42xx8: add reset-gpio in binding document
Add reset-gpio property, which is an optional option
Signed-off-by: Shengjiu Wang
---
Changes in RESEND v3
- send updated binding document only
Changes in v3
- update binding document.
Changes in v2
- use devm_gpiod_get_optional instead of of_get_named_gpio
Documentation/devicetree/bindings/sound/cs42xx8.txt | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/cs42xx8.txt
b/Documentation/devicetree/bindings/sound/cs42xx8.txt
index 8619a156d038..ab8f54095269 100644
--- a/Documentation/devicetree/bindings/sound/cs42xx8.txt
+++ b/Documentation/devicetree/bindings/sound/cs42xx8.txt
@@ -14,6 +14,11 @@ Required properties:
- VA-supply, VD-supply, VLS-supply, VLC-supply: power supplies for the
device,
as covered in Documentation/devicetree/bindings/regulator/regulator.txt
+Optional properties:
+
+ - reset-gpio : a GPIO spec to define which pin is connected to the chip's
+!RESET pin
+
Example:
cs42888: codec@48 {
@@ -25,4 +30,5 @@ cs42888: codec@48 {
VD-supply = <_audio>;
VLS-supply = <_audio>;
VLC-supply = <_audio>;
+ reset-gpio = <_b 1 1>;
};
--
2.21.0
RE: [EXT] Re: [PATCH RESEND V3] ASoC: cs42xx8: add reset-gpio in binding document
Hi > > > +Optional properties: > > + > > + - reset-gpio : a GPIO spec to define which pin is connected to the chip's > > +!RESET pin > > gpio properties are supposed to be called -gpios even if there's a single > GPIO possible due to DT rules. The code will accept plain -gpio but the > documentation should say gpios. Ok, will update in v4 Best regards Wang shengjiu
[PATCH V4] ASoC: cs42xx8: add reset-gpios in binding document
Add reset-gpios property, which is optional.
Signed-off-by: Shengjiu Wang
---
Changes in V4
- use gpios instead of gpio
Changes in RESEND v3
- send updated binding document only
Changes in v3
- update binding document.
Changes in v2
- use devm_gpiod_get_optional instead of of_get_named_gpio
Documentation/devicetree/bindings/sound/cs42xx8.txt | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/cs42xx8.txt
b/Documentation/devicetree/bindings/sound/cs42xx8.txt
index 8619a156d038..0138ef559cc4 100644
--- a/Documentation/devicetree/bindings/sound/cs42xx8.txt
+++ b/Documentation/devicetree/bindings/sound/cs42xx8.txt
@@ -14,6 +14,11 @@ Required properties:
- VA-supply, VD-supply, VLS-supply, VLC-supply: power supplies for the
device,
as covered in Documentation/devicetree/bindings/regulator/regulator.txt
+Optional properties:
+
+ - reset-gpios : a GPIO spec to define which pin is connected to the chip's
+!RESET pin
+
Example:
cs42888: codec@48 {
@@ -25,4 +30,5 @@ cs42888: codec@48 {
VD-supply = <_audio>;
VLS-supply = <_audio>;
VLC-supply = <_audio>;
+ reset-gpios = <_b 1 1>;
};
--
2.21.0
Re: [alsa-devel] [PATCH V4] ASoC: cs42xx8: add reset-gpios in binding document
Hi
> On Thu, May 16, 2019 at 8:36 AM S.j. Wang
> wrote:
>
> > cs42888: codec@48 {
> > @@ -25,4 +30,5 @@ cs42888: codec@48 {
> > VD-supply = <_audio>;
> > VLS-supply = <_audio>;
> > VLC-supply = <_audio>;
> > + reset-gpios = <_b 1 1>;
>
> reset-gpios = <_b 1 GPIO_ACTIVE_LOW>; please
Ok, sent v5.
Best regards
Wang shengjiu
[PATCH V5] ASoC: cs42xx8: add reset-gpios in binding document
Add reset-gpios property, which is optional.
Signed-off-by: Shengjiu Wang
---
Changes in V5
- use GPIO_ACTIVE_LOW
Changes in V4
- use gpios instead of gpio
Changes in RESEND v3
- send updated binding document only
Changes in v3
- update binding document.
Changes in v2
- use devm_gpiod_get_optional instead of of_get_named_gpio
Documentation/devicetree/bindings/sound/cs42xx8.txt | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/cs42xx8.txt
b/Documentation/devicetree/bindings/sound/cs42xx8.txt
index 8619a156d038..bbfe39347c20 100644
--- a/Documentation/devicetree/bindings/sound/cs42xx8.txt
+++ b/Documentation/devicetree/bindings/sound/cs42xx8.txt
@@ -14,6 +14,11 @@ Required properties:
- VA-supply, VD-supply, VLS-supply, VLC-supply: power supplies for the
device,
as covered in Documentation/devicetree/bindings/regulator/regulator.txt
+Optional properties:
+
+ - reset-gpios : a GPIO spec to define which pin is connected to the chip's
+!RESET pin
+
Example:
cs42888: codec@48 {
@@ -25,4 +30,5 @@ cs42888: codec@48 {
VD-supply = <_audio>;
VLS-supply = <_audio>;
VLC-supply = <_audio>;
+ reset-gpios = <_b 1 GPIO_ACTIVE_LOW>;
};
--
2.21.0
RE: linux-next: Fixes tag needs some work in the sound-asoc tree
Hi
Do I need to resend the patch?
Best regards
Wang shengjiu
>
> In commit
>
> b06c58c2a1ee ("ASoC: fsl_asrc: Fix the issue about unsupported rate")
>
> Fixes tag
>
> Fixes: fff6e03c7b65 ("ASoC: fsl_asrc: add support for 8-30kHz
>
> has these problem(s):
>
> - Subject has leading but no trailing parentheses
> - Subject has leading but no trailing quotes
>
> Please do not split fixes tags over more that one line.
>
> --
> Cheers,
> Stephen Rothwell
[PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
There is chip errata ERR008000, the reference doc is
(https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf),
The issue is "While using ESAI transmit or receive and
an underrun/overrun happens, channel swap may occur.
The only recovery mechanism is to reset the ESAI."
In this commit add a tasklet to handle reset of ESAI
after xrun happens
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_esai.c | 166 +++
1 file changed, 166 insertions(+)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 10d2210c91ef..149972894c95 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -52,17 +52,20 @@ struct fsl_esai {
struct clk *extalclk;
struct clk *fsysclk;
struct clk *spbaclk;
+ struct tasklet_struct task;
u32 fifo_depth;
u32 slot_width;
u32 slots;
u32 tx_mask;
u32 rx_mask;
+ u32 tx_channels;
u32 hck_rate[2];
u32 sck_rate[2];
bool hck_dir[2];
bool sck_div[2];
bool slave_mode;
bool synchronous;
+ bool reset_at_xrun;
char name[32];
};
@@ -71,8 +74,14 @@ static irqreturn_t esai_isr(int irq, void *devid)
struct fsl_esai *esai_priv = (struct fsl_esai *)devid;
struct platform_device *pdev = esai_priv->pdev;
u32 esr;
+ u32 saisr;
regmap_read(esai_priv->regmap, REG_ESAI_ESR, );
+ regmap_read(esai_priv->regmap, REG_ESAI_SAISR, );
+
+ if ((saisr & (ESAI_SAISR_TUE | ESAI_SAISR_ROE))
+ && esai_priv->reset_at_xrun)
+ tasklet_schedule(_priv->task);
if (esr & ESAI_ESR_TINIT_MASK)
dev_dbg(>dev, "isr: Transmission Initialized\n");
@@ -552,6 +561,9 @@ static int fsl_esai_trigger(struct snd_pcm_substream
*substream, int cmd,
u32 pins = DIV_ROUND_UP(channels, esai_priv->slots);
u32 mask;
+ if (tx)
+ esai_priv->tx_channels = channels;
+
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
@@ -585,10 +597,16 @@ static int fsl_esai_trigger(struct snd_pcm_substream
*substream, int cmd,
regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx),
ESAI_xSMA_xS_MASK, ESAI_xSMA_xS(mask));
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
+ ESAI_xCR_xEIE_MASK, ESAI_xCR_xEIE);
+
break;
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
+ ESAI_xCR_xEIE_MASK, 0);
+
regmap_update_bits(esai_priv->regmap, REG_ESAI_xCR(tx),
tx ? ESAI_xCR_TE_MASK : ESAI_xCR_RE_MASK, 0);
regmap_update_bits(esai_priv->regmap, REG_ESAI_xSMA(tx),
@@ -618,6 +636,145 @@ static const struct snd_soc_dai_ops fsl_esai_dai_ops = {
.set_tdm_slot = fsl_esai_set_dai_tdm_slot,
};
+static void fsl_esai_reset(unsigned long arg)
+{
+ struct fsl_esai *esai_priv = (struct fsl_esai *)arg;
+ u32 saisr;
+ u32 tsma, tsmb, rsma, rsmb, tcr, rcr, tfcr, rfcr;
+ int i;
+
+ /*
+* stop the tx & rx
+*/
+ regmap_read(esai_priv->regmap, REG_ESAI_TSMA, );
+ regmap_read(esai_priv->regmap, REG_ESAI_TSMB, );
+ regmap_read(esai_priv->regmap, REG_ESAI_RSMA, );
+ regmap_read(esai_priv->regmap, REG_ESAI_RSMB, );
+
+ regmap_read(esai_priv->regmap, REG_ESAI_TCR, );
+ regmap_read(esai_priv->regmap, REG_ESAI_RCR, );
+
+ regmap_read(esai_priv->regmap, REG_ESAI_TFCR, );
+ regmap_read(esai_priv->regmap, REG_ESAI_RFCR, );
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
+ ESAI_xCR_xEIE_MASK | ESAI_xCR_TE_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR,
+ ESAI_xCR_xEIE_MASK | ESAI_xCR_RE_MASK, 0);
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TSMA,
+ ESAI_xSMA_xS_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TSMB,
+ ESAI_xSMB_xS_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_RSMA,
+ ESAI_xSMA_xS_MASK, 0);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_RSMB,
+ ESAI_xSMB_xS_MASK, 0);
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TFCR,
+ ESAI_xFCR_xFR | ESAI_xFCR_xFEN, ESAI_xFCR_xFR);
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_TFCR,
+ ESAI_xFCR_xFR, 0);
+
+ regmap_update_bits(esai_priv->regmap, REG_ESAI_RFCR,
+ ESAI_xFCR_xFR | ESAI_xFCR_xFEN, ESAI_xFCR_xFR);
+
RE: [alsa-devel] [PATCH] ASoC: cs42xx8: Add regcache mask dirty
Hi
> Add regcache_mark_dirty before regcache_sync for power of codec may be
> lost at suspend, then all the register need to be reconfigured.
>
> Fixes: 0c516b4ff85c ("ASoC: cs42xx8: Add codec driver support for
> CS42448/CS42888")
The Fixes tag is split, will send v2.
Best regards
Wang shengjiu
[PATCH V2] ASoC: cs42xx8: Add regcache mask dirty
Add regcache_mark_dirty before regcache_sync for power
of codec may be lost at suspend, then all the register
need to be reconfigured.
Fixes: 0c516b4ff85c ("ASoC: cs42xx8: Add codec driver support for
CS42448/CS42888")
Cc:
Signed-off-by: Shengjiu Wang
---
Changs in V2
- Don't split Fixes tag.
sound/soc/codecs/cs42xx8.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
index ebb9e0cf8364..28a4ac36c4f8 100644
--- a/sound/soc/codecs/cs42xx8.c
+++ b/sound/soc/codecs/cs42xx8.c
@@ -558,6 +558,7 @@ static int cs42xx8_runtime_resume(struct device *dev)
msleep(5);
regcache_cache_only(cs42xx8->regmap, false);
+ regcache_mark_dirty(cs42xx8->regmap);
ret = regcache_sync(cs42xx8->regmap);
if (ret) {
--
2.21.0
[PATCH V5] ASoC: fsl_esai: Fix missing break in switch statement
case ESAI_HCKT_EXTAL and case ESAI_HCKR_EXTAL should be
independent of each other, so replace fall-through with break.
Fixes: 43d24e76b698 ("ASoC: fsl_esai: Add ESAI CPU DAI driver")
Signed-off-by: Shengjiu Wang
Acked-by: Nicolin Chen
Cc:
---
Changes in v5
- remove new line after Fixes
Changes in v4
- Add acked-by
Changes in v3
- Update subject line and cc stable
Changes in v2
- Fix "Fixes" tag
sound/soc/fsl/fsl_esai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 3623aa9a6f2e..15202a637197 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -251,7 +251,7 @@ static int fsl_esai_set_dai_sysclk(struct snd_soc_dai *dai,
int clk_id,
break;
case ESAI_HCKT_EXTAL:
ecr |= ESAI_ECR_ETI;
- /* fall through */
+ break;
case ESAI_HCKR_EXTAL:
ecr |= ESAI_ECR_ERI;
break;
--
1.9.1
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
Hi
>
> On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > Error log:
> > > sysfs: cannot create duplicate filename
> '/bus/platform/devices/3000.bus'
> > >
> > > The spba bus name is duplicate with aips bus name.
> > > Refine spba bus name to fix this issue.
> > >
> > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous Sample
> > > Rate Converter")
> > > Signed-off-by: Shengjiu Wang
> > > ---
> > > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > #size-cells = <1>;
> > > ranges;
> > >
> > > - spba: bus@3000 {
> > > + spba: spba-bus@3000 {
> >
> > The proper node name is "bus" so basically you introduce wrong name to
> > other problem. Introducing wrong names at least requires a comment.
>
> I just noticed that my message was barely understandable... so let me fix it:
>
> The proper node name is "bus" so basically you introduce wrong name to
> _fix_ other problem. Introducing wrong names at least requires a comment.
>
> > However the actual problem here is not in node names but in addresses:
> >
> > aips1: bus@3000 {
> > spba: bus@3000 {
> >
> > You have to devices with the same unit address. How do you share the
> > address space?
> >
> > I think this should be rather fixed.
>
> And again, hungry keyboard ate a letter, so:
>
> You have _two_ devices with the same unit address. How do you share the
> address space?
> I think this should be rather fixed.
>
spba is the first block of aips1 space, so it has same start address as
aips1.
Best regards
Wang shengjiu
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
> > > >
> > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski wrote:
> > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > Error log:
> > > > > > sysfs: cannot create duplicate filename
> > > > '/bus/platform/devices/3000.bus'
> > > > > >
> > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > Refine spba bus name to fix this issue.
> > > > > >
> > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable Asynchronous
> > > > > > Sample Rate Converter")
> > > > > > Signed-off-by: Shengjiu Wang
> > > > > > ---
> > > > > > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > > > > #size-cells = <1>;
> > > > > > ranges;
> > > > > >
> > > > > > - spba: bus@3000 {
> > > > > > + spba: spba-bus@3000 {
> > > > >
> > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > name to other problem. Introducing wrong names at least requires a
> comment.
> > > >
> > > > I just noticed that my message was barely understandable... so let me
> fix it:
> > > >
> > > > The proper node name is "bus" so basically you introduce wrong
> > > > name to _fix_ other problem. Introducing wrong names at least
> requires a comment.
> > > >
> > > > > However the actual problem here is not in node names but in
> addresses:
> > > > >
> > > > > aips1: bus@3000 {
> > > > > spba: bus@3000 {
> > > > >
> > > > > You have to devices with the same unit address. How do you share
> > > > > the address space?
> > > > >
> > > > > I think this should be rather fixed.
> > > >
> > > > And again, hungry keyboard ate a letter, so:
> > > >
> > > > You have _two_ devices with the same unit address. How do you
> > > > share the address space?
> > > > I think this should be rather fixed.
> > > >
> > >
> > > spba is the first block of aips1 space, so it has same start address
> > > as aips1.
> >
> > The reference manual describes it "Reserved for SDMA2 internal
> > memory", so indeed it is first address but does it have to be mapped?
> > Anyway, why don't you use ranges to remove the conflict?
>
> The IO address space remapping could be a solution but there is another
> problem - the hardware representation in DT does not match what reference
> manual is saying.
>
> The AIPS bus @3000 has several IPs:
> - SAI2@3002
> - ...
> - GPIO1@3020
>
> However in DTS you will find additional SPBA bus for 3000 - 300c.
> It's not really the SDMA, as SDMA is at different address. It is rather an
> address space which SDMA should map... but it is not a bus with children.
> Adding spba-bus@3000 with its children does not look like correct
> representation of HW in DTS.
>
In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
Range is (3000 - 300F)
SPBA is a sub-bus under AIPS1. The SAI2@3002 - ASRC@300c
Are the devices under SPBA bus.
So it doesn't violate RM.
Best regards
Wang shengjiu
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
> On Tue, Dec 08, 2020 at 08:44:51AM +0000, S.j. Wang wrote:
> > > > > >
> > > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof Kozlowski
> wrote:
> > > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang wrote:
> > > > > > > > Error log:
> > > > > > > > sysfs: cannot create duplicate filename
> > > > > > '/bus/platform/devices/3000.bus'
> > > > > > > >
> > > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > > Refine spba bus name to fix this issue.
> > > > > > > >
> > > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable
> > > > > > > > Asynchronous Sample Rate Converter")
> > > > > > > > Signed-off-by: Shengjiu Wang
> > > > > > > > ---
> > > > > > > > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > > > > > > #size-cells = <1>;
> > > > > > > > ranges;
> > > > > > > >
> > > > > > > > - spba: bus@3000 {
> > > > > > > > + spba: spba-bus@3000 {
> > > > > > >
> > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > wrong name to other problem. Introducing wrong names at
> > > > > > > least requires a
> > > comment.
> > > > > >
> > > > > > I just noticed that my message was barely understandable... so
> > > > > > let me
> > > fix it:
> > > > > >
> > > > > > The proper node name is "bus" so basically you introduce wrong
> > > > > > name to _fix_ other problem. Introducing wrong names at least
> > > requires a comment.
> > > > > >
> > > > > > > However the actual problem here is not in node names but in
> > > addresses:
> > > > > > >
> > > > > > > aips1: bus@3000 {
> > > > > > > spba: bus@3000 {
> > > > > > >
> > > > > > > You have to devices with the same unit address. How do you
> > > > > > > share the address space?
> > > > > > >
> > > > > > > I think this should be rather fixed.
> > > > > >
> > > > > > And again, hungry keyboard ate a letter, so:
> > > > > >
> > > > > > You have _two_ devices with the same unit address. How do you
> > > > > > share the address space?
> > > > > > I think this should be rather fixed.
> > > > > >
> > > > >
> > > > > spba is the first block of aips1 space, so it has same start
> > > > > address as aips1.
> > > >
> > > > The reference manual describes it "Reserved for SDMA2 internal
> > > > memory", so indeed it is first address but does it have to be mapped?
> > > > Anyway, why don't you use ranges to remove the conflict?
> > >
> > > The IO address space remapping could be a solution but there is
> > > another problem - the hardware representation in DT does not match
> > > what reference manual is saying.
> > >
> > > The AIPS bus @3000 has several IPs:
> > > - SAI2@3002
> > > - ...
> > > - GPIO1@3020
> > >
> > > However in DTS you will find additional SPBA bus for 3000 -
> 300c.
> > > It's not really the SDMA, as SDMA is at different address. It is
> > > rather an address space which SDMA should map... but it is not a bus
> with children.
> > > Adding spba-bus@3000 with its children does not look like
> > > correct representation of HW in DTS.
> > >
> >
> > In the RM, it says AIPS-1 (s_b_1, via SPBA) Glob. Module Enable.
> > Range is (3000 - 300F)
>
> No, AIPS-1 is till 303F_.
Yes, AIPSA-1 is till 303F_, but it is divided to 2 parts.
(3000 - 300F) is the first part.
Please go to table 2-3 AIPS1 memory map in RM. In the "region" column,
There is " AIPS-1 (s_b_1, via SPBA) Glob. Module Enable". It means
This part is connect to SPBA bus.
Best regards
Wang Shengjiu
Re: [PATCH] arm64: dts: imx8mn: Fix duplicate node name
>
> On Tue, Dec 08, 2020 at 08:57:49AM +0000, S.j. Wang wrote:
> > > On Tue, Dec 08, 2020 at 08:44:51AM +, S.j. Wang wrote:
> > > > > > > >
> > > > > > > > On Mon, Dec 07, 2020 at 02:21:40PM +0100, Krzysztof
> > > > > > > > Kozlowski
> > > wrote:
> > > > > > > > > On Mon, Dec 07, 2020 at 02:53:24PM +0800, Shengjiu Wang
> wrote:
> > > > > > > > > > Error log:
> > > > > > > > > > sysfs: cannot create duplicate filename
> > > > > > > > '/bus/platform/devices/3000.bus'
> > > > > > > > > >
> > > > > > > > > > The spba bus name is duplicate with aips bus name.
> > > > > > > > > > Refine spba bus name to fix this issue.
> > > > > > > > > >
> > > > > > > > > > Fixes: 970406eaef3a ("arm64: dts: imx8mn: Enable
> > > > > > > > > > Asynchronous Sample Rate Converter")
> > > > > > > > > > Signed-off-by: Shengjiu Wang
> > > > > > > > > > ---
> > > > > > > > > > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
> > > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > index fd669c0f3fe5..30762eb4f0a7 100644
> > > > > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > > > > > > > > @@ -246,7 +246,7 @@ aips1: bus@3000 {
> > > > > > > > > > #size-cells = <1>;
> > > > > > > > > > ranges;
> > > > > > > > > >
> > > > > > > > > > - spba: bus@3000 {
> > > > > > > > > > + spba: spba-bus@3000 {
> > > > > > > > >
> > > > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > > > wrong name to other problem. Introducing wrong names at
> > > > > > > > > least requires a
> > > > > comment.
> > > > > > > >
> > > > > > > > I just noticed that my message was barely
> > > > > > > > understandable... so let me
> > > > > fix it:
> > > > > > > >
> > > > > > > > The proper node name is "bus" so basically you introduce
> > > > > > > > wrong name to _fix_ other problem. Introducing wrong
> > > > > > > > names at least
> > > > > requires a comment.
> > > > > > > >
> > > > > > > > > However the actual problem here is not in node names but
> > > > > > > > > in
> > > > > addresses:
> > > > > > > > >
> > > > > > > > > aips1: bus@3000 {
> > > > > > > > > spba: bus@3000 {
> > > > > > > > >
> > > > > > > > > You have to devices with the same unit address. How do
> > > > > > > > > you share the address space?
> > > > > > > > >
> > > > > > > > > I think this should be rather fixed.
> > > > > > > >
> > > > > > > > And again, hungry keyboard ate a letter, so:
> > > > > > > >
> > > > > > > > You have _two_ devices with the same unit address. How do
> > > > > > > > you share the address space?
> > > > > > > > I think this should be rather fixed.
> > > > > > > >
> > > > > > >
> > > > > > > spba is the first block of aips1 space, so it has same start
> > > > > > > address as aips1.
> > > > > >
> > > > > > The reference manual describes it "Reserved for SDMA2 internal
>
Re: [PATCH] ASoC: wm8960: Remove bitclk relax condition
> >
> > Using a higher bitclk then expected doesn't always work.
> > Here is an example:
> >
> > aplay -Dhw:0,0 -d 5 -r 48000 -f S24_LE -c 2 audio48k24b2c.wav
> >
> > In this case, the required bitclk is 48000 * 24 * 2 = 2304000 but the
> > closest bitclk that can be derived is 3072000. Since the clock is
> > faster than expected, it will start to send bytes from the next
> > channel so the sound will be corrupted.
> >
> > Fixes: 82bab88910ee ("ASoC: codec: wm8960: Relax bit clock computation
> > when using PLL")
> > Fixes: 3c01b9ee2ab9 ("ASoC: codec: wm8960: Relax bit clock
> > computation")
> > Signed-off-by: Daniel Baluta
> > Signed-off-by: Shengjiu Wang
> > ---
>
> I think this is probably going to need a much more involved fix.
> The problem is that there are systems that depend on this behaviour, so you
> can't just flat out revert it. And to be fair the I2S specification says that
> bit
> clock can run at a higher rate than required for the data, so the behaviour is
> correct as well.
>
> Probably the best solution here is to add additional contraints from the
> machine driver on which rates/bit depths/channels are supported.
>
Just double check the issue, the real reason is in below:
The call sequence in wm8960_configure_clocking is
ret = wm8960_configure_sysclk();
if (ret >= 0)
goto configure_clock;
ret = wm8960_configure_pll();
configure_clock:
...
wm8960_configure_sysclk is called before wm8960_configure_pll, as
there is bitclk relax on both functions, so wm8960_configure_sysclk
always return success, then wm8960_configure_pll() never be called.
So bitclk relax condition should be removed in wm8960_configure_sysclk,
That wm8960_configure_pll can be called, and there is also bitclk relax
function in wm8960_configure_pll.
I will update and resend this patch.
Best regards
Wang shengjiu
