From: Nicolin Chen <nicoleotsuka@gmail.com>
To: "S.j. Wang" <shengjiu.wang@nxp.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
"timur@kernel.org" <timur@kernel.org>,
"Xiubo.Lee@gmail.com" <Xiubo.Lee@gmail.com>,
"festevam@gmail.com" <festevam@gmail.com>,
"broonie@kernel.org" <broonie@kernel.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [EXT] Re: [PATCH] ASoC: fsl_asrc: replace the process_option table with function
Date: Wed, 10 Apr 2019 01:00:53 -0700 [thread overview]
Message-ID: <20190410080052.GA5180@Asurada> (raw)
In-Reply-To: <VE1PR04MB64798A5ADF025047AF5B9076E32E0@VE1PR04MB6479.eurprd04.prod.outlook.com>
On Wed, Apr 10, 2019 at 07:22:31AM +0000, S.j. Wang wrote:
> > The table was copied directly from the Reference Manual. We also have
> > listed all supported input and output sample rates just right behind that table.
> > If there're missing rates, we probably should update those two lists also?
> > Otherwise, how could we have a driver limiting both I/O sample rates while
> > we still see something not in the table?
> >
>
> Yes, I plan to send another patch to update the in/out rate list. Do I need
> To merge that to this commit? Actually we want to support 12k and 24KHz
Please send separate patches but in one series. And a question:
Is it possible to update the table? It'd be way quicker to use
lookup table than real-time calculation all the time. I believe
you can simply calculate all the values out for 12KHz and 24KHz
since you have the function. If there are certain combinations
of these two not being supported, then we could mark it with a
special value and add an if-check to error out.
> > > +static int proc_autosel(int Fsin, int Fsout, int *pre_proc, int
> > > +*post_proc)
> >
> > Please add some comments to this function to explain what it does, and how
> > it works. And better to rename it to something like "fsl_asrc_sel_proc".
> >
> Yes, some comments should be added, but not so detail, because this function
As much comments as possible.
> Is get from the design team, but the owner has left.
OK...that's sad...
> > Another thing confuses me: so we could have supported sample rates in the
> > list but the hardware might not support some of them because we couldn't
> > calculate their processing options?
>
> No, just want to support 12k, 24KHz, or others as customer like.
I was confused because the I/O rate lists not getting updated.
It makes sense now if you are abort to update them.
next prev parent reply other threads:[~2019-04-10 8:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-10 3:15 [PATCH] ASoC: fsl_asrc: replace the process_option table with function S.j. Wang
2019-04-10 4:26 ` Nicolin Chen
2019-04-10 7:22 ` [EXT] " S.j. Wang
2019-04-10 8:00 ` Nicolin Chen [this message]
2019-04-10 8:26 ` S.j. Wang
2019-04-10 9:51 ` Nicolin Chen
2019-04-10 10:34 ` S.j. Wang
2019-04-10 17:54 ` Nicolin Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190410080052.GA5180@Asurada \
--to=nicoleotsuka@gmail.com \
--cc=Xiubo.Lee@gmail.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=festevam@gmail.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=shengjiu.wang@nxp.com \
--cc=timur@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).