linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume
@ 2022-09-05 10:29 Shengjiu Wang
  2022-09-05 13:15 ` Nicolin Chen
  0 siblings, 1 reply; 4+ messages in thread
From: Shengjiu Wang @ 2022-09-05 10:29 UTC (permalink / raw)
  To: nicoleotsuka, Xiubo.Lee, festevam, shengjiu.wang, lgirdwood,
	broonie, perex, tiwai, alsa-devel
  Cc: linuxppc-dev, linux-kernel

If the initialization is not finished, then filling input data to
the FIFO may fail. So it is better to add initialization finishing
check in the runtime resume for suspend & resume case.

And consider the case of three instances working in parallel,
increase the retry times to 50 for more initialization time.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index aa5edf32d988..bae263b90ac1 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -20,6 +20,7 @@
 
 #define IDEAL_RATIO_DECIMAL_DEPTH 26
 #define DIVIDER_NUM  64
+#define INIT_TRY_NUM 50
 
 #define pair_err(fmt, ...) \
 	dev_err(&asrc->pdev->dev, "Pair %c: " fmt, 'A' + index, ##__VA_ARGS__)
@@ -579,7 +580,7 @@ static void fsl_asrc_start_pair(struct fsl_asrc_pair *pair)
 {
 	struct fsl_asrc *asrc = pair->asrc;
 	enum asrc_pair_index index = pair->index;
-	int reg, retry = 10, i;
+	int reg, retry = INIT_TRY_NUM, i;
 
 	/* Enable the current pair */
 	regmap_update_bits(asrc->regmap, REG_ASRCTR,
@@ -592,6 +593,10 @@ static void fsl_asrc_start_pair(struct fsl_asrc_pair *pair)
 		reg &= ASRCFG_INIRQi_MASK(index);
 	} while (!reg && --retry);
 
+	/* FIXME: Doesn't treat initialization timeout as error */
+	if (!retry)
+		dev_warn(&asrc->pdev->dev, "initialization isn't finished\n");
+
 	/* Make the input fifo to ASRC STALL level */
 	regmap_read(asrc->regmap, REG_ASRCNCR, &reg);
 	for (i = 0; i < pair->channels * 4; i++)
@@ -1257,6 +1262,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 {
 	struct fsl_asrc *asrc = dev_get_drvdata(dev);
 	struct fsl_asrc_priv *asrc_priv = asrc->private;
+	int reg, retry = INIT_TRY_NUM;
 	int i, ret;
 	u32 asrctr;
 
@@ -1295,6 +1301,17 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 	regmap_update_bits(asrc->regmap, REG_ASRCTR,
 			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
 
+	/* Wait for status of initialization for every enabled pairs */
+	do {
+		udelay(5);
+		regmap_read(asrc->regmap, REG_ASRCFG, &reg);
+		reg = (reg >> ASRCFG_INIRQi_SHIFT(0)) & 0x7;
+	} while ((reg != ((asrctr >> ASRCTR_ASRCEi_SHIFT(0)) & 0x7)) && --retry);
+
+	/* FIXME: Doesn't treat initialization timeout as error */
+	if (!retry)
+		dev_warn(dev, "initialization isn't finished\n");
+
 	return 0;
 
 disable_asrck_clk:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume
  2022-09-05 10:29 [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume Shengjiu Wang
@ 2022-09-05 13:15 ` Nicolin Chen
       [not found]   ` <CAA+D8APOL1Qx0fAhyajXXzh0_tqEmDJoDBh3Xgo6uYNhV0usBw@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolin Chen @ 2022-09-05 13:15 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Xiubo.Lee, festevam, shengjiu.wang, lgirdwood, broonie, perex,
	tiwai, alsa-devel, linuxppc-dev, linux-kernel

On Mon, Sep 5, 2022 at 3:47 AM Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> @@ -1295,6 +1301,17 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>         regmap_update_bits(asrc->regmap, REG_ASRCTR,
>                            ASRCTR_ASRCEi_ALL_MASK, asrctr);
>
> +       /* Wait for status of initialization for every enabled pairs */
> +       do {
> +               udelay(5);
> +               regmap_read(asrc->regmap, REG_ASRCFG, &reg);
> +               reg = (reg >> ASRCFG_INIRQi_SHIFT(0)) & 0x7;
> +       } while ((reg != ((asrctr >> ASRCTR_ASRCEi_SHIFT(0)) & 0x7)) && --retry);
> +
> +       /* FIXME: Doesn't treat initialization timeout as error */
> +       if (!retry)
> +               dev_warn(dev, "initialization isn't finished\n");

Any reason why not just dev_err?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume
       [not found]   ` <CAA+D8APOL1Qx0fAhyajXXzh0_tqEmDJoDBh3Xgo6uYNhV0usBw@mail.gmail.com>
@ 2022-09-06  9:50     ` Nicolin Chen
       [not found]       ` <CAA+D8AMYrQsu-nM_WdrBtB4iPU2UYNDhPW-zdY_Z+-9L03gu1Q@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolin Chen @ 2022-09-06  9:50 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Shengjiu Wang, Xiubo Li, Fabio Estevam, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	linuxppc-dev, linux-kernel

On Mon, Sep 5, 2022 at 6:54 PM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:

>> > +       /* Wait for status of initialization for every enabled pairs */
>> > +       do {
>> > +               udelay(5);
>> > +               regmap_read(asrc->regmap, REG_ASRCFG, &reg);
>> > +               reg = (reg >> ASRCFG_INIRQi_SHIFT(0)) & 0x7;
>> > +       } while ((reg != ((asrctr >> ASRCTR_ASRCEi_SHIFT(0)) & 0x7)) && --retry);
>> > +
>> > +       /* FIXME: Doesn't treat initialization timeout as error */
>> > +       if (!retry)
>> > +               dev_warn(dev, "initialization isn't finished\n");
>>
>> Any reason why not just dev_err?
>
> Just hesitate to use dev_err. if use dev_err, then should return an error.
> May one of the pairs is finished, it still can continue.

Makes sense. In that case, why "FIXME" :)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume
       [not found]       ` <CAA+D8AMYrQsu-nM_WdrBtB4iPU2UYNDhPW-zdY_Z+-9L03gu1Q@mail.gmail.com>
@ 2022-09-06 11:41         ` Nicolin Chen
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolin Chen @ 2022-09-06 11:41 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Shengjiu Wang, Xiubo Li, Fabio Estevam, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	linuxppc-dev, linux-kernel

On Tue, Sep 6, 2022 at 3:50 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
>> >> > +       /* Wait for status of initialization for every enabled pairs */
>> >> > +       do {
>> >> > +               udelay(5);
>> >> > +               regmap_read(asrc->regmap, REG_ASRCFG, &reg);
>> >> > +               reg = (reg >> ASRCFG_INIRQi_SHIFT(0)) & 0x7;
>> >> > +       } while ((reg != ((asrctr >> ASRCTR_ASRCEi_SHIFT(0)) & 0x7)) && --retry);
>> >> > +
>> >> > +       /* FIXME: Doesn't treat initialization timeout as error */
>> >> > +       if (!retry)
>> >> > +               dev_warn(dev, "initialization isn't finished\n");
>> >>
>> >> Any reason why not just dev_err?
>> >
>> > Just hesitate to use dev_err. if use dev_err, then should return an error.
>> > May one of the pairs is finished, it still can continue.
>>
>> Makes sense. In that case, why "FIXME" :)

> Just want to have a record/note here, need to care about this warning.

"FIXME" feels like something is wrong and literally means that it is
waiting for a fix/solution. In your case, it's not waiting for a fix
at all, but just an annotation? So, shouldn't it be just "Note:"?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-09-06 11:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05 10:29 [PATCH] ASoC: fsl_asrc: Add initialization finishing check in runtime resume Shengjiu Wang
2022-09-05 13:15 ` Nicolin Chen
     [not found]   ` <CAA+D8APOL1Qx0fAhyajXXzh0_tqEmDJoDBh3Xgo6uYNhV0usBw@mail.gmail.com>
2022-09-06  9:50     ` Nicolin Chen
     [not found]       ` <CAA+D8AMYrQsu-nM_WdrBtB4iPU2UYNDhPW-zdY_Z+-9L03gu1Q@mail.gmail.com>
2022-09-06 11:41         ` Nicolin Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).