Re: [PATCH v2 0/5] PM: domains: Add helpers for multi PM domains to avoid open-coding
On Fri, Jan 5, 2024 at 6:02 PM Ulf Hansson wrote: > > Updates in v2: > - Ccing Daniel Baluta and Iuliana Prodan the NXP remoteproc patches to > requests help with testing. > - Fixed NULL pointer bug in patch1, pointed out by Nikunj. > - Added some tested/reviewed-by tags. Thanks for doing this Ulf. I remember that I've tried introducing the helpers some time ago but got side tracked by other tasks. https://lore.kernel.org/linux-pm/20200624103247.7115-1-daniel.bal...@oss.nxp.com/t/ Will review the series and test the remoteproc part this week. Daniel.
Re: [PATCH 2/2] arm64: dts: imx8mp: add reserve-memory nodes for DSP
On Tue, Sep 12, 2023 at 12:54 PM Iuliana Prodan wrote:
>
> On 9/12/2023 11:26 AM, Krzysztof Kozlowski wrote:
> > On 12/09/2023 10:13, Iuliana Prodan wrote:
> >> On 9/12/2023 10:07 AM, Krzysztof Kozlowski wrote:
> >>> On 12/09/2023 00:44, Iuliana Prodan (OSS) wrote:
> From: Iuliana Prodan
>
> Add the reserve-memory nodes used by DSP when the rpmsg
> feature is enabled.
> These can be later used in a dsp node, like:
> dsp: dsp@3b6e8000 {
> compatible = "fsl,imx8mp-dsp";
> reg = <0x3b6e8000 0x88000>;
> mbox-names = "tx0", "rx0", "rxdb0";
> mboxes = < 2 0>, < 2 1>,
> < 3 0>, < 3 1>;
> memory-region = <_vdev0buffer>, <_vdev0vring0>,
> <_vdev0vring1>, <_reserved>;
> status = "okay";
> >>> Drop this example from commit msg, useless and not really correct.
> >> Ok, will drop it. But this is a correct example, is just incomplete.
> > No, status=okay is redundant, thus it is not a correct example.
> ok
> };
>
> Signed-off-by: Iuliana Prodan
> ---
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index cc406bb338fe..eedc1921af62 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -210,6 +210,18 @@
> dsp_reserved: dsp@9240 {
> reg = <0 0x9240 0 0x200>;
> no-map;
> >>> Please test the patches before sending. This does not build.
> >> I've tested on remoteproc tree, but it seems I missed a bracket when
> >> sending upstream. Sorry abut this, will fix it in v2.
> > No, this is not how testing works. You must test this patch. This means
> > you tested something, then ported patch to entirely different tree,
> > resolved conflicts in buggy way and send it without testing. Nope.
> >
> >> Should I test this on other tree(s)?
> > You test the patch on the tree you send it. What is the point to test it
> > on some old code, cherry-pick with bugs and then send?
> >
> > If you have cross-tree dependencies between subsystem, isn't linux-next
> > for this?
linux-next tree is the tree we want.
[PATCH v2] ASoC: core: Don't set platform name when of_node is set
From: Daniel Baluta
A DAI link has 3 components:
* CPU
* platform
* codec(s)
A component is specified via:
* name
* of_node
* dai_name
In order to avoid confusion when building a sound card we disallow
matching by both name and of_node (1).
soc_check_tplg_fes allows overriding certain BE links by overriding
BE link name. This doesn't work well if BE link was specified via DT,
because we end up with a link with both name and of_node specified
which is conflicting with (1).
In order to fix this we need to:
* override of_node if component was specified via DT
* override name, otherwise.
Signed-off-by: Daniel Baluta
---
sound/soc/soc-core.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 522bae73640a..7a1c011a7fe0 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1648,7 +1648,11 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
dev_err(card->dev, "init platform error");
continue;
}
- dai_link->platforms->name = component->name;
+
+ if (component->dev->of_node)
+ dai_link->platforms->of_node =
component->dev->of_node;
+ else
+ dai_link->platforms->name = component->name;
/* convert non BE into BE */
if (!dai_link->no_pcm) {
--
2.27.0
Re: [PATCH] ASoC: core: Don't set platform name when of_node is set
On Fri, Mar 12, 2021 at 4:24 PM Mark Brown wrote:
>
> On Fri, Mar 12, 2021 at 02:37:30PM +0200, Daniel Baluta wrote:
> > On Fri, Mar 12, 2021 at 1:59 PM Mark Brown wrote:
>
> > > No, just the opposite! If there's an explict name configured why do you
> > > want to ignore it?
>
> > Because the initial assignment:
>
> > dai_link->platforms->name = component->name;
>
> > doesn't take into consideration that dai_link->platform->of_node is
> > also explicitly configured.
>
> But why should we take that into consideration here?
>
> > dai->link->platforms->of_node
> > configured and we hit this error:
> >
> > soc_dai_link_sanity_check:
> > /*
> > * Platform may be specified by either name or OF node, but it
> > * can be left unspecified, then no components will be inserted
> > * in the rtdcom list
> > */
> > if (!!platform->name == !!platform->of_node) {
> > dev_err(card->dev,
> > "ASoC: Neither/both platform name/of_node are set for %s\n",
> > link->name);
> > return -EINVAL;
> > }
>
> OK, but then does this check actually make sense? The code has been
> that way since the initial DT introduction since we disallow matching by
> both name and OF node in order to avoid confusion when building the card
> so I think it does but it's only with this mail that I get the
> information to figure out that this is something we actually check for
> then go find the reason why we check.
I will enhance the commit message and send v2. Hope to catch all the
inner details.
Re: [PATCH] ASoC: core: Don't set platform name when of_node is set
On Fri, Mar 12, 2021 at 1:59 PM Mark Brown wrote:
>
> On Fri, Mar 12, 2021 at 12:59:29PM +0200, Daniel Baluta wrote:
> > On Fri, Mar 12, 2021 at 12:50 PM Mark Brown wrote:
>
> > > If an explicit name has been provided why would we override it with an
> > > autogenerated one?
>
> > Wait, are you asking why the initial code:
>
> > dai_link->platforms->name = component->name;
>
> > is there in the initial code?
>
> No, just the opposite! If there's an explict name configured why do you
> want to ignore it?
Because the initial assignment:
dai_link->platforms->name = component->name;
doesn't take into consideration that dai_link->platform->of_node is
also explicitly configured.
So, my change only configures the name (dai_link->platform->name)
if the dai->platform->of_node wasn't previously explicitly configured.
Otherwise, we end up with both dai_link->platforms->name and
dai->link->platforms->of_node
configured and we hit this error:
soc_dai_link_sanity_check:
/*
* Platform may be specified by either name or OF node, but it
* can be left unspecified, then no components will be inserted
* in the rtdcom list
*/
if (!!platform->name == !!platform->of_node) {
dev_err(card->dev,
"ASoC: Neither/both platform name/of_node are set for %s\n", link->name);
return -EINVAL;
}
I start with a simple-audio-card node:
sof-sound-wm8960 {
compatible = "simple-audio-card";
simple-audio-card,dai-link {
format = "i2s";
cpu {
sound-dai = < 1>;
};
sndcodec: codec {
sound-dai = <>;
};
}
Notice that doesn't have any platform field.
But then in
sound/soc/generic/simple-card-utils.c:asoc_simple_canonicalize_platform
explicitly sets dai_link->platforms->of_node like this:
» if (!dai_link->platforms->of_node)
» » dai_link->platforms->of_node = dai_link->cpus->of_node;
I hope this is more clear.
Re: [PATCH] ASoC: core: Don't set platform name when of_node is set
On Fri, Mar 12, 2021 at 12:50 PM Mark Brown wrote: > > On Fri, Mar 12, 2021 at 10:32:54AM +0200, Daniel Baluta wrote: > > On Tue, Mar 9, 2021 at 5:38 PM Mark Brown wrote: > > > > > + if (!dai_link->platforms->of_node) > > > > + dai_link->platforms->name = > > > > component->name; > > > > Why would we prefer the node name over something explicitly configured? > > > Not sure I follow your question. I think the difference stands in the > > way we treat OF vs non-OF platforms. > > If an explicit name has been provided why would we override it with an > autogenerated one? Wait, are you asking why the initial code: dai_link->platforms->name = component->name; is there in the initial code?
Re: [PATCH] ASoC: core: Don't set platform name when of_node is set
On Tue, Mar 9, 2021 at 5:38 PM Mark Brown wrote: > > On Tue, Mar 09, 2021 at 10:23:28AM +0200, Daniel Baluta wrote: > > From: Daniel Baluta > > > > Platform may be specified by either name or OF node but not > > both. > > > > For OF node platforms (e.g i.MX) we end up with both platform name > > and of_node set and sound card registration will fail with the error: > > > > asoc-simple-card sof-sound-wm8960: ASoC: Neither/both > > platform name/of_node are set for sai1-wm8960-hifi > > This doesn't actually say what the change does. Will send v2 with a better explanation. > > > - dai_link->platforms->name = component->name; > > + > > + if (!dai_link->platforms->of_node) > > + dai_link->platforms->name = component->name; > > Why would we prefer the node name over something explicitly configured? Not sure I follow your question. I think the difference stands in the way we treat OF vs non-OF platforms. With OF-platforms, dai_link->platforms->of_node is always set! So we no longer need to set dai->platforms->name. Actually setting both of_node and name will make sound card registration fail! In this is the case I'm trying to fix here.
[PATCH] ASoC: core: Don't set platform name when of_node is set
From: Daniel Baluta
Platform may be specified by either name or OF node but not
both.
For OF node platforms (e.g i.MX) we end up with both platform name
and of_node set and sound card registration will fail with the error:
asoc-simple-card sof-sound-wm8960: ASoC: Neither/both
platform name/of_node are set for sai1-wm8960-hifi
Signed-off-by: Daniel Baluta
---
sound/soc/soc-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 16ba54eb8164..76ab42fa9461 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1660,7 +1660,9 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
dev_err(card->dev, "init platform error");
continue;
}
- dai_link->platforms->name = component->name;
+
+ if (!dai_link->platforms->of_node)
+ dai_link->platforms->name = component->name;
/* convert non BE into BE */
if (!dai_link->no_pcm) {
--
2.27.0
Re: [PATCH] ASoC: wm8962: Relax bit clock divider searching
On Fri, Mar 5, 2021 at 1:15 AM Shengjiu Wang wrote: > > With S20_3LE format case, the sysclk = rate * 384, > the bclk = rate * 20 * 2, there is no proper bclk divider > for 384 / 40, because current condition needs exact match. > So driver fails to configure the clocking: > > wm8962 3-001a: Unsupported BCLK ratio 9 > > Fix this by relaxing bitclk divider searching, so that when > no exact value can be derived from sysclk pick the closest > value greater than expected bitclk. > > Signed-off-by: Shengjiu Wang Reviewed-by: Daniel Baluta
Re: [PATCH V3 1/5] firmware: imx: scu-seco: Add Secure Controller APIS
On Tue, Nov 10, 2020 at 4:15 PM wrote: > > From: Franck LENORMAND Looks good to me. Thanks Franck! Reviewed-by: Daniel Baluta
[PATCH RESEND 3/3] firmware: imx-dsp: Export functions to request/free channels
In order to save power, we only need to request a channel
when the communication with the DSP active.
For this we export the following functions:
- imx_dsp_request_channel, gets a channel with a given index
- imx_dsp_free_channel, frees a channel with a given index
Notice that we still request channels at probe to support devices
that do not have PM callbacks implemented.
More explanations about why requesting a channel has an effect
on power savings:
- requesting an mailbox channel will call mailbox's startup
function.
- startup function calls pm_runtime_get_sync which increments device
usage count and will keep the device active. Specifically, mailbox
clock will be always ON when a mailbox channel is requested.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 25 +
include/linux/firmware/imx/dsp.h | 10 ++
2 files changed, 35 insertions(+)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index b6e95d6d34c0..a6c06d7476c3 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,6 +60,31 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return ERR_PTR(-EINVAL);
+
+ dsp_chan = _ipc->chans[idx];
+ dsp_chan->ch = mbox_request_channel_byname(_chan->cl,
dsp_chan->name);
+ return dsp_chan->ch;
+}
+EXPORT_SYMBOL(imx_dsp_request_channel);
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return;
+
+ dsp_chan = _ipc->chans[idx];
+ mbox_free_channel(dsp_chan->ch);
+}
+EXPORT_SYMBOL(imx_dsp_free_channel);
+
static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
struct device *dev = dsp_ipc->dev;
diff --git a/include/linux/firmware/imx/dsp.h b/include/linux/firmware/imx/dsp.h
index 7562099c9e46..4f7895a3b73c 100644
--- a/include/linux/firmware/imx/dsp.h
+++ b/include/linux/firmware/imx/dsp.h
@@ -55,6 +55,9 @@ static inline void *imx_dsp_get_data(struct imx_dsp_ipc *ipc)
int imx_dsp_ring_doorbell(struct imx_dsp_ipc *dsp, unsigned int chan_idx);
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx);
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx);
+
#else
static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc *ipc,
@@ -63,5 +66,12 @@ static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc
*ipc,
return -ENOTSUPP;
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx) { }
+
#endif
#endif /* _IMX_DSP_IPC_H */
--
2.17.1
[PATCH RESEND 2/3] firmware: imx: Save channel name for further use
We want to request / free channels on demand later in order
to save power.
For this for each channel we save the name and use it to
reference the channel later.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index a3a018c87b52..b6e95d6d34c0 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -79,6 +79,7 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
return -ENOMEM;
dsp_chan = _ipc->chans[i];
+ dsp_chan->name = chan_name;
cl = _chan->cl;
cl->dev = dev;
cl->tx_block = false;
@@ -97,16 +98,14 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc
*dsp_ipc)
}
dev_dbg(dev, "request mbox chan %s\n", chan_name);
- /* chan_name is not used anymore by framework */
- kfree(chan_name);
}
return 0;
out:
- kfree(chan_name);
for (j = 0; j < i; j++) {
dsp_chan = _ipc->chans[j];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return ret;
@@ -147,6 +146,7 @@ static int imx_dsp_remove(struct platform_device *pdev)
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
dsp_chan = _ipc->chans[i];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return 0;
--
2.17.1
[PATCH RESEND 0/3] Allow on demand channel request / free
Requesting an mailbox channel will call mailbox's startup function. startup function calls pm_runtime_get_sync which increments device usage count and will keep the device active. Specifically, mailbox clock will be always ON when a mailbox channel is requested. For this, reason we introduce a way to request/free IMX DSP channels on demand to save power when the channels are not used. First two patches are doing code refactoring preparing the path for 3rd patch which exports functions for on demand channel request/free Daniel Baluta (3): firmware: imx: Introduce imx_dsp_setup_channels firmware: imx: Save channel name for further use firmware: imx-dsp: Export functions to request/free channels drivers/firmware/imx/imx-dsp.c | 72 include/linux/firmware/imx/dsp.h | 10 + 2 files changed, 64 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH RESEND 1/3] firmware: imx: Introduce imx_dsp_setup_channels
Create a separate function that sets up DSP mailbox channels
so that imx_dsp_probe function will be easier to read.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 41 +-
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index 4265e9dbed84..a3a018c87b52 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,22 +60,15 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
-static int imx_dsp_probe(struct platform_device *pdev)
+static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
- struct device *dev = >dev;
- struct imx_dsp_ipc *dsp_ipc;
+ struct device *dev = dsp_ipc->dev;
struct imx_dsp_chan *dsp_chan;
struct mbox_client *cl;
char *chan_name;
int ret;
int i, j;
- device_set_of_node_from_dev(>dev, pdev->dev.parent);
-
- dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
- if (!dsp_ipc)
- return -ENOMEM;
-
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
if (i < 2)
chan_name = kasprintf(GFP_KERNEL, "txdb%d", i);
@@ -108,12 +101,6 @@ static int imx_dsp_probe(struct platform_device *pdev)
kfree(chan_name);
}
- dsp_ipc->dev = dev;
-
- dev_set_drvdata(dev, dsp_ipc);
-
- dev_info(dev, "NXP i.MX DSP IPC initialized\n");
-
return 0;
out:
kfree(chan_name);
@@ -125,6 +112,30 @@ static int imx_dsp_probe(struct platform_device *pdev)
return ret;
}
+static int imx_dsp_probe(struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ struct imx_dsp_ipc *dsp_ipc;
+ int ret;
+
+ device_set_of_node_from_dev(>dev, pdev->dev.parent);
+
+ dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
+ if (!dsp_ipc)
+ return -ENOMEM;
+
+ dsp_ipc->dev = dev;
+ dev_set_drvdata(dev, dsp_ipc);
+
+ ret = imx_dsp_setup_channels(dsp_ipc);
+ if (ret < 0)
+ return ret;
+
+ dev_info(dev, "NXP i.MX DSP IPC initialized\n");
+
+ return 0;
+}
+
static int imx_dsp_remove(struct platform_device *pdev)
{
struct imx_dsp_chan *dsp_chan;
--
2.17.1
Re: [PATCH] ASoC: fsl_spdif: Add support for higher sample rates
On Tue, Oct 13, 2020 at 1:49 PM Shengjiu Wang wrote: > > On Tue, Oct 13, 2020 at 6:42 PM Daniel Baluta wrote: > > > > On Tue, Oct 13, 2020 at 12:29 PM Nicolin Chen > > wrote: > > > > > > Hi Shengjiu, > > > > > > On Mon, Oct 12, 2020 at 04:49:42PM +0800, Shengjiu Wang wrote: > > > > Add 88200Hz and 176400Hz sample rates support for TX. > > > > Add 88200Hz, 176400Hz, 192000Hz sample rates support for RX. > > > > > > > > Signed-off-by: Shengjiu Wang > > > > Signed-off-by: Viorel Suman > > > > > > Probably should put your own Signed-off at the bottom? > > > > Hi Shengjiu, > > > > Also please keep the original author of the patch. You can change that > > using git commit --amend --author="Viorel Suman ". > > Actually I combined my commit with viorel suman's commit to one commit, > not only viorel suman's. I see. Ok then :) Reviewed-by: Daniel Baluta
Re: [PATCH] ASoC: fsl_spdif: Add support for higher sample rates
On Tue, Oct 13, 2020 at 12:29 PM Nicolin Chen wrote: > > Hi Shengjiu, > > On Mon, Oct 12, 2020 at 04:49:42PM +0800, Shengjiu Wang wrote: > > Add 88200Hz and 176400Hz sample rates support for TX. > > Add 88200Hz, 176400Hz, 192000Hz sample rates support for RX. > > > > Signed-off-by: Shengjiu Wang > > Signed-off-by: Viorel Suman > > Probably should put your own Signed-off at the bottom? Hi Shengjiu, Also please keep the original author of the patch. You can change that using git commit --amend --author="Viorel Suman ". With that, Reviewed-by: Daniel Baluta
[PATCH 1/2] ASoC: SOF: Activate runtime PM with SOF OF device
From: Daniel Baluta SOF boots the DSP at probe and keeps it up all the time. With this change, after booting if no one is using the DSP the SOF core will turn off the DSP to save power. Reviewed-by: Kai Vehmanen Reviewed-by: Paul Olaru Reviewed-by: Pierre-Louis Bossart Signed-off-by: Daniel Baluta --- sound/soc/sof/sof-of-dev.c | 4 1 file changed, 4 insertions(+) diff --git a/sound/soc/sof/sof-of-dev.c b/sound/soc/sof/sof-of-dev.c index f492c5dfa659..de9acc0e33cb 100644 --- a/sound/soc/sof/sof-of-dev.c +++ b/sound/soc/sof/sof-of-dev.c @@ -56,7 +56,11 @@ static void sof_of_probe_complete(struct device *dev) /* allow runtime_pm */ pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); pm_runtime_use_autosuspend(dev); + pm_runtime_set_active(dev); pm_runtime_enable(dev); + + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); } static int sof_of_probe(struct platform_device *pdev) -- 2.17.1
[PATCH 2/2] ASoC: SOF: Add .prepare/.complete callbacks
From: Daniel Baluta
Use SOF defined callbacks (snd_sof_prepare/snd_sof_complete)
in order to update internal SOF system suspend target.
Reviewed-by: Kai Vehmanen
Reviewed-by: Paul Olaru
Reviewed-by: Pierre-Louis Bossart
Signed-off-by: Daniel Baluta
---
sound/soc/sof/sof-of-dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sound/soc/sof/sof-of-dev.c b/sound/soc/sof/sof-of-dev.c
index de9acc0e33cb..85ff0db88eb7 100644
--- a/sound/soc/sof/sof-of-dev.c
+++ b/sound/soc/sof/sof-of-dev.c
@@ -46,6 +46,8 @@ static struct sof_dev_desc sof_of_imx8mp_desc = {
#endif
static const struct dev_pm_ops sof_of_pm = {
+ .prepare = snd_sof_prepare,
+ .complete = snd_sof_complete,
SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
NULL)
--
2.17.1
[PATCH 0/2] Enable runtime PM for SOF device
From: Daniel Baluta This enables runtime PM for SOF device. Next patchseries will provide PM suspend/resume handlers for i.MX8 specific devices. Daniel Baluta (2): ASoC: SOF: Activate runtime PM with SOF OF device ASoC: SOF: Add .prepare/.complete callbacks sound/soc/sof/sof-of-dev.c | 6 ++ 1 file changed, 6 insertions(+) -- 2.17.1
[RESEND PATCH 1/3] firmware: imx: Introduce imx_dsp_setup_channels
From: Daniel Baluta
Create a separate function that sets up DSP mailbox channels
so that imx_dsp_probe function will be easier to read.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 41 +-
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index 4265e9dbed84..a3a018c87b52 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,22 +60,15 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
-static int imx_dsp_probe(struct platform_device *pdev)
+static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
- struct device *dev = >dev;
- struct imx_dsp_ipc *dsp_ipc;
+ struct device *dev = dsp_ipc->dev;
struct imx_dsp_chan *dsp_chan;
struct mbox_client *cl;
char *chan_name;
int ret;
int i, j;
- device_set_of_node_from_dev(>dev, pdev->dev.parent);
-
- dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
- if (!dsp_ipc)
- return -ENOMEM;
-
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
if (i < 2)
chan_name = kasprintf(GFP_KERNEL, "txdb%d", i);
@@ -108,12 +101,6 @@ static int imx_dsp_probe(struct platform_device *pdev)
kfree(chan_name);
}
- dsp_ipc->dev = dev;
-
- dev_set_drvdata(dev, dsp_ipc);
-
- dev_info(dev, "NXP i.MX DSP IPC initialized\n");
-
return 0;
out:
kfree(chan_name);
@@ -125,6 +112,30 @@ static int imx_dsp_probe(struct platform_device *pdev)
return ret;
}
+static int imx_dsp_probe(struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ struct imx_dsp_ipc *dsp_ipc;
+ int ret;
+
+ device_set_of_node_from_dev(>dev, pdev->dev.parent);
+
+ dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
+ if (!dsp_ipc)
+ return -ENOMEM;
+
+ dsp_ipc->dev = dev;
+ dev_set_drvdata(dev, dsp_ipc);
+
+ ret = imx_dsp_setup_channels(dsp_ipc);
+ if (ret < 0)
+ return ret;
+
+ dev_info(dev, "NXP i.MX DSP IPC initialized\n");
+
+ return 0;
+}
+
static int imx_dsp_remove(struct platform_device *pdev)
{
struct imx_dsp_chan *dsp_chan;
--
2.17.1
[RESEND PATCH 3/3] firmware: imx-dsp: Export functions to request/free channels
From: Daniel Baluta
In order to save power, we only need to request a channel
when the communication with the DSP active.
For this we export the following functions:
- imx_dsp_request_channel, gets a channel with a given index
- imx_dsp_free_channel, frees a channel with a given index
Notice that we still request channels at probe to support devices
that do not have PM callbacks implemented.
More explanations about why requesting a channel has an effect
on power savings:
- requesting an mailbox channel will call mailbox's startup
function.
- startup function calls pm_runtime_get_sync which increments device
usage count and will keep the device active. Specifically, mailbox
clock will be always ON when a mailbox channel is requested.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 25 +
include/linux/firmware/imx/dsp.h | 10 ++
2 files changed, 35 insertions(+)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index b6e95d6d34c0..a6c06d7476c3 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,6 +60,31 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return ERR_PTR(-EINVAL);
+
+ dsp_chan = _ipc->chans[idx];
+ dsp_chan->ch = mbox_request_channel_byname(_chan->cl,
dsp_chan->name);
+ return dsp_chan->ch;
+}
+EXPORT_SYMBOL(imx_dsp_request_channel);
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return;
+
+ dsp_chan = _ipc->chans[idx];
+ mbox_free_channel(dsp_chan->ch);
+}
+EXPORT_SYMBOL(imx_dsp_free_channel);
+
static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
struct device *dev = dsp_ipc->dev;
diff --git a/include/linux/firmware/imx/dsp.h b/include/linux/firmware/imx/dsp.h
index 7562099c9e46..4f7895a3b73c 100644
--- a/include/linux/firmware/imx/dsp.h
+++ b/include/linux/firmware/imx/dsp.h
@@ -55,6 +55,9 @@ static inline void *imx_dsp_get_data(struct imx_dsp_ipc *ipc)
int imx_dsp_ring_doorbell(struct imx_dsp_ipc *dsp, unsigned int chan_idx);
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx);
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx);
+
#else
static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc *ipc,
@@ -63,5 +66,12 @@ static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc
*ipc,
return -ENOTSUPP;
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx) { }
+
#endif
#endif /* _IMX_DSP_IPC_H */
--
2.17.1
[RESEND PATCH 2/3] firmware: imx: Save channel name for further use
From: Daniel Baluta
We want to request / free channels on demand later in order
to save power.
For this for each channel we save the name and use it to
reference the channel later.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index a3a018c87b52..b6e95d6d34c0 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -79,6 +79,7 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
return -ENOMEM;
dsp_chan = _ipc->chans[i];
+ dsp_chan->name = chan_name;
cl = _chan->cl;
cl->dev = dev;
cl->tx_block = false;
@@ -97,16 +98,14 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc
*dsp_ipc)
}
dev_dbg(dev, "request mbox chan %s\n", chan_name);
- /* chan_name is not used anymore by framework */
- kfree(chan_name);
}
return 0;
out:
- kfree(chan_name);
for (j = 0; j < i; j++) {
dsp_chan = _ipc->chans[j];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return ret;
@@ -147,6 +146,7 @@ static int imx_dsp_remove(struct platform_device *pdev)
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
dsp_chan = _ipc->chans[i];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return 0;
--
2.17.1
[RESEND PATCH 0/3] Allow on demand channel request / free
From: Daniel Baluta Requesting an mailbox channel will call mailbox's startup function. startup function calls pm_runtime_get_sync which increments device usage count and will keep the device active. Specifically, mailbox clock will be always ON when a mailbox channel is requested. For this, reason we introduce a way to request/free IMX DSP channels on demand to save power when the channels are not used. First two patches are doing code refactoring preparing the path for 3rd patch which exports functions for on demand channel request/free Daniel Baluta (3): firmware: imx: Introduce imx_dsp_setup_channels firmware: imx: Save channel name for further use firmware: imx-dsp: Export functions to request/free channels drivers/firmware/imx/imx-dsp.c | 72 include/linux/firmware/imx/dsp.h | 10 + 2 files changed, 64 insertions(+), 18 deletions(-) -- 2.17.1
Re: [RFC PATCH v2 6/6] ASoC: q6asm-dai: add support to set_codec_params
On Tue, Jul 21, 2020 at 8:03 PM Srinivas Kandagatla
wrote:
>
> Make use of new set_codec_params callback to allow decoder switching
> during gapless playback.
>
> Signed-off-by: Srinivas Kandagatla
> ---
> sound/soc/qcom/qdsp6/q6asm-dai.c | 33
> 1 file changed, 33 insertions(+)
>
> diff --git a/sound/soc/qcom/qdsp6/q6asm-dai.c
> b/sound/soc/qcom/qdsp6/q6asm-dai.c
> index b5c719682919..a8cfb1996614 100644
> --- a/sound/soc/qcom/qdsp6/q6asm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6asm-dai.c
> @@ -876,6 +876,37 @@ static int __q6asm_dai_compr_set_codec_params(struct
> snd_soc_component *componen
> return 0;
> }
>
> +static int q6asm_dai_compr_set_codec_params(struct snd_soc_component
> *component,
> + struct snd_compr_stream *stream,
> + struct snd_codec *codec)
> +{
> + struct snd_compr_runtime *runtime = stream->runtime;
> + struct q6asm_dai_rtd *prtd = runtime->private_data;
> + int ret;
> +
> + ret = q6asm_open_write(prtd->audio_client, prtd->next_track_stream_id,
> + codec->id, codec->profile,
> prtd->bits_per_sample,
> + true);
> + if (ret < 0) {
> + pr_err("q6asm_open_write failed\n");
Since you have component->dev here I think it is worth it to use
dev_err instead of pr_err.
Same for the rest of the code.
[PATCH 3/7] ASoC: SOF: imx8: Fix ESAI DAI driver name for i.MX8/iMX8X
From: Daniel Baluta
This must match DAI name from topology. Also, esai-port is too generic
as they are 2 ESAIs on i.MX8/i.MX8X boards.
SOF integration only uses ESAI0 for now.
Signed-off-by: Daniel Baluta
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Ranjani Sridharan
Reviewed-by: Guennadi Liakhovetski
---
sound/soc/sof/imx/imx8.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index c7aab646cb8e..f1308824e2cd 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -374,7 +374,7 @@ static int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
static struct snd_soc_dai_driver imx8_dai[] = {
{
- .name = "esai-port",
+ .name = "esai0",
.playback = {
.channels_min = 1,
.channels_max = 8,
--
2.17.1
[PATCH 0/7] SOF IMX fixes
From: Daniel Baluta This patchseries contains a couple of SOF IMX fixes found during our first IMX SOF release. Daniel Baluta (7): ASoC: SOF: define INFO_ flags in dsp_ops for imx8 ASoC: SOF: imx: Use ARRAY_SIZE instead of hardcoded value ASoC: SOF: imx8: Fix ESAI DAI driver name for i.MX8/iMX8X ASoC: SOF: imx8m: Fix SAI DAI driver for i.MX8M ASoC: SOF: imx8: Add SAI dai driver for i.MX/i.MX8X ASoC: SOF: topology: Update SAI config bclk/fsync rate ASoC: SOF: pcm: Update rate/channels for SAI/ESAI DAIs sound/soc/sof/imx/imx8.c | 24 +--- sound/soc/sof/imx/imx8m.c | 4 ++-- sound/soc/sof/pcm.c | 8 sound/soc/sof/topology.c | 2 ++ 4 files changed, 33 insertions(+), 5 deletions(-) -- 2.17.1
[PATCH 2/7] ASoC: SOF: imx: Use ARRAY_SIZE instead of hardcoded value
From: Daniel Baluta
With this change we no longer need to update num_drv when adding
new DAI driver.
Signed-off-by: Daniel Baluta
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Ranjani Sridharan
Reviewed-by: Guennadi Liakhovetski
---
sound/soc/sof/imx/imx8.c | 4 ++--
sound/soc/sof/imx/imx8m.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index b558132bb609..c7aab646cb8e 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -415,7 +415,7 @@ struct snd_sof_dsp_ops sof_imx8_ops = {
/* DAI drivers */
.drv = imx8_dai,
- .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
+ .num_drv = ARRAY_SIZE(imx8_dai),
/* ALSA HW info flags */
.hw_info = SNDRV_PCM_INFO_MMAP |
@@ -455,7 +455,7 @@ struct snd_sof_dsp_ops sof_imx8x_ops = {
/* DAI drivers */
.drv = imx8_dai,
- .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
+ .num_drv = ARRAY_SIZE(imx8_dai),
/* ALSA HW info flags */
.hw_info = SNDRV_PCM_INFO_MMAP |
diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index 287114a37688..067d2424c682 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -280,7 +280,7 @@ struct snd_sof_dsp_ops sof_imx8m_ops = {
/* DAI drivers */
.drv = imx8m_dai,
- .num_drv = 1, /* we have only 1 SAI interface on i.MX8M */
+ .num_drv = ARRAY_SIZE(imx8m_dai),
.hw_info = SNDRV_PCM_INFO_MMAP |
SNDRV_PCM_INFO_MMAP_VALID |
--
2.17.1
[PATCH 7/7] ASoC: SOF: pcm: Update rate/channels for SAI/ESAI DAIs
From: Daniel Baluta Fixup BE DAI links rate/channels parameters to match any values from topology. Signed-off-by: Daniel Baluta Reviewed-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan --- sound/soc/sof/pcm.c | 8 1 file changed, 8 insertions(+) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 22fe9d5e932b..5cfd2611b252 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -718,17 +718,25 @@ static int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, /* do nothing for ALH dai_link */ break; case SOF_DAI_IMX_ESAI: + rate->min = dai->dai_config->esai.fsync_rate; + rate->max = dai->dai_config->esai.fsync_rate; channels->min = dai->dai_config->esai.tdm_slots; channels->max = dai->dai_config->esai.tdm_slots; + dev_dbg(component->dev, + "rate_min: %d rate_max: %d\n", rate->min, rate->max); dev_dbg(component->dev, "channels_min: %d channels_max: %d\n", channels->min, channels->max); break; case SOF_DAI_IMX_SAI: + rate->min = dai->dai_config->sai.fsync_rate; + rate->max = dai->dai_config->sai.fsync_rate; channels->min = dai->dai_config->sai.tdm_slots; channels->max = dai->dai_config->sai.tdm_slots; + dev_dbg(component->dev, + "rate_min: %d rate_max: %d\n", rate->min, rate->max); dev_dbg(component->dev, "channels_min: %d channels_max: %d\n", channels->min, channels->max); -- 2.17.1
[PATCH 1/7] ASoC: SOF: define INFO_ flags in dsp_ops for imx8
From: Daniel Baluta
In the past, the INFO_ flags such as PAUSE/NO_PERIOD_WAKEUP were
defined in the SOF PCM core, but that was changed since
commit 27e322fabd50 ("ASoC: SOF: define INFO_ flags in dsp_ops")
Now these flags must be set in DSP ops.
Signed-off-by: Daniel Baluta
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Ranjani Sridharan
Reviewed-by: Guennadi Liakhovetski
---
sound/soc/sof/imx/imx8.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index a4fa8451d8cb..b558132bb609 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -416,6 +416,13 @@ struct snd_sof_dsp_ops sof_imx8_ops = {
/* DAI drivers */
.drv = imx8_dai,
.num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
+
+ /* ALSA HW info flags */
+ .hw_info = SNDRV_PCM_INFO_MMAP |
+ SNDRV_PCM_INFO_MMAP_VALID |
+ SNDRV_PCM_INFO_INTERLEAVED |
+ SNDRV_PCM_INFO_PAUSE |
+ SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
};
EXPORT_SYMBOL(sof_imx8_ops);
--
2.17.1
[PATCH 5/7] ASoC: SOF: imx8: Add SAI dai driver for i.MX/i.MX8X
From: Daniel Baluta
With SOF we support 1 ESAI interface and 1 SAI interface.
This patch adds SAI1 interface support existing on i.MX8/i.MX8X
boards.
Signed-off-by: Daniel Baluta
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Ranjani Sridharan
Reviewed-by: Guennadi Liakhovetski
---
sound/soc/sof/imx/imx8.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index f1308824e2cd..bc0628c7b88c 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -384,6 +384,17 @@ static struct snd_soc_dai_driver imx8_dai[] = {
.channels_max = 8,
},
},
+{
+ .name = "sai1",
+ .playback = {
+ .channels_min = 1,
+ .channels_max = 32,
+ },
+ .capture = {
+ .channels_min = 1,
+ .channels_max = 32,
+ },
+},
};
/* i.MX8 ops */
--
2.17.1
[PATCH 4/7] ASoC: SOF: imx8m: Fix SAI DAI driver for i.MX8M
From: Daniel Baluta
This must match DAI name from topology. Also, sai-port
is too generic. Physical DAI port on i.MX8MP is labeled SAI3.
Signed-off-by: Daniel Baluta
Reviewed-by: Pierre-Louis Bossart
Reviewed-by: Kai Vehmanen
Reviewed-by: Ranjani Sridharan
Reviewed-by: Guennadi Liakhovetski
---
sound/soc/sof/imx/imx8m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index 067d2424c682..3b9c560cd40f 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -239,7 +239,7 @@ static int imx8m_ipc_pcm_params(struct snd_sof_dev *sdev,
static struct snd_soc_dai_driver imx8m_dai[] = {
{
- .name = "sai-port",
+ .name = "sai3",
.playback = {
.channels_min = 1,
.channels_max = 32,
--
2.17.1
[PATCH 6/7] ASoC: SOF: topology: Update SAI config bclk/fsync rate
From: Daniel Baluta These parameters are read from topology file and sent to DSP. Signed-off-by: Daniel Baluta Reviewed-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan --- sound/soc/sof/topology.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 6a9703e5ff60..13e10a0c0b05 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2831,6 +2831,8 @@ static int sof_link_sai_load(struct snd_soc_component *scomp, int index, } config->sai.mclk_rate = le32_to_cpu(hw_config->mclk_rate); + config->sai.bclk_rate = le32_to_cpu(hw_config->bclk_rate); + config->sai.fsync_rate = le32_to_cpu(hw_config->fsync_rate); config->sai.mclk_direction = hw_config->mclk_direction; config->sai.tdm_slots = le32_to_cpu(hw_config->tdm_slots); -- 2.17.1
Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
On 7/17/20 2:44 AM, Anson Huang wrote:
Hi, Daniel
Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
module
On 7/16/20 6:21 PM, Anson Huang wrote:
Hi, Daniel
Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
driver as module
Hi Anson,
Few comments inline:
On 7/16/20 6:06 PM, Anson Huang wrote:
To support building i.MX SCU pinctrl driver as module, below things
need to
be changed:
- Export SCU related functions and use "IS_ENABLED" instead of
"ifdef" to support SCU pinctrl driver user and itself to be
built as module;
- Use function callbacks for SCU related functions in pinctrl-imx.c
in order to support the scenario of PINCTRL_IMX is built in
while PINCTRL_IMX_SCU is built as module;
- All drivers using SCU pinctrl driver need to initialize the
SCU related function callback;
- Change PINCTR_IMX_SCU to tristate;
- Add module author, description and license.
With above changes, i.MX SCU pinctrl driver can be built as module.
There are a lot of changes here. I think it would be better to try to
split them
per functionality. One functional change per patch.
Actually, I ever tried to split them, but the function will be broken.
All the changes are just to support the module build. If split them,
the bisect will have pinctrl build or function broken.
Hi Anson,
I see your point and I know that this is a very hard task to get it right from
the first patches.
But let me suggest at least that:
- changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file and
MODULE_ macros should go to a separate patch).
You meant in patch #2, the changes in Kconfig and the changes in .c file should
be split to 2 patches?
No. I mean look at path #1.
The section above should be placed in a separate patch.
static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c
b/drivers/pinctrl/freescale/pinctrl-scu.c
index 9df45d3..59b5f8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -7,6 +7,7 @@
#include
#include
+#include
#include
#include
#include
@@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
pin_scu->mux_mode, pin_scu->config);
}
EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
+
+MODULE_AUTHOR("Dong Aisheng");
+MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
+MODULE_LICENSE("GPL v2");
This can be in a separate patch.
Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
On 7/16/20 6:21 PM, Anson Huang wrote: Hi, Daniel Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module Hi Anson, Few comments inline: On 7/16/20 6:06 PM, Anson Huang wrote: To support building i.MX SCU pinctrl driver as module, below things need to be changed: - Export SCU related functions and use "IS_ENABLED" instead of "ifdef" to support SCU pinctrl driver user and itself to be built as module; - Use function callbacks for SCU related functions in pinctrl-imx.c in order to support the scenario of PINCTRL_IMX is built in while PINCTRL_IMX_SCU is built as module; - All drivers using SCU pinctrl driver need to initialize the SCU related function callback; - Change PINCTR_IMX_SCU to tristate; - Add module author, description and license. With above changes, i.MX SCU pinctrl driver can be built as module. There are a lot of changes here. I think it would be better to try to split them per functionality. One functional change per patch. Actually, I ever tried to split them, but the function will be broken. All the changes are just to support the module build. If split them, the bisect will have pinctrl build or function broken. Hi Anson, I see your point and I know that this is a very hard task to get it right from the first patches. But let me suggest at least that: - changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file and MODULE_ macros should go to a separate patch). thanks Daniel.
Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module
Hi Anson,
Few comments inline:
On 7/16/20 6:06 PM, Anson Huang wrote:
To support building i.MX SCU pinctrl driver as module, below things need to be
changed:
- Export SCU related functions and use "IS_ENABLED" instead of
"ifdef" to support SCU pinctrl driver user and itself to be
built as module;
- Use function callbacks for SCU related functions in pinctrl-imx.c
in order to support the scenario of PINCTRL_IMX is built in
while PINCTRL_IMX_SCU is built as module;
- All drivers using SCU pinctrl driver need to initialize the
SCU related function callback;
- Change PINCTR_IMX_SCU to tristate;
- Add module author, description and license.
With above changes, i.MX SCU pinctrl driver can be built as module.
There are a lot of changes here. I think it would be better to try to
split them
per functionality. One functional change per patch.
Signed-off-by: Anson Huang
---
drivers/pinctrl/freescale/Kconfig | 2 +-
drivers/pinctrl/freescale/pinctrl-imx.c | 8 ++--
drivers/pinctrl/freescale/pinctrl-imx.h | 57 -
drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 3 ++
drivers/pinctrl/freescale/pinctrl-imx8qm.c | 3 ++
drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 3 ++
drivers/pinctrl/freescale/pinctrl-scu.c | 5 +++
7 files changed, 42 insertions(+), 39 deletions(-)
diff --git a/drivers/pinctrl/freescale/Kconfig
b/drivers/pinctrl/freescale/Kconfig
index 08fcf5c..570355c 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -7,7 +7,7 @@ config PINCTRL_IMX
select REGMAP
config PINCTRL_IMX_SCU
- bool
+ tristate "IMX SCU pinctrl driver"
depends on IMX_SCU
select PINCTRL_IMX
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 507e4af..b80c450 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -373,7 +373,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
const struct imx_pinctrl_soc_info *info = ipctl->info;
if (info->flags & IMX_USE_SCU)
- return imx_pinconf_get_scu(pctldev, pin_id, config);
+ return info->imx_pinconf_get(pctldev, pin_id, config);
else
return imx_pinconf_get_mmio(pctldev, pin_id, config);
}
@@ -423,7 +423,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
const struct imx_pinctrl_soc_info *info = ipctl->info;
if (info->flags & IMX_USE_SCU)
- return imx_pinconf_set_scu(pctldev, pin_id,
+ return info->imx_pinconf_set(pctldev, pin_id,
configs, num_configs);
else
return imx_pinconf_set_mmio(pctldev, pin_id,
@@ -440,7 +440,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev
*pctldev,
int ret;
if (info->flags & IMX_USE_SCU) {
- ret = imx_pinconf_get_scu(pctldev, pin_id, );
+ ret = info->imx_pinconf_get(pctldev, pin_id, );
if (ret) {
dev_err(ipctl->dev, "failed to get %s pinconf\n",
pin_get_name(pctldev, pin_id));
@@ -629,7 +629,7 @@ static int imx_pinctrl_parse_groups(struct device_node *np,
for (i = 0; i < grp->num_pins; i++) {
pin = &((struct imx_pin *)(grp->data))[i];
if (info->flags & IMX_USE_SCU)
- imx_pinctrl_parse_pin_scu(ipctl, >pins[i],
+ info->imx_pinctrl_parse_pin(ipctl, >pins[i],
pin, );
else
imx_pinctrl_parse_pin_mmio(ipctl, >pins[i],
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h
b/drivers/pinctrl/freescale/pinctrl-imx.h
index 333d32b..bdb86c2 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -75,6 +75,21 @@ struct imx_cfg_params_decode {
bool invert;
};
+/**
+ * @dev: a pointer back to containing device
+ * @base: the offset to the controller in virtual memory
+ */
+struct imx_pinctrl {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ void __iomem *base;
+ void __iomem *input_sel_base;
+ const struct imx_pinctrl_soc_info *info;
+ struct imx_pin_reg *pin_regs;
+ unsigned int group_index;
+ struct mutex mutex;
+};
+
struct imx_pinctrl_soc_info {
const struct pinctrl_pin_desc *pins;
unsigned int npins;
@@ -98,21 +113,13 @@ struct imx_pinctrl_soc_info {
struct pinctrl_gpio_range *range,
unsigned offset,
bool input);
-};
-
-/**
- * @dev: a pointer back to containing device
- * @base: the offset to the controller in virtual memory
- */
-struct imx_pinctrl {
-
[PATCH 2/3] firmware: imx: Save channel name for further use
From: Daniel Baluta
We want to request / free channels on demand later in order
to save power.
For this for each channel we save the name and use it to
reference the channel later.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index a3a018c87b52..b6e95d6d34c0 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -79,6 +79,7 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
return -ENOMEM;
dsp_chan = _ipc->chans[i];
+ dsp_chan->name = chan_name;
cl = _chan->cl;
cl->dev = dev;
cl->tx_block = false;
@@ -97,16 +98,14 @@ static int imx_dsp_setup_channels(struct imx_dsp_ipc
*dsp_ipc)
}
dev_dbg(dev, "request mbox chan %s\n", chan_name);
- /* chan_name is not used anymore by framework */
- kfree(chan_name);
}
return 0;
out:
- kfree(chan_name);
for (j = 0; j < i; j++) {
dsp_chan = _ipc->chans[j];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return ret;
@@ -147,6 +146,7 @@ static int imx_dsp_remove(struct platform_device *pdev)
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
dsp_chan = _ipc->chans[i];
mbox_free_channel(dsp_chan->ch);
+ kfree(dsp_chan->name);
}
return 0;
--
2.17.1
[PATCH 0/3] Allow on demand channel request / free
From: Daniel Baluta Requesting an mailbox channel will call mailbox's startup function. startup function calls pm_runtime_get_sync which increments device usage count and will keep the device active. Specifically, mailbox clock will be always ON when a mailbox channel is requested. For this, reason we introduce a way to request/free IMX DSP channels· on demand to save power when the channels are not used. First two patches are doing code refactoring preparing the path for 3rd patch which exports functions for on demand channel request/free Daniel Baluta (3): firmware: imx: Introduce imx_dsp_setup_channels firmware: imx: Save channel name for further use firmware: imx-dsp: Export functions to request/free channels drivers/firmware/imx/imx-dsp.c | 72 include/linux/firmware/imx/dsp.h | 10 + 2 files changed, 64 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH 3/3] firmware: imx-dsp: Export functions to request/free channels
From: Daniel Baluta
In order to save power, we only need to request a channel
when the communication with the DSP active.
For this we export the following functions:
- imx_dsp_request_channel, gets a channel with a given index
- imx_dsp_free_channel, frees a channel with a given index
Notice that we still request channels at probe to support devices
that do not have PM callbacks implemented.
More explanations about why requesting a channel has an effect
on power savings:
- requesting an mailbox channel will call mailbox's startup
function.
- startup function calls pm_runtime_get_sync which increments device
usage count and will keep the device active. Specifically, mailbox
clock will be always ON when a mailbox channel is requested.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 25 +
include/linux/firmware/imx/dsp.h | 10 ++
2 files changed, 35 insertions(+)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index b6e95d6d34c0..a6c06d7476c3 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,6 +60,31 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return ERR_PTR(-EINVAL);
+
+ dsp_chan = _ipc->chans[idx];
+ dsp_chan->ch = mbox_request_channel_byname(_chan->cl,
dsp_chan->name);
+ return dsp_chan->ch;
+}
+EXPORT_SYMBOL(imx_dsp_request_channel);
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *dsp_ipc, int idx)
+{
+ struct imx_dsp_chan *dsp_chan;
+
+ if (idx >= DSP_MU_CHAN_NUM)
+ return;
+
+ dsp_chan = _ipc->chans[idx];
+ mbox_free_channel(dsp_chan->ch);
+}
+EXPORT_SYMBOL(imx_dsp_free_channel);
+
static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
struct device *dev = dsp_ipc->dev;
diff --git a/include/linux/firmware/imx/dsp.h b/include/linux/firmware/imx/dsp.h
index 7562099c9e46..4f7895a3b73c 100644
--- a/include/linux/firmware/imx/dsp.h
+++ b/include/linux/firmware/imx/dsp.h
@@ -55,6 +55,9 @@ static inline void *imx_dsp_get_data(struct imx_dsp_ipc *ipc)
int imx_dsp_ring_doorbell(struct imx_dsp_ipc *dsp, unsigned int chan_idx);
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx);
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx);
+
#else
static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc *ipc,
@@ -63,5 +66,12 @@ static inline int imx_dsp_ring_doorbell(struct imx_dsp_ipc
*ipc,
return -ENOTSUPP;
}
+struct mbox_chan *imx_dsp_request_channel(struct imx_dsp_ipc *ipc, int idx)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+void imx_dsp_free_channel(struct imx_dsp_ipc *ipc, int idx) { }
+
#endif
#endif /* _IMX_DSP_IPC_H */
--
2.17.1
[PATCH 1/3] firmware: imx: Introduce imx_dsp_setup_channels
From: Daniel Baluta
Create a separate function that sets up DSP mailbox channels
so that imx_dsp_probe function will be easier to read.
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 41 +-
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index 4265e9dbed84..a3a018c87b52 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -60,22 +60,15 @@ static void imx_dsp_handle_rx(struct mbox_client *c, void
*msg)
}
}
-static int imx_dsp_probe(struct platform_device *pdev)
+static int imx_dsp_setup_channels(struct imx_dsp_ipc *dsp_ipc)
{
- struct device *dev = >dev;
- struct imx_dsp_ipc *dsp_ipc;
+ struct device *dev = dsp_ipc->dev;
struct imx_dsp_chan *dsp_chan;
struct mbox_client *cl;
char *chan_name;
int ret;
int i, j;
- device_set_of_node_from_dev(>dev, pdev->dev.parent);
-
- dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
- if (!dsp_ipc)
- return -ENOMEM;
-
for (i = 0; i < DSP_MU_CHAN_NUM; i++) {
if (i < 2)
chan_name = kasprintf(GFP_KERNEL, "txdb%d", i);
@@ -108,12 +101,6 @@ static int imx_dsp_probe(struct platform_device *pdev)
kfree(chan_name);
}
- dsp_ipc->dev = dev;
-
- dev_set_drvdata(dev, dsp_ipc);
-
- dev_info(dev, "NXP i.MX DSP IPC initialized\n");
-
return 0;
out:
kfree(chan_name);
@@ -125,6 +112,30 @@ static int imx_dsp_probe(struct platform_device *pdev)
return ret;
}
+static int imx_dsp_probe(struct platform_device *pdev)
+{
+ struct device *dev = >dev;
+ struct imx_dsp_ipc *dsp_ipc;
+ int ret;
+
+ device_set_of_node_from_dev(>dev, pdev->dev.parent);
+
+ dsp_ipc = devm_kzalloc(dev, sizeof(*dsp_ipc), GFP_KERNEL);
+ if (!dsp_ipc)
+ return -ENOMEM;
+
+ dsp_ipc->dev = dev;
+ dev_set_drvdata(dev, dsp_ipc);
+
+ ret = imx_dsp_setup_channels(dsp_ipc);
+ if (ret < 0)
+ return ret;
+
+ dev_info(dev, "NXP i.MX DSP IPC initialized\n");
+
+ return 0;
+}
+
static int imx_dsp_remove(struct platform_device *pdev)
{
struct imx_dsp_chan *dsp_chan;
--
2.17.1
Re: [PATCH] drivers: soc: Fix mailbox suspend/resume no irq for IMX SCU
On 06.07.2020 18:00, Vincenzo Frascino wrote: imx_mu_suspend_noirq()/imx_mu_resume_noirq() are currently used only when CONFIG_PM_SLEEP configuration options is enabled. Having it disabled triggers the following warning at compile time: drivers/mailbox/imx-mailbox.c:611:12: warning: ‘imx_mu_resume_noirq’ defined but not used [-Wunused-function] static int imx_mu_resume_noirq(struct device *dev) drivers/mailbox/imx-mailbox.c:601:12: warning: ‘imx_mu_suspend_noirq’ defined but not used [-Wunused-function] static int imx_mu_suspend_noirq(struct device *dev) Make imx_mu_suspend_noirq()/imx_mu_resume_noirq() __maybe_unused to address the issue. Cc: Jassi Brar Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Signed-off-by: Vincenzo Frascino Reviewed-by: Daniel Baluta
[PATCH v3 2/2] ASoC: SOF: Use multi PM domains helpers
From: Daniel Baluta
Use dev_multi_pm_attach / dev_multi_pm_detach instead of the hardcoded
version.
Signed-off-by: Daniel Baluta
---
sound/soc/sof/imx/imx8.c | 60 ++--
1 file changed, 9 insertions(+), 51 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index 68b2edccd791..7229993c5af8 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -51,10 +51,7 @@ struct imx8_priv {
struct imx_sc_ipc *sc_ipc;
/* Power domain handling */
- int num_domains;
- struct device **pd_dev;
- struct device_link **link;
-
+ struct dev_multi_pm_domain_data *mpd;
};
static void imx8_get_reply(struct snd_sof_dev *sdev)
@@ -207,7 +204,6 @@ static int imx8_probe(struct snd_sof_dev *sdev)
struct resource res;
u32 base, size;
int ret = 0;
- int i;
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -218,45 +214,15 @@ static int imx8_probe(struct snd_sof_dev *sdev)
priv->sdev = sdev;
/* power up device associated power domains */
- priv->num_domains = of_count_phandle_with_args(np, "power-domains",
- "#power-domain-cells");
- if (priv->num_domains < 0) {
- dev_err(sdev->dev, "no power-domains property in %pOF\n", np);
- return priv->num_domains;
- }
-
- priv->pd_dev = devm_kmalloc_array(>dev, priv->num_domains,
- sizeof(*priv->pd_dev), GFP_KERNEL);
- if (!priv->pd_dev)
- return -ENOMEM;
-
- priv->link = devm_kmalloc_array(>dev, priv->num_domains,
- sizeof(*priv->link), GFP_KERNEL);
- if (!priv->link)
- return -ENOMEM;
-
- for (i = 0; i < priv->num_domains; i++) {
- priv->pd_dev[i] = dev_pm_domain_attach_by_id(>dev, i);
- if (IS_ERR(priv->pd_dev[i])) {
- ret = PTR_ERR(priv->pd_dev[i]);
- goto exit_unroll_pm;
- }
- priv->link[i] = device_link_add(>dev, priv->pd_dev[i],
- DL_FLAG_STATELESS |
- DL_FLAG_PM_RUNTIME |
- DL_FLAG_RPM_ACTIVE);
- if (!priv->link[i]) {
- ret = -ENOMEM;
- dev_pm_domain_detach(priv->pd_dev[i], false);
- goto exit_unroll_pm;
- }
- }
+ priv->mpd = dev_multi_pm_attach(>dev);
+ if (IS_ERR(priv->mpd))
+ return PTR_ERR(priv->mpd);
ret = imx_scu_get_handle(>sc_ipc);
if (ret) {
dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",
ret);
- goto exit_unroll_pm;
+ goto exit_detach_pm;
}
priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
@@ -264,7 +230,7 @@ static int imx8_probe(struct snd_sof_dev *sdev)
pdev, sizeof(*pdev));
if (IS_ERR(priv->ipc_dev)) {
ret = PTR_ERR(priv->ipc_dev);
- goto exit_unroll_pm;
+ goto exit_detach_pm;
}
priv->dsp_ipc = dev_get_drvdata(>ipc_dev->dev);
@@ -328,26 +294,18 @@ static int imx8_probe(struct snd_sof_dev *sdev)
exit_pdev_unregister:
platform_device_unregister(priv->ipc_dev);
-exit_unroll_pm:
- while (--i >= 0) {
- device_link_del(priv->link[i]);
- dev_pm_domain_detach(priv->pd_dev[i], false);
- }
-
+exit_detach_pm:
+ dev_multi_pm_detach(priv->mpd);
return ret;
}
static int imx8_remove(struct snd_sof_dev *sdev)
{
struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
- int i;
platform_device_unregister(priv->ipc_dev);
- for (i = 0; i < priv->num_domains; i++) {
- device_link_del(priv->link[i]);
- dev_pm_domain_detach(priv->pd_dev[i], false);
- }
+ dev_multi_pm_detach(priv->mpd);
return 0;
}
--
2.17.1
[PATCH v3 1/2] PM / domains: Introduce multi PM domains helpers
From: Daniel Baluta
This patch introduces helpers support for multi PM domains.
API consists of:
1) dev_multi_pm_attach - powers up all PM domains associated with a given
device. Because we can attach one PM domain per device, we create
virtual devices (children of initial device) and associate PM domains
one per virtual device.
2) dev_multi_pm_detach - detaches all virtual devices from PM domains
attached with.
Signed-off-by: Daniel Baluta
---
drivers/base/power/common.c | 93 +
include/linux/pm_domain.h | 19
2 files changed, 112 insertions(+)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index bbddb267c2e6..b0a4d0109810 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -228,3 +228,96 @@ void dev_pm_domain_set(struct device *dev, struct
dev_pm_domain *pd)
device_pm_check_callbacks(dev);
}
EXPORT_SYMBOL_GPL(dev_pm_domain_set);
+
+/**
+ * dev_multi_pm_attach - power up device associated power domains
+ * @dev: The device used to lookup the PM domains
+ *
+ * Parse device's OF node to find all PM domains specifiers. For each power
+ * domain found, create a virtual device and associate it with the
+ * current power domain.
+ *
+ * This function should typically be invoked by a driver during the
+ * probe phase, in the case its device requires power management through
+ * multiple PM domains.
+ *
+ * Returns a pointer to @dev_multi_pm_domain_data if successfully attached PM
+ * domains, NULL when the device doesn't need a PM domain or when single
+ * power-domains exists for it, else an ERR_PTR() in case of
+ * failures.
+ */
+struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev)
+{
+ struct dev_multi_pm_domain_data *mpd, *retp;
+ int num_domains;
+ int i;
+
+ num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
+"#power-domain-cells");
+ if (num_domains < 2)
+ return NULL;
+
+ mpd = devm_kzalloc(dev, sizeof(*mpd), GFP_KERNEL);
+ if (!mpd)
+ return ERR_PTR(-ENOMEM);
+
+ mpd->dev = dev;
+ mpd->num_domains = num_domains;
+
+ mpd->virt_devs = devm_kmalloc_array(dev, mpd->num_domains,
+ sizeof(*mpd->virt_devs),
+ GFP_KERNEL);
+ if (!mpd->virt_devs)
+ return ERR_PTR(-ENOMEM);
+
+ mpd->links = devm_kmalloc_array(dev, mpd->num_domains,
+ sizeof(*mpd->links), GFP_KERNEL);
+ if (!mpd->links)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < mpd->num_domains; i++) {
+ mpd->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i);
+ if (IS_ERR(mpd->virt_devs[i])) {
+ retp = (struct dev_multi_pm_domain_data *)
+ mpd->virt_devs[i];
+ goto exit_unroll_pm;
+ }
+ mpd->links[i] = device_link_add(dev, mpd->virt_devs[i],
+ DL_FLAG_STATELESS |
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE);
+ if (!mpd->links[i]) {
+ retp = ERR_PTR(-ENOMEM);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ goto exit_unroll_pm;
+ }
+ }
+ return mpd;
+
+exit_unroll_pm:
+ while (--i >= 0) {
+ device_link_del(mpd->links[i]);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ }
+
+ return retp;
+}
+EXPORT_SYMBOL(dev_multi_pm_attach);
+
+/**
+ * dev_multi_pm_detach - Detach a device from its PM domains.
+ * Each multi power domain is attached to a virtual children device
+ *
+ * @mpd: multi power domains data, contains the association between
+ * virtul device and PM domain
+ */
+void dev_multi_pm_detach(struct dev_multi_pm_domain_data *mpd)
+{
+ int i;
+
+ for (i = 0; i < mpd->num_domains; i++) {
+ device_link_del(mpd->links[i]);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ }
+}
+EXPORT_SYMBOL(dev_multi_pm_detach);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9ec78ee53652..af1107731044 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -183,6 +183,13 @@ struct generic_pm_domain_data {
void *data;
};
+struct dev_multi_pm_domain_data {
+ struct device *dev; /* parent device */
+ struct device **virt_devs; /* virtual children links */
+ struct device_link **links; /* links parent <-> virtual children */
+ int num_domains;
+};
+
#ifdef C
[PATCH v3 0/2] Introduce multi PM domains helpers
From: Daniel Baluta i.MX8QXP/i.MX8QM has IPs that need multiple power domains to be up in order to work. In order to help drivers, we introduce multi PM domains helpers that are able to activate/deactivate multi PM domains. First patch introduces the helpers and second patch demonstrates how a driver can use them instead of hardcoding the PM domains handling. Changes since v2: ?? - fix kernel test robot reported issues (missing static inline for newly introduced functions in headers and arguments swapped for devm_kzalloc). Changes since v1: (addressed Ranjani's comments) ?? - enhanced description for dev_multi_pm_attach return value ?? - renamed exit_unroll_pm label to exit_detach_pm Ideally would be to have patch 1/2 merged via power tree and then I will submit again patch 2/2 on sound tree. Daniel Baluta (2): PM / domains: Introduce multi PM domains helpers ASoC: SOF: Use multi PM domains helpers drivers/base/power/common.c | 93 + include/linux/pm_domain.h | 19 sound/soc/sof/imx/imx8.c| 60 3 files changed, 121 insertions(+), 51 deletions(-) -- 2.17.1
[RESEND PATCH v2 0/2] Introduce multi PM domains helpers
From: Daniel Baluta i.MX8QXP/i.MX8QM has IPs that need multiple power domains to be up in order to work. In order to help drivers, we introduce multi PM domains helpers that are able to activate/deactivate multi PM domains. First patch introduces the helpers and second patch demonstrates how a driver can use them instead of hardcoding the PM domains handling. Changes since v1: (addressed Ranjani's comments) - enhanced description for dev_multi_pm_attach return value - renamed exit_unroll_pm label to exit_detach_pm Ideally would be to have patch 1/2 merged via power tree and then I will submit again patch 2/2 on sound tree. Daniel Baluta (2): PM / domains: Introduce multi PM domains helpers ASoC: SOF: Use multi PM domains helpers drivers/base/power/common.c | 93 + include/linux/pm_domain.h | 19 sound/soc/sof/imx/imx8.c| 60 3 files changed, 121 insertions(+), 51 deletions(-) -- 2.17.1
[RESEND PATCH v2 2/2] ASoC: SOF: Use multi PM domains helpers
From: Daniel Baluta
Use dev_multi_pm_attach / dev_multi_pm_detach instead of the hardcoded
version.
Signed-off-by: Daniel Baluta
---
sound/soc/sof/imx/imx8.c | 60 ++--
1 file changed, 9 insertions(+), 51 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index b692752b2178..2e7635b697cf 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -51,10 +51,7 @@ struct imx8_priv {
struct imx_sc_ipc *sc_ipc;
/* Power domain handling */
- int num_domains;
- struct device **pd_dev;
- struct device_link **link;
-
+ struct dev_multi_pm_domain_data *mpd;
};
static void imx8_get_reply(struct snd_sof_dev *sdev)
@@ -207,7 +204,6 @@ static int imx8_probe(struct snd_sof_dev *sdev)
struct resource res;
u32 base, size;
int ret = 0;
- int i;
priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -218,45 +214,15 @@ static int imx8_probe(struct snd_sof_dev *sdev)
priv->sdev = sdev;
/* power up device associated power domains */
- priv->num_domains = of_count_phandle_with_args(np, "power-domains",
- "#power-domain-cells");
- if (priv->num_domains < 0) {
- dev_err(sdev->dev, "no power-domains property in %pOF\n", np);
- return priv->num_domains;
- }
-
- priv->pd_dev = devm_kmalloc_array(>dev, priv->num_domains,
- sizeof(*priv->pd_dev), GFP_KERNEL);
- if (!priv->pd_dev)
- return -ENOMEM;
-
- priv->link = devm_kmalloc_array(>dev, priv->num_domains,
- sizeof(*priv->link), GFP_KERNEL);
- if (!priv->link)
- return -ENOMEM;
-
- for (i = 0; i < priv->num_domains; i++) {
- priv->pd_dev[i] = dev_pm_domain_attach_by_id(>dev, i);
- if (IS_ERR(priv->pd_dev[i])) {
- ret = PTR_ERR(priv->pd_dev[i]);
- goto exit_unroll_pm;
- }
- priv->link[i] = device_link_add(>dev, priv->pd_dev[i],
- DL_FLAG_STATELESS |
- DL_FLAG_PM_RUNTIME |
- DL_FLAG_RPM_ACTIVE);
- if (!priv->link[i]) {
- ret = -ENOMEM;
- dev_pm_domain_detach(priv->pd_dev[i], false);
- goto exit_unroll_pm;
- }
- }
+ priv->mpd = dev_multi_pm_attach(>dev);
+ if (IS_ERR(priv->mpd))
+ return PTR_ERR(priv->mpd);
ret = imx_scu_get_handle(>sc_ipc);
if (ret) {
dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",
ret);
- goto exit_unroll_pm;
+ goto exit_detach_pm;
}
priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
@@ -264,7 +230,7 @@ static int imx8_probe(struct snd_sof_dev *sdev)
pdev, sizeof(*pdev));
if (IS_ERR(priv->ipc_dev)) {
ret = PTR_ERR(priv->ipc_dev);
- goto exit_unroll_pm;
+ goto exit_detach_pm;
}
priv->dsp_ipc = dev_get_drvdata(>ipc_dev->dev);
@@ -328,26 +294,18 @@ static int imx8_probe(struct snd_sof_dev *sdev)
exit_pdev_unregister:
platform_device_unregister(priv->ipc_dev);
-exit_unroll_pm:
- while (--i >= 0) {
- device_link_del(priv->link[i]);
- dev_pm_domain_detach(priv->pd_dev[i], false);
- }
-
+exit_detach_pm:
+ dev_multi_pm_detach(priv->mpd);
return ret;
}
static int imx8_remove(struct snd_sof_dev *sdev)
{
struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
- int i;
platform_device_unregister(priv->ipc_dev);
- for (i = 0; i < priv->num_domains; i++) {
- device_link_del(priv->link[i]);
- dev_pm_domain_detach(priv->pd_dev[i], false);
- }
+ dev_multi_pm_detach(priv->mpd);
return 0;
}
--
2.17.1
[RESEND PATCH v2 1/2] PM / domains: Introduce multi PM domains helpers
From: Daniel Baluta
This patch introduces helpers support for multi PM domains.
API consists of:
1) dev_multi_pm_attach - powers up all PM domains associated with a given
device. Because we can attach one PM domain per device, we create
virtual devices (children of initial device) and associate PM domains
one per virtual device.
2) dev_multi_pm_detach - detaches all virtual devices from PM domains
attached with.
Signed-off-by: Daniel Baluta
---
drivers/base/power/common.c | 93 +
include/linux/pm_domain.h | 19
2 files changed, 112 insertions(+)
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index bbddb267c2e6..6d1f142833b1 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -228,3 +228,96 @@ void dev_pm_domain_set(struct device *dev, struct
dev_pm_domain *pd)
device_pm_check_callbacks(dev);
}
EXPORT_SYMBOL_GPL(dev_pm_domain_set);
+
+/**
+ * dev_multi_pm_attach - power up device associated power domains
+ * @dev: The device used to lookup the PM domains
+ *
+ * Parse device's OF node to find all PM domains specifiers. For each power
+ * domain found, create a virtual device and associate it with the
+ * current power domain.
+ *
+ * This function should typically be invoked by a driver during the
+ * probe phase, in the case its device requires power management through
+ * multiple PM domains.
+ *
+ * Returns a pointer to @dev_multi_pm_domain_data if successfully attached PM
+ * domains, NULL when the device doesn't need a PM domain or when single
+ * power-domains exists for it, else an ERR_PTR() in case of
+ * failures.
+ */
+struct dev_multi_pm_domain_data *dev_multi_pm_attach(struct device *dev)
+{
+ struct dev_multi_pm_domain_data *mpd, *retp;
+ int num_domains;
+ int i;
+
+ num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
+"#power-domain-cells");
+ if (num_domains < 2)
+ return NULL;
+
+ mpd = devm_kzalloc(dev, GFP_KERNEL, sizeof(*mpd));
+ if (!mpd)
+ return ERR_PTR(-ENOMEM);
+
+ mpd->dev = dev;
+ mpd->num_domains = num_domains;
+
+ mpd->virt_devs = devm_kmalloc_array(dev, mpd->num_domains,
+ sizeof(*mpd->virt_devs),
+ GFP_KERNEL);
+ if (!mpd->virt_devs)
+ return ERR_PTR(-ENOMEM);
+
+ mpd->links = devm_kmalloc_array(dev, mpd->num_domains,
+ sizeof(*mpd->links), GFP_KERNEL);
+ if (!mpd->links)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < mpd->num_domains; i++) {
+ mpd->virt_devs[i] = dev_pm_domain_attach_by_id(dev, i);
+ if (IS_ERR(mpd->virt_devs[i])) {
+ retp = (struct dev_multi_pm_domain_data *)
+ mpd->virt_devs[i];
+ goto exit_unroll_pm;
+ }
+ mpd->links[i] = device_link_add(dev, mpd->virt_devs[i],
+ DL_FLAG_STATELESS |
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE);
+ if (!mpd->links[i]) {
+ retp = ERR_PTR(-ENOMEM);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ goto exit_unroll_pm;
+ }
+ }
+ return mpd;
+
+exit_unroll_pm:
+ while (--i >= 0) {
+ device_link_del(mpd->links[i]);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ }
+
+ return retp;
+}
+EXPORT_SYMBOL(dev_multi_pm_attach);
+
+/**
+ * dev_multi_pm_detach - Detach a device from its PM domains.
+ * Each multi power domain is attached to a virtual children device
+ *
+ * @mpd: multi power domains data, contains the association between
+ * virtul device and PM domain
+ */
+void dev_multi_pm_detach(struct dev_multi_pm_domain_data *mpd)
+{
+ int i;
+
+ for (i = 0; i < mpd->num_domains; i++) {
+ device_link_del(mpd->links[i]);
+ dev_pm_domain_detach(mpd->virt_devs[i], false);
+ }
+}
+EXPORT_SYMBOL(dev_multi_pm_detach);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9ec78ee53652..5bcb35150af2 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -183,6 +183,13 @@ struct generic_pm_domain_data {
void *data;
};
+struct dev_multi_pm_domain_data {
+ struct device *dev; /* parent device */
+ struct device **virt_devs; /* virtual children links */
+ struct device_link **links; /* links parent <-> virtual children */
+ int num_domains;
+};
+
#ifdef C
Re: [PATCH AUTOSEL 5.7 055/388] ASoC: SOF: Do nothing when DSP PM callbacks are not set
On 6/18/20 2:01 PM, Mark Brown wrote: On Wed, Jun 17, 2020 at 09:02:32PM -0400, Sasha Levin wrote: From: Daniel Baluta [ Upstream commit c26fde3b15ed41f5f452f1da727795f787833287 ] This provides a better separation between runtime and PM sleep callbacks. Only do nothing if given runtime flag is set and calback is not set. With the current implementation, if PM sleep callback is set but runtime callback is not set then at runtime resume we reload the firmware even if we do not support runtime resume callback. This doesn't look like a bugfix, just an optimization? Indeed can be seen as an optimization, but it does unexpected things which can cause trouble and weird behavior for people not familiar with the matter. For example, as explained in the commit message if you only provide System PM handler but not runtime PM handler, then the DSP will be resetted even if this is not the intention.
Re: [PATCH V4 4/9] pinctrl: imx8mn: Support building as module
On 11.06.2020 11:48, Anson Huang wrote: Hi, Daniel Subject: Re: [PATCH V4 4/9] pinctrl: imx8mn: Support building as module Maybe this is obvious but I would really like to see an explanation of why we are switching from arch_initcall to platform_init. Commit message act as documentation for the reviewers. Yes, I noticed this, and it looks like unnecessary change, and I just replied in mail that I will change it back to arch_initcall() in order to make sure nothing changed for built-in config. Below is what I replied in cover-letter mail: " I will keep the arch_initcall() there in next version patch series, it can make sure the change does NOT impact built-in config. For module build, the arch_initcall() will be same as module_init(), user needs to insmod the .ko with correct sequence." Ok, that's great. Lets try to keep in mind that the commit message should answer to a simple question: "Why the change is needed" :). Then, the details about the change should be added.
Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module
On 11.06.2020 11:44, Anson Huang wrote: Hi, Daniel Subject: Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module Hi Anson, Patch series mostly looks good to me. I have a comment about adding the MODULE_LICENSE. This is a pretty important change. Can you please add this change in a separate patch with a proper explanation of why it is needed. Most likely it is because it was forgotten in the previous patches. Yes, it is obviously missed in the previous patches, as previously these pinctrl drivers do NOT support module build at all. And MODULE_LICENSE is a MUST when drivers supporting module build, build will report failure if module license missed, so I think it is also part of the module build support patch, do you mean it is better to add a separate patch to add the MODULE_LICENSE to all pinctrl drivers missing it? Maybe we can get more opinion from maintainer, I am NOT very sure whether it is better to separate the module license as a single patch Hi Anson, This is my feeling. That the first patch in the series should add the MODULE_LICENSE to all of files. This keeps things simple, explains our intention with next patches. Daniel.
Re: [PATCH V4 4/9] pinctrl: imx8mn: Support building as module
Maybe this is obvious but I would really like to see an explanation
of why we are switching from arch_initcall to platform_init.
Commit message act as documentation for the reviewers.
On 10.06.2020 10:57, Anson Huang wrote:
Support building i.MX8MN pinctrl driver as module.
Signed-off-by: Anson Huang
---
No change.
---
drivers/pinctrl/freescale/Kconfig | 2 +-
drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 --
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/pinctrl/freescale/Kconfig
b/drivers/pinctrl/freescale/Kconfig
index 556adc3..fe3e49c 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -132,7 +132,7 @@ config PINCTRL_IMX8MM
Say Y here to enable the imx8mm pinctrl driver
config PINCTRL_IMX8MN
- bool "IMX8MN pinctrl driver"
+ tristate "IMX8MN pinctrl driver"
depends on ARCH_MXC
select PINCTRL_IMX
help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mn.c
b/drivers/pinctrl/freescale/pinctrl-imx8mn.c
index 100ed8c..b6db780 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mn.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mn.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -326,6 +327,7 @@ static const struct of_device_id imx8mn_pinctrl_of_match[]
= {
{ .compatible = "fsl,imx8mn-iomuxc", .data = _pinctrl_info, },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, imx8mn_pinctrl_of_match);
static int imx8mn_pinctrl_probe(struct platform_device *pdev)
{
@@ -340,9 +342,5 @@ static struct platform_driver imx8mn_pinctrl_driver = {
},
.probe = imx8mn_pinctrl_probe,
};
-
-static int __init imx8mn_pinctrl_init(void)
-{
- return platform_driver_register(_pinctrl_driver);
-}
-arch_initcall(imx8mn_pinctrl_init);
+module_platform_driver(imx8mn_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module
Hi Anson, Patch series mostly looks good to me. I have a comment about adding the MODULE_LICENSE. This is a pretty important change. Can you please add this change in a separate patch with a proper explanation of why it is needed. Most likely it is because it was forgotten in the previous patches. thanks, daniel. On 10.06.2020 10:57, Anson Huang wrote: There are more and mroe requirements that SoC specific modules should be built as module in order to support generic kernel image, such as Android GKI concept. This patch series supports i.MX8 SoCs pinctrl drivers to be built as module, including i.MX8MQ/MM/MN/MP/QXP/QM/DXL SoCs, and it also supports building i.MX common pinctrl driver and i.MX SCU common pinctrl driver as module. Compared to V3, the changes are as below: - change the config dependency back to original; - use function callbacks for SCU related functions, and all drivers using SCU pinctrl driver need to initialize the function callbacks, pinctrl-imx.c will check the SCU function callback and call it when it is valid, then no build issue when PINCTRL_IMX is built in and PINCTRL_IMX_SCU is built as module. Anson Huang (9): pinctrl: imx: Support building SCU pinctrl driver as module pinctrl: imx: Support building i.MX pinctrl driver as module pinctrl: imx8mm: Support building as module pinctrl: imx8mn: Support building as module pinctrl: imx8mq: Support building as module pinctrl: imx8mp: Support building as module pinctrl: imx8qxp: Support building as module pinctrl: imx8qm: Support building as module pinctrl: imx8dxl: Support building as module drivers/pinctrl/freescale/Kconfig | 19 +- drivers/pinctrl/freescale/pinctrl-imx.c | 22 ++- drivers/pinctrl/freescale/pinctrl-imx.h | 57 - drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 12 +++--- drivers/pinctrl/freescale/pinctrl-imx8mm.c | 10 ++--- drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 ++--- drivers/pinctrl/freescale/pinctrl-imx8mp.c | 10 ++--- drivers/pinctrl/freescale/pinctrl-imx8mq.c | 9 ++--- drivers/pinctrl/freescale/pinctrl-imx8qm.c | 12 +++--- drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 12 +++--- drivers/pinctrl/freescale/pinctrl-scu.c | 6 +++ 11 files changed, 86 insertions(+), 93 deletions(-)
Re: [PATCH] soc: imx: scu: use devm_kasprintf
On 03.06.2020 12:29, peng@nxp.com wrote: From: Peng Fan Use devm_kasprintf to simplify code Signed-off-by: Peng Fan Reviewed-by: Daniel Baluta
Re: [PATCH] arm64: dts: imx8mm-beacon: Fix voltages on LDO1 and LDO2
On Tue, May 26, 2020 at 8:11 PM Adam Ford wrote:
>
> LDO1 and LDO2 settings are wrong and case the voltage to go above the
> maximum level of 2.15V permitted by the SoC to 3.0V.
>
> This patch is based on work done on the i.MX8M Mini-EVK which utilizes
> the same fix.
>
> Fixes: 593816fa2f35 ("arm64: dts: imx: Add Beacon i.MX8m-Mini development
> kit")
>
> Signed-off-by: Adam Ford
No need for a new line between fixes and signed-off-by.
With that:
Reviewed-by: Daniel Baluta
Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
On Thu, May 14, 2020 at 7:02 PM Martin Kepplinger wrote: > > From: "Angus Ainslie (Purism)" > > Add a devicetree description for the Librem 5 phone. The early batches > that have been sold are supported as well as the mass-produced device > available later this year, see https://puri.sm/products/librem-5/ > > This boots to a working console with working WWAN modem, wifi usdhc, > IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs. > > Signed-off-by: Martin Kepplinger > Signed-off-by: Angus Ainslie (Purism) > Signed-off-by: Guido Günther For audio related part: Reviewed-by: Daniel Baluta
Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
On 22.05.2020 17:26, Jonathan Albrieux wrote: On Fri, May 22, 2020 at 01:47:21PM +0300, Daniel Baluta wrote: + +maintainers: + - can't find a mantainer, author is Daniel Baluta Daniel is still active in the kernel, just not at Intel any more. +CC Oh ok thank you! Daniel are you still maintaining this driver? I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants to take this over, will be more than happy. Other than that we can add my gmail address: Daniel Baluta Well if you'd like to review this patch I'd really appreciate :-) Forgive me for not having understood your answer regarding the maintainer field, can I add you to this binding as maintainer or are you saying to not add you? Thank you and sorry for the repeated question, OK, so I think would be better not to add me as a maintainer because this would set some expecation from people, and I most likely won't have time to met them. Can you instead add the linux-iio mailing list as a maintainer, not sure if this is a common practice though.
Re: [PATCH v2 1/4] dt-bindings: iio: imu: bmi160: convert txt format to yaml
+ +maintainers: + - can't find a mantainer, author is Daniel Baluta Daniel is still active in the kernel, just not at Intel any more. +CC Oh ok thank you! Daniel are you still maintaining this driver? I can do reviews if requested but I'm not actively maintaining this driver. If anyone wants to take this over, will be more than happy. Other than that we can add my gmail address: Daniel Baluta
Re: [RFC PATCH 1/4] drm/etnaviv: Prevent IRQ triggering at probe time on i.MX8MM
On 4/30/20 3:46 PM, Schrempf Frieder wrote:
+ /*
+* On i.MX8MM there is an interrupt getting triggered immediately
+* after requesting the IRQ, which leads to a stall as the handler
+* accesses the GPU registers whithout the clock being enabled.
+* Enabling the clocks briefly seems to clear the IRQ state, so we do
+* this here before requesting the IRQ.
+*/
+ err = etnaviv_gpu_clk_enable(gpu);
+ if (err)
+ return err;
+
+ err = etnaviv_gpu_clk_disable(gpu);
+ if (err)
+ return err;
+
+ err = devm_request_irq(>dev, gpu->irq, irq_handler, 0,
+ dev_name(gpu->dev), gpu);
+ if (err) {
+ dev_err(dev, "failed to request IRQ%u: %d\n", gpu->irq, err);
+ return err;
+ }
Shouldn't you disable the clk after devm_request_irq is called?
Re: [PATCH] ASoC: SOF: sort out Kconfig, again
Hi Arnd, Pierre,
Thanks for looking at this.
> >>> Thanks Arnd, do you mind sharing your config?
> >>
> >> https://pastebin.com/HRX5xi3R
> >
> > will give it a try, thanks!
> >
> >>> We noticed last week that
> >>> there's a depend/select confusion might be simpler to fix, see
> >>> https://github.com/thesofproject/linux/pull/2047/commits
> >>>
> >>> If I look at the first line I see a IMX_DSP=n which looks exactly like
> >>> what we wanted to fix.
> >>
> >> Yes, I think that fix addresses the build warning as well, but looking
> >> more closely I don't think it's what you want: If you do this on
> >> a config that has the IMX_DSP disabled, it would appear to the
> >> user that you have enabled the drivers, but the actual code is still
> >> disabled.
> >
> > Are you sure? we added a select IMX_DSP, so not sure how it can be
> > disabled?
>
> I just tested Arnd's config with the patch we came up with for SOF
> (attached) and it makes the unmet dependency go away and builds fine.
> the problem is really using select IMX_DSP if it can be disabled by
> something else. My proposal looks simpler but I will agree it's not
> necessarily super elegant to move the dependency on IMX_BOX into SOF, so
> no sustained objection from me on Arnd's proposal.
>
> Daniel, this is your part of SOF, please chime in.
I would go in favor of Arnd's patch as it gets rid of exposing
IMX_MBOX into SOF.
The code will be fine even IMX_DSP=n, because the exported functions
used by SOF have dummy implementations in case IMX_DSP is not selected.
One concern is that we could end up in a case where IMX_DSP={y|m} but
IMX_MBOX=n.
Technically this is not possible because IMX_DSP depends on IMX_MBOX. So,
one cannot generate such a .config file from menuconfig interface.
You can add my:
Acked-by: Daniel Baluta
Re: [PATCH] soc: imx8m: Make imx8m_dsp_ops static
On Sat, Apr 25, 2020 at 11:03 AM ChenTao wrote: > > Fix the following warning: > > sound/soc/sof/imx/imx8m.c:95:20: warning: > symbol 'imx8m_dsp_ops' was not declared. Should it be static? > > Reported-by: Hulk Robot > Signed-off-by: ChenTao Reviewed-by: Daniel Baluta Can you please resend (picking Acked-by/Reviewed-by tags) and as Kai said add: To: Mark Brown (broo...@kernel.org Cc: alsa-de...@alsa-project.org
Re: [PATCH 1/2] arm64: dts: imx8mm: Remove duplicated machine compatible
On Wed, 2019-10-23 at 14:34 +0800, Anson Huang wrote:
> Machine compatible string normally is located in board DT, remove
> the duplicated one from SoC dtsi.
>
> Signed-off-by: Anson Huang
Reviewed-by: Daniel Baluta
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index 9258150..5ff9b6b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -12,7 +12,6 @@
> #include "imx8mm-pinfunc.h"
>
> / {
> - compatible = "fsl,imx8mm";
> interrupt-parent = <>;
> #address-cells = <2>;
> #size-cells = <2>;
Re: [PATCH 2/2] arm64: dts: imx8mn: Remove duplicated machine compatible
On Wed, 2019-10-23 at 14:34 +0800, Anson Huang wrote:
> Machine compatible string normally is located in board DT, remove
> the duplicated one from SoC dtsi.
>
> Signed-off-by: Anson Huang
Reviewed-by: Daniel Baluta
> ---
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index 46c218e..7341549 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -11,7 +11,6 @@
> #include "imx8mn-pinfunc.h"
>
> / {
> - compatible = "fsl,imx8mn";
> interrupt-parent = <>;
> #address-cells = <2>;
> #size-cells = <2>;
Re: [PATCH] ARM64: dts: imx8mm-evk: Assigned clocks for audio plls
On Wed, 2019-10-16 at 10:36 +, S.j. Wang wrote:
> 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
Reviewed-by: Daniel Baluta
> ---
> 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 {
Re: linux-next: build failure after merge of the sound-asoc tree
Hi Stephen,
On Fri, Oct 11, 2019 at 3:04 AM Stephen Rothwell wrote:
>
> Hi all,
>
> After merging the sound-asoc tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> In file included from include/sound/sof/dai-imx.h:11,
> from :
> include/sound/sof/header.h:125:2: error: unknown type name 'uint32_t'
> 125 | uint32_t size; /**< size of structure */
> | ^~~~
> include/sound/sof/header.h:136:2: error: unknown type name 'uint32_t'
> 136 | uint32_t size; /**< size of structure */
> | ^~~~
> include/sound/sof/header.h:137:2: error: unknown type name 'uint32_t'
> 137 | uint32_t cmd; /**< SOF_IPC_GLB_ + cmd */
> | ^~~~
> include/sound/sof/header.h:146:2: error: unknown type name 'int32_t'
> 146 | int32_t error; /**< negative error numbers */
> | ^~~
> include/sound/sof/header.h:160:2: error: unknown type name 'uint32_t'
> 160 | uint32_t count; /**< count of 0 means end of compound sequence */
> | ^~~~
> include/sound/sof/header.h:167:2: error: unknown type name 'uint32_t'
> 167 | uint32_t arch; /* Identifier of architecture */
> | ^~~~
> include/sound/sof/header.h:168:2: error: unknown type name 'uint32_t'
> 168 | uint32_t totalsize; /* Total size of oops message */
> | ^~~~
> include/sound/sof/header.h:175:2: error: unknown type name 'uint32_t'
> 175 | uint32_t configidhi; /* ConfigID hi 32bits */
> | ^~~~
> include/sound/sof/header.h:176:2: error: unknown type name 'uint32_t'
> 176 | uint32_t configidlo; /* ConfigID lo 32bits */
> | ^~~~
> include/sound/sof/header.h:177:2: error: unknown type name 'uint32_t'
> 177 | uint32_t numaregs; /* Special regs num */
> | ^~~~
> include/sound/sof/header.h:178:2: error: unknown type name 'uint32_t'
> 178 | uint32_t stackoffset; /* Offset to stack pointer from beginning of
> | ^~~~
> include/sound/sof/header.h:181:2: error: unknown type name 'uint32_t'
> 181 | uint32_t stackptr; /* Stack ptr */
> | ^~~~
> In file included from :
> include/sound/sof/dai-imx.h:18:2: error: unknown type name 'uint16_t'
>18 | uint16_t reserved1;
> | ^~~~
> include/sound/sof/dai-imx.h:19:2: error: unknown type name 'uint16_t'
>19 | uint16_t mclk_id;
> | ^~~~
> include/sound/sof/dai-imx.h:20:2: error: unknown type name 'uint32_t'
>20 | uint32_t mclk_direction;
> | ^~~~
> include/sound/sof/dai-imx.h:22:2: error: unknown type name 'uint32_t'
>22 | uint32_t mclk_rate; /* MCLK frequency in Hz */
> | ^~~~
> include/sound/sof/dai-imx.h:23:2: error: unknown type name 'uint32_t'
>23 | uint32_t fsync_rate; /* FSYNC frequency in Hz */
> | ^~~~
> include/sound/sof/dai-imx.h:24:2: error: unknown type name 'uint32_t'
>24 | uint32_t bclk_rate; /* BCLK frequency in Hz */
> | ^~~~
> include/sound/sof/dai-imx.h:27:2: error: unknown type name 'uint32_t'
>27 | uint32_t tdm_slots;
> | ^~~~
> include/sound/sof/dai-imx.h:28:2: error: unknown type name 'uint32_t'
>28 | uint32_t rx_slots;
> | ^~~~
> include/sound/sof/dai-imx.h:29:2: error: unknown type name 'uint32_t'
>29 | uint32_t tx_slots;
> | ^~~~
> include/sound/sof/dai-imx.h:30:2: error: unknown type name 'uint16_t'
>30 | uint16_t tdm_slot_width;
> | ^~~~
> include/sound/sof/dai-imx.h:31:2: error: unknown type name 'uint16_t'
>31 | uint16_t reserved2; /* alignment */
> | ^~~~
>
> Caused by commit
>
> b4be427683cf ("ASoC: SOF: imx: Describe ESAI parameters to be sent to DSP")
>
> I added the following fix for today (include/sound/sof/header.h
> probably should have something similar):
Thanks for doing this! Is this patch in linux-next already? I couldn't find it.
For include/sound/sof/header.h Morimoto-san sent a patch to alsa-devel.
>
> From: Stephen Rothwell
> Date: Fri, 11 Oct 2019 10:56:46 +1100
> Subject: [PATCH] ASOC: SOF: dai-imx.h needs linux/types.h
>
> Signed-off-by: Stephen Rothwell
> ---
> include/sound/sof/dai-imx.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/sound/sof/dai-imx.h b/include/sound/sof/dai-imx.h
> index e02fb0b0fae1..31ccb87a8273 100644
> --- a/include/sound/sof/dai-imx.h
> +++ b/include/sound/sof/dai-imx.h
> @@ -8,6 +8,7 @@
> #ifndef __INCLUDE_SOUND_SOF_DAI_IMX_H__
> #define __INCLUDE_SOUND_SOF_DAI_IMX_H__
>
> +#include
> #include
>
> /* ESAI Configuration Request - SOF_IPC_DAI_ESAI_CONFIG */
> --
> 2.23.0
>
> --
> Cheers,
> Stephen Rothwell
[PATCH] firmware: imx: Remove call to devm_of_platform_populate
IMX DSP device is created by SOF layer. The current call to
devm_of_platform_populate is not needed and it doesn't produce
any effects.
Fixes: ffbf23d50353915d ("firmware: imx: Add DSP IPC protocol interface)
Signed-off-by: Daniel Baluta
---
drivers/firmware/imx/imx-dsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/imx/imx-dsp.c b/drivers/firmware/imx/imx-dsp.c
index a43d2db5cbdb..4265e9dbed84 100644
--- a/drivers/firmware/imx/imx-dsp.c
+++ b/drivers/firmware/imx/imx-dsp.c
@@ -114,7 +114,7 @@ static int imx_dsp_probe(struct platform_device *pdev)
dev_info(dev, "NXP i.MX DSP IPC initialized\n");
- return devm_of_platform_populate(dev);
+ return 0;
out:
kfree(chan_name);
for (j = 0; j < i; j++) {
--
2.17.1
Re: [alsa-devel] [RFC PATCH 2/2] ASoC: simple-card: Add documentation for force-dpcm property
On Mon, Oct 14, 2019 at 2:57 PM Mark Brown wrote: > > On Sun, Oct 13, 2019 at 10:00:14PM +0300, Daniel Baluta wrote: > > > This property can be global in which case all links created will be DPCM > > or present in certian dai-link subnode in which case only that specific > > link is forced to be DPCM. > > > +- force-dpcm : Indicates dai-link is always DPCM. > > DPCM is an implementation detail of Linux (and one that we want to phase > out going forwards too), we shouldn't be putting it in the DT bindings > where it becomes an ABI. Hi Mark, I see your point. This is way I marked the patch series as RFC. I need to find another way to reuse simple-card as machine driver for SOF. thanks, Daniel.
[RFC PATCH 1/2] ASoC: simple-card: Introduce force-dpcm DT property
Until now dai_link uses dynamic PCM in the following conditions:
* it is dpcm_selectable (this means compatible string is
simple-scu-card)
* it has convert-xxx property
* or it has more than one CPU DAIs
Our use case requires to be able to build a DPCM link with just 1 CPU.
Add force-dpcm DT property to realize this.
Signed-off-by: Daniel Baluta
---
include/sound/simple_card_utils.h | 4
sound/soc/generic/simple-card-utils.c | 17 +
sound/soc/generic/simple-card.c | 25 +++--
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/include/sound/simple_card_utils.h
b/include/sound/simple_card_utils.h
index 985a5f583de4..0578bbfa4a24 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -48,6 +48,7 @@ struct asoc_simple_priv {
struct asoc_simple_data adata;
struct snd_soc_codec_conf *codec_conf;
unsigned int mclk_fs;
+ unsigned int force_dpcm;
} *dai_props;
struct asoc_simple_jack hp_jack;
struct asoc_simple_jack mic_jack;
@@ -120,6 +121,9 @@ void asoc_simple_convert_fixup(struct asoc_simple_data
*data,
void asoc_simple_parse_convert(struct device *dev,
struct device_node *np, char *prefix,
struct asoc_simple_data *data);
+void asoc_simple_parse_force_dpcm(struct device *dev,
+ struct device_node *np, char *prefix,
+ unsigned int *force_dpcm);
int asoc_simple_parse_routing(struct snd_soc_card *card,
char *prefix);
diff --git a/sound/soc/generic/simple-card-utils.c
b/sound/soc/generic/simple-card-utils.c
index 9b794775df53..2f03a73f8a8a 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -52,6 +52,23 @@ void asoc_simple_parse_convert(struct device *dev,
}
EXPORT_SYMBOL_GPL(asoc_simple_parse_convert);
+void asoc_simple_parse_force_dpcm(struct device *dev,
+ struct device_node *np,
+ char *prefix,
+ unsigned int *force_dpcm)
+{
+ char prop[128];
+
+ if (!prefix)
+ prefix = "";
+
+ /* dpcm property */
+ snprintf(prop, sizeof(prop), "%s%s", prefix, "force-dpcm");
+ if (of_find_property(np, prop, NULL))
+ *force_dpcm = 1;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_force_dpcm);
+
int asoc_simple_parse_daifmt(struct device *dev,
struct device_node *node,
struct device_node *codec,
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index fc9c753db8dd..e40e22c8813b 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -92,6 +92,21 @@ static void simple_parse_convert(struct device *dev,
of_node_put(node);
}
+static void simple_parse_force_dpcm(struct device *dev,
+struct device_node *np,
+unsigned int *force_dpcm)
+{
+ struct device_node *top = dev->of_node;
+ struct device_node *node = of_get_parent(np);
+
+ asoc_simple_parse_force_dpcm(dev, top, PREFIX, force_dpcm);
+ asoc_simple_parse_force_dpcm(dev, node, PREFIX, force_dpcm);
+ asoc_simple_parse_force_dpcm(dev, node, NULL, force_dpcm);
+ asoc_simple_parse_force_dpcm(dev, np, NULL, force_dpcm);
+
+ of_node_put(node);
+}
+
static void simple_parse_mclk_fs(struct device_node *top,
struct device_node *cpu,
struct device_node *codec,
@@ -372,6 +387,7 @@ static int simple_for_each_link(struct asoc_simple_priv
*priv,
struct asoc_simple_data adata;
struct device_node *codec;
struct device_node *np;
+ unsigned int force_dpcm = 0;
int num = of_get_child_count(node);
/* get codec */
@@ -387,15 +403,20 @@ static int simple_for_each_link(struct asoc_simple_priv
*priv,
for_each_child_of_node(node, np)
simple_parse_convert(dev, np, );
+ /* get force-dpcm property */
+ for_each_child_of_node(node, np)
+ simple_parse_force_dpcm(dev, np, _dpcm);
+
/* loop for all CPU/Codec node */
for_each_child_of_node(node, np) {
/*
* It is DPCM
* if it has many CPUs,
-* or has convert-xxx property
+* or it has convert-xxx property
+* or it has force-dpcm property
*/
[RFC PATCH 2/2] ASoC: simple-card: Add documentation for force-dpcm property
This property can be global in which case all links created will be DPCM or present in certian dai-link subnode in which case only that specific link is forced to be DPCM. Signed-off-by: Daniel Baluta --- Documentation/devicetree/bindings/sound/simple-card.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 79954cd6e37b..15e6f5329857 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -63,6 +63,7 @@ Optional dai-link subnode properties: - mclk-fs : Multiplication factor between stream rate and codec mclk, applied only for the dai-link. +- force-dpcm : Indicates dai-link is always DPCM. For backward compatibility the frame-master and bitclock-master properties can be used as booleans in codec subnode to indicate if the -- 2.17.1
[RFC PATCH 0/2] Introduce for-dpcm DT property
We need to be able to create DPCM links even if we have a single CPU DAI or just a dummy CPU DAI. Daniel Baluta (2): ASoC: simple-card: Introduce force-dpcm DT property ASoC: simple-card: Add documentation for force-dpcm property .../devicetree/bindings/sound/simple-card.txt | 1 + include/sound/simple_card_utils.h | 4 +++ sound/soc/generic/simple-card-utils.c | 17 + sound/soc/generic/simple-card.c | 25 +-- 4 files changed, 45 insertions(+), 2 deletions(-) -- 2.17.1
Re: [EXT] Re: [RESEND PATCH v5 4/4] mailbox: imx: add support for imx v1 mu
On Sat, Oct 12, 2019 at 4:12 AM Richard Zhu wrote: > > Hi Daniel: > New version patch-set had been sent out on Oct9. > https://patchwork.kernel.org/cover/11180683/ Thanks Richard. Jassi, care to have a look? Daniel
[PATCH 2/2] ASoC: simple_card_utils.h: Fix potential multiple redefinition error
asoc_simple_debug_info and asoc_simple_debug_dai must be static
otherwise we might a compilation error if the compiler decides
not to inline the given function.
Fixes: 0580dde59438686d ("ASoC: simple-card-utils: add
asoc_simple_debug_info()")
Signed-off-by: Daniel Baluta
---
include/sound/simple_card_utils.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/sound/simple_card_utils.h
b/include/sound/simple_card_utils.h
index 293ff8115960..bbdd1542d6f1 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -136,9 +136,9 @@ int asoc_simple_init_priv(struct asoc_simple_priv *priv,
struct link_info *li);
#ifdef DEBUG
-inline void asoc_simple_debug_dai(struct asoc_simple_priv *priv,
- char *name,
- struct asoc_simple_dai *dai)
+static inline void asoc_simple_debug_dai(struct asoc_simple_priv *priv,
+char *name,
+struct asoc_simple_dai *dai)
{
struct device *dev = simple_priv_to_dev(priv);
@@ -168,7 +168,7 @@ inline void asoc_simple_debug_dai(struct asoc_simple_priv
*priv,
dev_dbg(dev, "%s clk %luHz\n", name, clk_get_rate(dai->clk));
}
-inline void asoc_simple_debug_info(struct asoc_simple_priv *priv)
+static inline void asoc_simple_debug_info(struct asoc_simple_priv *priv)
{
struct snd_soc_card *card = simple_priv_to_card(priv);
struct device *dev = simple_priv_to_dev(priv);
--
2.17.1
[PATCH 1/2] ASoC: simple_card_utils.h: Add missing include
When debug is enabled compiler cannot find the definition of
clk_get_rate resulting in the following error:
./include/sound/simple_card_utils.h:168:40: note: previous implicit
declaration of ‘clk_get_rate’ was here
dev_dbg(dev, "%s clk %luHz\n", name, clk_get_rate(dai->clk));
./include/sound/simple_card_utils.h:168:3: note: in expansion of macro
‘dev_dbg’
dev_dbg(dev, "%s clk %luHz\n", name, clk_get_rate(dai->clk));
Fix this by including the appropriate header.
Fixes: 0580dde59438686d ("ASoC: simple-card-utils: add
asoc_simple_debug_info()")
Signed-off-by: Daniel Baluta
---
include/sound/simple_card_utils.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/sound/simple_card_utils.h
b/include/sound/simple_card_utils.h
index 985a5f583de4..293ff8115960 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -8,6 +8,7 @@
#ifndef __SIMPLE_CARD_UTILS_H
#define __SIMPLE_CARD_UTILS_H
+#include
#include
#define asoc_simple_init_hp(card, sjack, prefix) \
--
2.17.1
[PATCH 0/2] ASoC: simple_card_utils.h: Fix two potential compilation errors
For this to happen we symbol DEBUG to be defined. Daniel Baluta (2): ASoC: simple_card_utils.h: Add missing include ASoC: Fix potential multiple redefinition error include/sound/simple_card_utils.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.17.1
Re: [RESEND PATCH v5 4/4] mailbox: imx: add support for imx v1 mu
Hi Richard,
Can you please rebase and resend this patch series?
On Mon, Aug 5, 2019 at 10:21 PM Daniel Baluta wrote:
>
> On Mon, Aug 5, 2019 at 8:16 AM Richard Zhu wrote:
> >
> > There is a version 1.0 MU on i.MX7ULP platform.
> > One new version ID register is added, and it's offset is 0.
> > TRn registers are defined at the offset 0x20 ~ 0x2C.
> > RRn registers are defined at the offset 0x40 ~ 0x4C.
> > SR/CR registers are defined at 0x60/0x64.
> > Extend this driver to support it.
> >
> > Signed-off-by: Richard Zhu
> > Suggested-by: Oleksij Rempel
> > Reviewed-by: Dong Aisheng
> > Reviewed-by: Oleksij Rempel
>
> Very clean solution. Thanks Richard!
>
> Reviewed-by: Daniel Baluta
>
> > ---
> > drivers/mailbox/imx-mailbox.c | 55
> > ++-
> > 1 file changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> > index afe625e..2cdcdc5 100644
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -12,19 +12,11 @@
> > #include
> > #include
> >
> > -/* Transmit Register */
> > -#define IMX_MU_xTRn(x) (0x00 + 4 * (x))
> > -/* Receive Register */
> > -#define IMX_MU_xRRn(x) (0x10 + 4 * (x))
> > -/* Status Register */
> > -#define IMX_MU_xSR 0x20
> > #define IMX_MU_xSR_GIPn(x) BIT(28 + (3 - (x)))
> > #define IMX_MU_xSR_RFn(x) BIT(24 + (3 - (x)))
> > #define IMX_MU_xSR_TEn(x) BIT(20 + (3 - (x)))
> > #define IMX_MU_xSR_BRDIP BIT(9)
> >
> > -/* Control Register */
> > -#define IMX_MU_xCR 0x24
> > /* General Purpose Interrupt Enable */
> > #define IMX_MU_xCR_GIEn(x) BIT(28 + (3 - (x)))
> > /* Receive Interrupt Enable */
> > @@ -44,6 +36,13 @@ enum imx_mu_chan_type {
> > IMX_MU_TYPE_RXDB, /* Rx doorbell */
> > };
> >
> > +struct imx_mu_dcfg {
> > + u32 xTR[4]; /* Transmit Registers */
> > + u32 xRR[4]; /* Receive Registers */
> > + u32 xSR;/* Status Register */
> > + u32 xCR;/* Control Register */
> > +};
> > +
> > struct imx_mu_con_priv {
> > unsigned intidx;
> > charirq_desc[IMX_MU_CHAN_NAME_SIZE];
> > @@ -61,12 +60,27 @@ struct imx_mu_priv {
> > struct mbox_chanmbox_chans[IMX_MU_CHANS];
> >
> > struct imx_mu_con_priv con_priv[IMX_MU_CHANS];
> > + const struct imx_mu_dcfg*dcfg;
> > struct clk *clk;
> > int irq;
> >
> > boolside_b;
> > };
> >
> > +static const struct imx_mu_dcfg imx_mu_cfg_imx6sx = {
> > + .xTR= {0x0, 0x4, 0x8, 0xc},
> > + .xRR= {0x10, 0x14, 0x18, 0x1c},
> > + .xSR= 0x20,
> > + .xCR= 0x24,
> > +};
> > +
> > +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> > + .xTR= {0x20, 0x24, 0x28, 0x2c},
> > + .xRR= {0x40, 0x44, 0x48, 0x4c},
> > + .xSR= 0x60,
> > + .xCR= 0x64,
> > +};
> > +
> > static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox)
> > {
> > return container_of(mbox, struct imx_mu_priv, mbox);
> > @@ -88,10 +102,10 @@ static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv,
> > u32 set, u32 clr)
> > u32 val;
> >
> > spin_lock_irqsave(>xcr_lock, flags);
> > - val = imx_mu_read(priv, IMX_MU_xCR);
> > + val = imx_mu_read(priv, priv->dcfg->xCR);
> > val &= ~clr;
> > val |= set;
> > - imx_mu_write(priv, val, IMX_MU_xCR);
> > + imx_mu_write(priv, val, priv->dcfg->xCR);
> > spin_unlock_irqrestore(>xcr_lock, flags);
> >
> > return val;
> > @@ -111,8 +125,8 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
> > struct imx_mu_con_priv *cp = chan->con_priv;
> > u32 val, ctrl, dat;
> >
> > - ctrl = imx_mu_read(priv, IMX_MU_xCR);
> > - val = imx_mu_read(priv, IMX_MU_xSR);
> > + ctrl = imx_mu_read(priv, priv->dcfg->xCR);
> > + val = imx_mu_read(priv, priv->dcfg->xSR);
> >
> > switch (cp->type) {
> > case IMX_MU_TYPE_TX:
> > @@ -138,10 +152,10 @@ static irqreturn_t imx_mu_isr(int irq, void *p)
&
Re: [PATCH] ASoC: SOF: imx: fix reverse CONFIG_SND_SOC_SOF_OF dependency
On Tue, 2019-10-01 at 16:20 +0200, Arnd Bergmann wrote:
> CONFIG_SND_SOC_SOF_IMX depends on CONFIG_SND_SOC_SOF, but is in
> turn referenced by the sof-of-dev driver. This creates a reverse
> dependency that manifests in a link error when CONFIG_SND_SOC_SOF_OF
> is built-in but CONFIG_SND_SOC_SOF_IMX=m:
>
> sound/soc/sof/sof-of-dev.o:(.data+0x118): undefined reference to
> `sof_imx8_ops'
>
> Make the latter a 'bool' symbol and change the Makefile so the imx8
> driver is compiled the same way as the driver using it.
>
> A nicer way would be to reverse the layering and move all
> the imx specific bits of sof-of-dev.c into the imx driver
> itself, which can then call into the common code. Doing this
> would need more testing and can be done if we add another
> driver like the first one.
>
> Fixes: f4df4e4042b0 ("ASoC: SOF: imx8: Fix COMPILE_TEST error")
> Fixes: 202acc565a1f ("ASoC: SOF: imx: Add i.MX8 HW support")
> Signed-off-by: Arnd Bergmann
Acked-by: Daniel Baluta
Indeed we will need to somehow avoid getting sof_imx8_ops from
sof-of-dev.c by directly referencing it.
Will keep this in mind for the next platform.
Thanks a lot Arnd!
> ---
> sound/soc/sof/imx/Kconfig | 2 +-
> sound/soc/sof/imx/Makefile | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> index 5acae75f5750..a3891654a1fc 100644
> --- a/sound/soc/sof/imx/Kconfig
> +++ b/sound/soc/sof/imx/Kconfig
> @@ -12,7 +12,7 @@ config SND_SOC_SOF_IMX_TOPLEVEL
> if SND_SOC_SOF_IMX_TOPLEVEL
>
> config SND_SOC_SOF_IMX8
> - tristate "SOF support for i.MX8"
> + bool "SOF support for i.MX8"
> depends on IMX_SCU
> depends on IMX_DSP
> help
> diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
> index 6ef908e8c807..9e8f35df0ff2 100644
> --- a/sound/soc/sof/imx/Makefile
> +++ b/sound/soc/sof/imx/Makefile
> @@ -1,4 +1,6 @@
> # SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> snd-sof-imx8-objs := imx8.o
>
> -obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o
> +ifdef CONFIG_SND_SOC_SOF_IMX8
> +obj-$(CONFIG_SND_SOC_SOF_OF) += snd-sof-imx8.o
> +endif
[PATCH] ASoC: core: Clarify usage of ignore_machine
For a sound card ignore_machine means that existing FEs links should be
ignored and existing BEs links should be overridden with some information
from the matching component driver.
Current code make some confusions about this so fix it!
Signed-off-by: Daniel Baluta
---
sound/soc/soc-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 88978a3036c4..e32a45f6bd88 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1853,7 +1853,7 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
for_each_component(component) {
- /* does this component override FEs ? */
+ /* does this component override BEs ? */
if (!component->driver->ignore_machine)
continue;
@@ -1874,7 +1874,7 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
continue;
}
- dev_info(card->dev, "info: override FE DAI link %s\n",
+ dev_info(card->dev, "info: override BE DAI link %s\n",
card->dai_link[i].name);
/* override platform component */
--
2.17.1
[PATCH v2 0/3] Several SAI fixes
This series contains several fixes for SAI. They are unrelated but grouped them in a patch series to be easier applied. Daniel Baluta (1): ASoC: fsl_sai: Fix TCSR.TE/RCSR.RE in synchronous mode Mihai Serban (1): ASoC: fsl_sai: Fix noise when using EDMA Shengjiu Wang (1): ASoC: fsl_sai: Fix xMR setting in synchronous mode sound/soc/fsl/fsl_sai.c | 31 ++- sound/soc/fsl/fsl_sai.h | 1 + 2 files changed, 23 insertions(+), 9 deletions(-) -- 2.17.1
[PATCH v2 3/3] ASoC: fsl_sai: Fix TCSR.TE/RCSR.RE in synchronous mode
The SAI transmitter and receiver can be configured to operate with
synchronous bit clock and frame sync.
When Tx is synchronous with receiver RCSR.RE should be set in playback
to enable the receiver which provides bit clock and frame sync.
When Rx is synchronous with transmitter TCSR.TE should be set in record
to enable the transmitter which provides bit clock and frame sync.
Cc: NXP Linux Team
Signed-off-by: Daniel Baluta
---
Changes since v1:
* new patch
sound/soc/fsl/fsl_sai.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 6598a1ae0a2d..a59300e37549 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -539,8 +539,8 @@ static int fsl_sai_trigger(struct snd_pcm_substream
*substream, int cmd,
sai->synchronous[RX] ? FSL_SAI_CR2_SYNC : 0);
/*
-* It is recommended that the transmitter is the last enabled
-* and the first disabled.
+* it is recommended that the asynchronous block to be the last enabled
+* and the first disabled
*/
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
@@ -549,9 +549,11 @@ static int fsl_sai_trigger(struct snd_pcm_substream
*substream, int cmd,
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_FRDE, FSL_SAI_CSR_FRDE);
- regmap_update_bits(sai->regmap, FSL_SAI_RCSR(ofs),
- FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
- regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
+ if (sai->synchronous[tx])
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(!tx, ofs),
+ FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
FSL_SAI_CSR_TERE, FSL_SAI_CSR_TERE);
regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
--
2.17.1
[PATCH v2 2/3] ASoC: fsl_sai: Fix xMR setting in synchronous mode
From: Shengjiu Wang
When Tx is synchronous with receiver the RMR should not be changed.
When Rx is synchronous with transmitter the TMR should not be changed.
Cc: NXP Linux Team
Signed-off-by: Shengjiu Wang
Signed-off-by: Daniel Baluta
---
Changes since v1:
* new patch
sound/soc/fsl/fsl_sai.c | 4
1 file changed, 4 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index b517e4bc1b87..6598a1ae0a2d 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -482,8 +482,6 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_TMR,
- ~0UL - ((1 << channels) - 1));
} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx)
{
regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
@@ -491,8 +489,6 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_RMR,
- ~0UL - ((1 << channels) - 1));
}
}
--
2.17.1
[PATCH v2 1/3] ASoC: fsl_sai: Fix noise when using EDMA
From: Mihai Serban
EDMA requires the period size to be multiple of maxburst. Otherwise the
remaining bytes are not transferred and thus noise is produced.
We can handle this issue by adding a constraint on
SNDRV_PCM_HW_PARAM_PERIOD_SIZE to be multiple of tx/rx maxburst value.
Signed-off-by: Mihai Serban
Signed-off-by: Daniel Baluta
---
Changes since v1:
* rename variable to use_edma as per Nicolin's suggestion.
sound/soc/fsl/fsl_sai.c | 15 +++
sound/soc/fsl/fsl_sai.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index ef0b74693093..b517e4bc1b87 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -628,6 +628,16 @@ static int fsl_sai_startup(struct snd_pcm_substream
*substream,
FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE);
+ /*
+* EDMA controller needs period size to be a multiple of
+* tx/rx maxburst
+*/
+ if (sai->soc_data->use_edma)
+ snd_pcm_hw_constraint_step(substream->runtime, 0,
+ SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+ tx ? sai->dma_params_tx.maxburst :
+ sai->dma_params_rx.maxburst);
+
ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_RATE, _sai_rate_constraints);
@@ -1026,30 +1036,35 @@ static int fsl_sai_remove(struct platform_device *pdev)
static const struct fsl_sai_soc_data fsl_sai_vf610_data = {
.use_imx_pcm = false,
+ .use_edma = false,
.fifo_depth = 32,
.reg_offset = 0,
};
static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
.use_imx_pcm = true,
+ .use_edma = false,
.fifo_depth = 32,
.reg_offset = 0,
};
static const struct fsl_sai_soc_data fsl_sai_imx7ulp_data = {
.use_imx_pcm = true,
+ .use_edma = false,
.fifo_depth = 16,
.reg_offset = 8,
};
static const struct fsl_sai_soc_data fsl_sai_imx8mq_data = {
.use_imx_pcm = true,
+ .use_edma = false,
.fifo_depth = 128,
.reg_offset = 8,
};
static const struct fsl_sai_soc_data fsl_sai_imx8qm_data = {
.use_imx_pcm = true,
+ .use_edma = true,
.fifo_depth = 64,
.reg_offset = 0,
};
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index b12cb578f6d0..76b15deea80c 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -157,6 +157,7 @@
struct fsl_sai_soc_data {
bool use_imx_pcm;
+ bool use_edma;
unsigned int fifo_depth;
unsigned int reg_offset;
};
--
2.17.1
Re: [alsa-devel] [PATCH] ASoC: fsl_sai: Implement set_bclk_ratio
On Wed, Sep 11, 2019 at 2:01 PM Mark Brown wrote: > > On Thu, Sep 05, 2019 at 06:29:39PM -0700, Nicolin Chen wrote: > > On Sat, Aug 31, 2019 at 12:59:10AM +0300, Daniel Baluta wrote: > > > > This is to allow machine drivers to set a certain bitclk rate > > > which might not be exactly rate * frame size. > > > Just a quick thought of mine: slot_width and slots could be > > set via set_dai_tdm_slot() actually, while set_bclk_ratio() > > would override that one with your change. I'm not sure which > > one could be more important...so would you mind elaborating > > your use case? > > The reason we have both operations is partly that some hardware > can configure the ratio but not do TDM and partly that setting > TDM slots forces us to configure the slot size depending on the > current stream configuration while just setting the ratio means > we can just fix the configuration once. I'd say it's just a user > error to try to do both simultaneously. Yes, exactly. We wanted to have a better control of bclk freq. Sorry for the late answer, I'm traveling.
Re: [PATCH] clk: imx: lpcg: write twice when writing lpcg regs
On Tue, Sep 10, 2019 at 1:40 PM Anson Huang wrote: > > > > > On Sat, Sep 7, 2019 at 9:47 PM Stephen Boyd wrote: > > > > > > Quoting Peng Fan (2019-08-27 01:17:50) > > > > From: Peng Fan > > > > > > > > There is hardware issue that: > > > > The output clock the LPCG cell will not turn back on as expected, > > > > even though a read of the IPG registers in the LPCG indicates that > > > > the clock should be enabled. > > > > > > > > The software workaround is to write twice to enable the LPCG clock > > > > output. > > > > > > > > Signed-off-by: Peng Fan > > > > > > Does this need a Fixes tag? > > > > Not sure as it's not code logic issue but a hardware bug. > > And 4.19 LTS still have not this driver support. > > Looks like there is an errata for this issue, and Ranjani just sent a patch > for review internally, > > Back-to-back LPCG writes can be ignored by the LPCG register due to a > HW bug. The writes need to be separated by atleast 4 cycles of the gated > clock. > The workaround is implemented as follows: > 1. For clocks running greater than 50MHz no delay is required as the > delay in accessing the LPCG register is sufficient. > 2. For clocks running greater than 23MHz, a read followed by the write > will provide the sufficient delay. > 3. For clocks running below 23MHz, LPCG is not used. Lets add this information in the commit message and also enhance the comment before the double write. Also, why can't we add a udelay after the first write and remove the second write as having two writes for writing a value looks very un-natural.
Re: [alsa-devel] [PATCH] ASoC: fsl_sai: Fix noise when using EDMA
On Fri, Sep 6, 2019 at 4:25 AM Nicolin Chen wrote:
>
> On Fri, Aug 30, 2019 at 11:09:00PM +0300, Daniel Baluta wrote:
> > From: Mihai Serban
> >
> > EDMA requires the period size to be multiple of maxburst. Otherwise the
> > remaining bytes are not transferred and thus noise is produced.
> >
> > We can handle this issue by adding a constraint on
> > SNDRV_PCM_HW_PARAM_PERIOD_SIZE to be multiple of tx/rx maxburst value.
> >
> > Cc: NXP Linux Team
> > Signed-off-by: Mihai Serban
> > Signed-off-by: Daniel Baluta
> > ---
> > sound/soc/fsl/fsl_sai.c | 15 +++
> > sound/soc/fsl/fsl_sai.h | 1 +
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 728307acab90..fe126029f4e3 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -612,6 +612,16 @@ static int fsl_sai_startup(struct snd_pcm_substream
> > *substream,
> > FSL_SAI_CR3_TRCE_MASK,
> > FSL_SAI_CR3_TRCE);
> >
> > + /*
> > + * some DMA controllers need period size to be a multiple of
> > + * tx/rx maxburst
> > + */
> > + if (sai->soc_data->use_constraint_period_size)
> > + snd_pcm_hw_constraint_step(substream->runtime, 0,
> > +SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> > +tx ? sai->dma_params_tx.maxburst :
> > +sai->dma_params_rx.maxburst);
>
> I feel that PERIOD_SIZE could be used for some other cases than
> being related to maxburst
>
> > static const struct of_device_id fsl_sai_ids[] = {
> > diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> > index b89b0ca26053..3a3f6f8e5595 100644
> > --- a/sound/soc/fsl/fsl_sai.h
> > +++ b/sound/soc/fsl/fsl_sai.h
> > @@ -157,6 +157,7 @@
> >
> > struct fsl_sai_soc_data {
> > bool use_imx_pcm;
> > + bool use_constraint_period_size;
>
> so maybe the soc specific flag here could be something like
> bool use_edma;
>
> What do you think?
I think your suggestion is a little bit better than what we have. But what if
in the future another DMA controler (not eDMA) will need the same constraint.
Wouldn't it be confusing?
Re: [PATCH -next] ASoC: SOF: imx8: Fix COMPILE_TEST error
On Thu, 2019-09-05 at 14:44 +0800, YueHaibing wrote:
> When do compile test, if SND_SOC_SOF_OF is not set, we get:
>
> sound/soc/sof/imx/imx8.o: In function `imx8_dsp_handle_request':
> imx8.c:(.text+0xb0): undefined reference to `snd_sof_ipc_msgs_rx'
> sound/soc/sof/imx/imx8.o: In function `imx8_ipc_msg_data':
> imx8.c:(.text+0xf4): undefined reference to `sof_mailbox_read'
> sound/soc/sof/imx/imx8.o: In function `imx8_dsp_handle_reply':
> imx8.c:(.text+0x160): undefined reference to `sof_mailbox_read'
>
> Make SND_SOC_SOF_IMX_TOPLEVEL always depends on SND_SOC_SOF_OF
>
> Reported-by: Hulk Robot
> Fixes: 202acc565a1f ("ASoC: SOF: imx: Add i.MX8 HW support")
> Signed-off-by: YueHaibing
Reviewed-by: Daniel Baluta
> ---
> sound/soc/sof/imx/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> index fd73d84..5acae75 100644
> --- a/sound/soc/sof/imx/Kconfig
> +++ b/sound/soc/sof/imx/Kconfig
> @@ -2,7 +2,8 @@
>
> config SND_SOC_SOF_IMX_TOPLEVEL
> bool "SOF support for NXP i.MX audio DSPs"
> - depends on ARM64 && SND_SOC_SOF_OF || COMPILE_TEST
> + depends on ARM64|| COMPILE_TEST
> + depends on SND_SOC_SOF_OF
> help
>This adds support for Sound Open Firmware for NXP i.MX
> platforms.
>Say Y if you have such a device.
Re: [PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
On Mon, Sep 2, 2019 at 3:42 PM Mark Brown wrote: > > On Sat, Aug 31, 2019 at 01:55:14AM +0300, Daniel Baluta wrote: > > > Fix this by setting CHMOD to Output Mode so that pins will output zero > > when slots are masked or channels are disabled. > > This patch seems to do this unconditionally. This is fine for > configurations where the SoC is the only thing driving the bus but will > mean that for TDM configurations where something else also drives some > of the slots we'll end up with both devices driving simultaneously. The > safest thing would be to set this only if TDM isn't configured. I thought that the SAI IP is the single owner of the audio data lines, so even in TDM mode SAI IP (which is inside SoC) is the only one adding data on the bus. Now, you say that there could be two devices driving some of he masked slots right? I'm not sure how to really figure out that SAI is running in TDM mode. RM says: When enabled, the SAI continuously transmits and/or receives frames of data. Each frame consists of a fixed number of words and each word consists of a fixed number of bits. Within each frame, any given word can be masked causing the receiver to ignore that word and the transmitter to tri-state for the duration of that word. Will try to ask IP designer about this, thanks a lot for your comment! Daniel.
[PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
>From SAI datasheet:
CHMOD, configures if transmit data pins are configured for TDM mode
or Output mode.
* (0) TDM mode, transmit data pins are tri-stated when slots are
masked or channels are disabled.
* (1) Output mode, transmit data pins are never tri-stated and
will output zero when slots are masked or channels are disabled.
When data pins are tri-stated, there is noise on some channels
when FS clock value is high and data is read while fsclk is
transitioning from high to low.
Fix this by setting CHMOD to Output Mode so that pins will output zero
when slots are masked or channels are disabled.
Cc: NXP Linux Team
Signed-off-by: Cosmin-Gabriel Samoila
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 15 ---
sound/soc/fsl/fsl_sai.h | 2 ++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index e896b577b1f7..b9daab0eb6eb 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -467,6 +467,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
+ /*
+* set CHMOD to Output Mode so that transmit data pins will
+* output zero when slots are masked or channels are disabled
+*/
+ val_cr4 |= FSL_SAI_CR4_CHMOD;
+
/*
* For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will
* generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4),
@@ -477,7 +483,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
if (!sai->is_slave_mode) {
if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
@@ -486,7 +493,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
~0UL - ((1 << channels) - 1));
} else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx)
{
regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
@@ -497,7 +505,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
}
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index f96f8d97489d..1e3b4a6889a8 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -119,6 +119,8 @@
#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16)
#define FSL_SAI_CR4_SYWD(x)(((x) - 1) << 8)
#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8)
+#define FSL_SAI_CR4_CHMOD BIT(5)
+#define FSL_SAI_CR4_CHMOD_MASK GENMASK(5, 5)
#define FSL_SAI_CR4_MF BIT(4)
#define FSL_SAI_CR4_FSEBIT(3)
#define FSL_SAI_CR4_FSPBIT(1)
--
2.17.1
[PATCH] ASoC: fsl_sai: Implement set_bclk_ratio
From: Viorel Suman
This is to allow machine drivers to set a certain bitclk rate
which might not be exactly rate * frame size.
Cc: NXP Linux Team
Signed-off-by: Viorel Suman
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 21 +++--
sound/soc/fsl/fsl_sai.h | 1 +
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index fe126029f4e3..e896b577b1f7 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -137,6 +137,16 @@ static int fsl_sai_set_dai_tdm_slot(struct snd_soc_dai
*cpu_dai, u32 tx_mask,
return 0;
}
+static int fsl_sai_set_dai_bclk_ratio(struct snd_soc_dai *dai,
+ unsigned int ratio)
+{
+ struct fsl_sai *sai = snd_soc_dai_get_drvdata(dai);
+
+ sai->bclk_ratio = ratio;
+
+ return 0;
+}
+
static int fsl_sai_set_dai_sysclk_tr(struct snd_soc_dai *cpu_dai,
int clk_id, unsigned int freq, int fsl_dir)
{
@@ -423,8 +433,14 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
slot_width = sai->slot_width;
if (!sai->is_slave_mode) {
- ret = fsl_sai_set_bclk(cpu_dai, tx,
- slots * slot_width * params_rate(params));
+ if (sai->bclk_ratio)
+ ret = fsl_sai_set_bclk(cpu_dai, tx,
+ sai->bclk_ratio *
+ params_rate(params));
+ else
+ ret = fsl_sai_set_bclk(cpu_dai, tx,
+ slots * slot_width *
+ params_rate(params));
if (ret)
return ret;
@@ -640,6 +656,7 @@ static void fsl_sai_shutdown(struct snd_pcm_substream
*substream,
}
static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
+ .set_bclk_ratio = fsl_sai_set_dai_bclk_ratio,
.set_sysclk = fsl_sai_set_dai_sysclk,
.set_fmt= fsl_sai_set_dai_fmt,
.set_tdm_slot = fsl_sai_set_dai_tdm_slot,
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 3a3f6f8e5595..f96f8d97489d 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -177,6 +177,7 @@ struct fsl_sai {
unsigned int mclk_streams;
unsigned int slots;
unsigned int slot_width;
+ unsigned int bclk_ratio;
const struct fsl_sai_soc_data *soc_data;
struct snd_dmaengine_dai_dma_data dma_params_rx;
--
2.17.1
[PATCH] ASoC: fsl_sai: Fix noise when using EDMA
From: Mihai Serban
EDMA requires the period size to be multiple of maxburst. Otherwise the
remaining bytes are not transferred and thus noise is produced.
We can handle this issue by adding a constraint on
SNDRV_PCM_HW_PARAM_PERIOD_SIZE to be multiple of tx/rx maxburst value.
Cc: NXP Linux Team
Signed-off-by: Mihai Serban
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 15 +++
sound/soc/fsl/fsl_sai.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 728307acab90..fe126029f4e3 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -612,6 +612,16 @@ static int fsl_sai_startup(struct snd_pcm_substream
*substream,
FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE);
+ /*
+* some DMA controllers need period size to be a multiple of
+* tx/rx maxburst
+*/
+ if (sai->soc_data->use_constraint_period_size)
+ snd_pcm_hw_constraint_step(substream->runtime, 0,
+ SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+ tx ? sai->dma_params_tx.maxburst :
+ sai->dma_params_rx.maxburst);
+
ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_RATE, _sai_rate_constraints);
@@ -1011,30 +1021,35 @@ static const struct fsl_sai_soc_data fsl_sai_vf610_data
= {
.use_imx_pcm = false,
.fifo_depth = 32,
.reg_offset = 0,
+ .use_constraint_period_size = false,
};
static const struct fsl_sai_soc_data fsl_sai_imx6sx_data = {
.use_imx_pcm = true,
.fifo_depth = 32,
.reg_offset = 0,
+ .use_constraint_period_size = false,
};
static const struct fsl_sai_soc_data fsl_sai_imx7ulp_data = {
.use_imx_pcm = true,
.fifo_depth = 16,
.reg_offset = 8,
+ .use_constraint_period_size = false,
};
static const struct fsl_sai_soc_data fsl_sai_imx8mq_data = {
.use_imx_pcm = true,
.fifo_depth = 128,
.reg_offset = 8,
+ .use_constraint_period_size = false,
};
static const struct fsl_sai_soc_data fsl_sai_imx8qm_data = {
.use_imx_pcm = true,
.fifo_depth = 64,
.reg_offset = 0,
+ .use_constraint_period_size = true,
};
static const struct of_device_id fsl_sai_ids[] = {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index b89b0ca26053..3a3f6f8e5595 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -157,6 +157,7 @@
struct fsl_sai_soc_data {
bool use_imx_pcm;
+ bool use_constraint_period_size;
unsigned int fifo_depth;
unsigned int reg_offset;
};
--
2.17.1
Re: [alsa-devel] [PATCH] ASoC: cs42xx8: Force suspend/resume during system suspend/resume
On Tue, 2019-08-27 at 18:13 -0400, Shengjiu Wang wrote:
> Use force_suspend/resume to make sure clocks are disabled/enabled
> accordingly during system suspend/resume.
>
> Signed-off-by: Dong Aisheng
> Signed-off-by: Shengjiu Wang
Reviewed-by: Daniel Baluta
> ---
> sound/soc/codecs/cs42xx8.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
> index 5b049fcdba20..94b1adb088fd 100644
> --- a/sound/soc/codecs/cs42xx8.c
> +++ b/sound/soc/codecs/cs42xx8.c
> @@ -684,6 +684,8 @@ static int cs42xx8_runtime_suspend(struct device
> *dev)
> #endif
>
> const struct dev_pm_ops cs42xx8_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(cs42xx8_runtime_suspend,
> cs42xx8_runtime_resume, NULL)
> };
> EXPORT_SYMBOL_GPL(cs42xx8_pm);
Re: [alsa-devel] [PATCH] ASoC: cs42xx8: Force suspend/resume during system suspend/resume
On Tue, Aug 27, 2019 at 1:15 PM Shengjiu Wang wrote:
>
> Use force_suspend/resume to make sure clocks are disabled/enabled
> accordingly during system suspend/resume.
>
> Signed-off-by: Dong Aisheng
> Signed-off-by: Shengjiu Wang
Reviewed-by: Daniel Baluta
> ---
> sound/soc/codecs/cs42xx8.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c
> index 5b049fcdba20..94b1adb088fd 100644
> --- a/sound/soc/codecs/cs42xx8.c
> +++ b/sound/soc/codecs/cs42xx8.c
> @@ -684,6 +684,8 @@ static int cs42xx8_runtime_suspend(struct device *dev)
> #endif
>
> const struct dev_pm_ops cs42xx8_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(cs42xx8_runtime_suspend, cs42xx8_runtime_resume,
> NULL)
> };
> EXPORT_SYMBOL_GPL(cs42xx8_pm);
> --
> 2.21.0
>
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH] ASoC: imx-audmix: register the card on a proper dev
On Tue, 2019-08-27 at 11:55 -0400, Shengjiu Wang wrote:
> This platform device is registered from "fsl_audmix", which is
> its parent device. If use pdev->dev.parent for the priv->card.dev,
> the value set by dev_set_drvdata in parent device will be covered
> by the value in child device.
>
> Fixes: b86ef5367761 ("ASoC: fsl: Add Audio Mixer machine driver")
> Signed-off-by: Viorel Suman
> Signed-off-by: Shengjiu Wang
Reviewed-by: Daniel Baluta
Thanks Shengjiu for the fix!
> ---
> sound/soc/fsl/imx-audmix.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c
> index 9e1cb18859ce..71590ca6394b 100644
> --- a/sound/soc/fsl/imx-audmix.c
> +++ b/sound/soc/fsl/imx-audmix.c
> @@ -325,14 +325,14 @@ static int imx_audmix_probe(struct
> platform_device *pdev)
> priv->card.num_configs = priv->num_dai_conf;
> priv->card.dapm_routes = priv->dapm_routes;
> priv->card.num_dapm_routes = priv->num_dapm_routes;
> - priv->card.dev = pdev->dev.parent;
> + priv->card.dev = >dev;
> priv->card.owner = THIS_MODULE;
> priv->card.name = "imx-audmix";
>
> platform_set_drvdata(pdev, >card);
> snd_soc_card_set_drvdata(>card, priv);
>
> - ret = devm_snd_soc_register_card(pdev->dev.parent,
> >card);
> + ret = devm_snd_soc_register_card(>dev, >card);
> if (ret) {
> dev_err(>dev, "snd_soc_register_card failed\n");
> return ret;
Re: [PATCH -next] ASoC: SOF: imx8: Fix return value check in imx8_probe()
On Mon, 2019-08-26 at 12:00 +, Wei Yongjun wrote:
> In case of error, the function devm_ioremap_wc() returns NULL pointer
> not ERR_PTR(). The IS_ERR() test in the return value check should be
> replaced with NULL test.
>
> Fixes: 202acc565a1f ("ASoC: SOF: imx: Add i.MX8 HW support")
> Signed-off-by: Wei Yongjun
Good catch. Thanks!
Reviewed-by: Daniel Baluta
> ---
> sound/soc/sof/imx/imx8.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
> index e502f584207f..263d4df35fe8 100644
> --- a/sound/soc/sof/imx/imx8.c
> +++ b/sound/soc/sof/imx/imx8.c
> @@ -296,10 +296,10 @@ static int imx8_probe(struct snd_sof_dev *sdev)
> sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev,
> res.start,
> res.end -
> res.start +
> 1);
> - if (IS_ERR(sdev->bar[SOF_FW_BLK_TYPE_SRAM])) {
> + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
> dev_err(sdev->dev, "failed to ioremap mem 0x%x size
> 0x%x\n",
> base, size);
> - ret = PTR_ERR(sdev->bar[SOF_FW_BLK_TYPE_SRAM]);
> + ret = -ENOMEM;
> goto exit_pdev_unregister;
> }
> sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
>
>
>
Re: [alsa-devel] [PATCH -next] ASoC: SOF: imx8: Make some functions static
On Fri, Aug 23, 2019 at 4:12 PM YueHaibing wrote:
>
> Fix sparse warnings:
>
> sound/soc/sof/imx/imx8.c:104:6: warning: symbol 'imx8_dsp_handle_reply' was
> not declared. Should it be static?
> sound/soc/sof/imx/imx8.c:115:6: warning: symbol 'imx8_dsp_handle_request' was
> not declared. Should it be static?
> sound/soc/sof/imx/imx8.c:336:5: warning: symbol 'imx8_get_bar_index' was not
> declared. Should it be static?
> sound/soc/sof/imx/imx8.c:341:6: warning: symbol 'imx8_ipc_msg_data' was not
> declared. Should it be static?
> sound/soc/sof/imx/imx8.c:348:5: warning: symbol 'imx8_ipc_pcm_params' was not
> declared. Should it be static?
>
> Reported-by: Hulk Robot
> Signed-off-by: YueHaibing
Reviewed-by: Daniel Baluta
> ---
> sound/soc/sof/imx/imx8.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
> index e502f58..6404724 100644
> --- a/sound/soc/sof/imx/imx8.c
> +++ b/sound/soc/sof/imx/imx8.c
> @@ -101,7 +101,7 @@ static int imx8_get_window_offset(struct snd_sof_dev
> *sdev, u32 id)
> return MBOX_OFFSET;
> }
>
> -void imx8_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> +static void imx8_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> {
> struct imx8_priv *priv = imx_dsp_get_data(ipc);
> unsigned long flags;
> @@ -112,7 +112,7 @@ void imx8_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> spin_unlock_irqrestore(>sdev->ipc_lock, flags);
> }
>
> -void imx8_dsp_handle_request(struct imx_dsp_ipc *ipc)
> +static void imx8_dsp_handle_request(struct imx_dsp_ipc *ipc)
> {
> struct imx8_priv *priv = imx_dsp_get_data(ipc);
>
> @@ -333,21 +333,21 @@ static int imx8_remove(struct snd_sof_dev *sdev)
> }
>
> /* on i.MX8 there is 1 to 1 match between type and BAR idx */
> -int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type)
> +static int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type)
> {
> return type;
> }
>
> -void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
> - struct snd_pcm_substream *substream,
> - void *p, size_t sz)
> +static void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
> + struct snd_pcm_substream *substream,
> + void *p, size_t sz)
> {
> sof_mailbox_read(sdev, sdev->dsp_box.offset, p, sz);
> }
>
> -int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
> - struct snd_pcm_substream *substream,
> - const struct sof_ipc_pcm_params_reply *reply)
> +static int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
> + struct snd_pcm_substream *substream,
> + const struct sof_ipc_pcm_params_reply *reply)
> {
> return 0;
> }
> --
> 2.7.4
>
>
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Re: [PATCH] soc: imx: gpcv2: Print the correct error code
On Wed, 2019-08-21 at 18:33 +0200, Guido Günther wrote: > The current code prints 'ret' (thus 0) while it should use 'err'. > > Signed-off-by: Guido Günther Reviewed-by: Daniel Baluta > --- > drivers/soc/imx/gpcv2.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c > index 31b8d002d855..b0dffb06c05d 100644 > --- a/drivers/soc/imx/gpcv2.c > +++ b/drivers/soc/imx/gpcv2.c > @@ -198,7 +198,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct > generic_pm_domain *genpd, > err = regulator_disable(domain->regulator); > if (err) > dev_err(domain->dev, > - "failed to disable regulator: %d\n", > ret); > + "failed to disable regulator: %d\n", > err); > /* Preserve earlier error code */ > ret = ret ?: err; > }
Re: [PATCH] ASoC: fsl_sai: Handle slave mode per TX/RX direction
On Wed, Aug 14, 2019 at 4:01 AM Nicolin Chen wrote: > > On Sun, Aug 11, 2019 at 10:45:17PM +0300, Daniel Baluta wrote: > > From: Viorel Suman > > > > The SAI interface can be a clock supplier or consumer > > as a function of stream direction. e.g SAI can be master > > for Tx and slave for Rx. > > > > Signed-off-by: Viorel Suman > > Signed-off-by: Daniel Baluta > > --- > > sound/soc/fsl/fsl_sai.c | 18 +- > > sound/soc/fsl/fsl_sai.h | 2 +- > > 2 files changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c > > index 4a346fcb5630..69cf3678c859 100644 > > --- a/sound/soc/fsl/fsl_sai.c > > +++ b/sound/soc/fsl/fsl_sai.c > > @@ -273,18 +273,18 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai > > *cpu_dai, > > This function is called for both TX and RX at the same time from > fsl_sai_set_dai_fmt() so I don't actually see how it can operate > in two opposite directions from this change alone. Anything that > I have missed? Good catch. I'm missing a patch that updates fmt after the first call of fsl_sai_set_dai_fmt_tr. Let me update the patch and resend.
Re: [RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
On Wed, Aug 14, 2019 at 4:39 AM Nicolin Chen wrote: > > On Sun, Aug 11, 2019 at 10:55:45PM +0300, Daniel Baluta wrote: > > An audio data frame consists of a number of slots one for each > > channel. In the case of I2S there are 2 data slots / frame. > > > > The maximum number of SAI slots / frame is configurable at > > IP integration time. This affects the width of Mask Register. > > SAI supports up to 32 slots per frame. > > > > The number of datalines is also configurable (up to 8 datalines) > > and affects TCE/RCE and the number of data/FIFO registers. > > > > The number of needed data lines (pins) is computed as follows: > > > > * pins = channels / slots. > > > > This can be computed in hw_params function so lets move TRCE bits > > seting from startup to hw_params. > > > > Signed-off-by: Daniel Baluta > > --- > > sound/soc/fsl/fsl_sai.c | 34 +- > > sound/soc/fsl/fsl_sai.h | 2 +- > > 2 files changed, 14 insertions(+), 22 deletions(-) > > > > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c > > index 69cf3678c859..b70032c82fe2 100644 > > --- a/sound/soc/fsl/fsl_sai.c > > +++ b/sound/soc/fsl/fsl_sai.c > > > @@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream > > *substream, > > > - regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) - > > 1)); > > + regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1)); > > Would this break mono channel audio? Indeed, this isn't good for mono. I see in our internal tree that we have min(channels, slots). This would fix mono, let me think if this is really the right solution. > > > static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai) > > > @@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev) > > return -ENOMEM; > > > > sai->pdev = pdev; > > + > > Seemly unnecessary Oh, ok. Will fix in next version.
[PATCH 1/2] ASoC: fsl_sai: Add support for imx8qm
SAI module on imx8qm features a register map similar with imx6 series
(it doesn't have VERID and PARAM registers at the beginning
of address spece).
Also, it has one FIFO which can help up to 64 * 32 bit samples.
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 69cf3678c859..168e1c9cda5c 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1031,12 +1031,19 @@ static const struct fsl_sai_soc_data
fsl_sai_imx8mq_data = {
.reg_offset = 8,
};
+static const struct fsl_sai_soc_data fsl_sai_imx8qm_data = {
+ .use_imx_pcm = true,
+ .fifo_depth = 64,
+ .reg_offset = 0,
+};
+
static const struct of_device_id fsl_sai_ids[] = {
{ .compatible = "fsl,vf610-sai", .data = _sai_vf610_data },
{ .compatible = "fsl,imx6sx-sai", .data = _sai_imx6sx_data },
{ .compatible = "fsl,imx6ul-sai", .data = _sai_imx6sx_data },
{ .compatible = "fsl,imx7ulp-sai", .data = _sai_imx7ulp_data },
{ .compatible = "fsl,imx8mq-sai", .data = _sai_imx8mq_data },
+ { .compatible = "fsl,imx8qm-sai", .data = _sai_imx8qm_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, fsl_sai_ids);
--
2.17.1
[PATCH 0/2] Add support for i.MX8QM
i.MX8QM SOC integrates 6 SAI instances. Register map is similar with i.MX6 series. Daniel Baluta (2): ASoC: fsl_sai: Add support for imx8qm ASoC: dt-bindings: Introduce compatible string for imx8qm Documentation/devicetree/bindings/sound/fsl-sai.txt | 3 ++- sound/soc/fsl/fsl_sai.c | 7 +++ 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH 2/2] ASoC: dt-bindings: Introduce compatible string for imx8qm
Register map for i.MX8QM is similar with i.MX6 series. Integration of SAI IP into i.MX8QM SOC features a FIFO size of 64 X 32 bits samples. Signed-off-by: Daniel Baluta --- Documentation/devicetree/bindings/sound/fsl-sai.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt index e61c0dc1fc0b..0dc83cc4a236 100644 --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt @@ -9,7 +9,8 @@ Required properties: - compatible : Compatible list, contains "fsl,vf610-sai", "fsl,imx6sx-sai", "fsl,imx6ul-sai", - "fsl,imx7ulp-sai" or "fsl,imx8mq-sai". + "fsl,imx7ulp-sai", "fsl,imx8mq-sai" or + "fsl,imx8qm-sai". - reg: Offset and length of the register set for the device. -- 2.17.1
[RFC PATCH] ASoC: fsl_sai: Enable data lines based on input channels
An audio data frame consists of a number of slots one for each
channel. In the case of I2S there are 2 data slots / frame.
The maximum number of SAI slots / frame is configurable at
IP integration time. This affects the width of Mask Register.
SAI supports up to 32 slots per frame.
The number of datalines is also configurable (up to 8 datalines)
and affects TCE/RCE and the number of data/FIFO registers.
The number of needed data lines (pins) is computed as follows:
* pins = channels / slots.
This can be computed in hw_params function so lets move TRCE bits
seting from startup to hw_params.
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 34 +-
sound/soc/fsl/fsl_sai.h | 2 +-
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 69cf3678c859..b70032c82fe2 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -414,6 +414,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
u32 val_cr4 = 0, val_cr5 = 0;
u32 slots = (channels == 1) ? 2 : channels;
u32 slot_width = word_width;
+ u32 pins;
int ret;
if (sai->slots)
@@ -422,6 +423,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
if (sai->slot_width)
slot_width = sai->slot_width;
+ pins = DIV_ROUND_UP(channels, slots);
+
if (!sai->is_slave_mode[tx]) {
ret = fsl_sai_set_bclk(cpu_dai, tx,
slots * slot_width * params_rate(params));
@@ -480,13 +483,17 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
}
}
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+ FSL_SAI_CR3_TRCE_MASK,
+ FSL_SAI_CR3_TRCE((1 << pins) - 1));
+
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
FSL_SAI_CR5_FBT_MASK, val_cr5);
- regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << channels) -
1));
+ regmap_write(sai->regmap, FSL_SAI_xMR(tx), ~0UL - ((1 << slots) - 1));
return 0;
}
@@ -496,6 +503,10 @@ static int fsl_sai_hw_free(struct snd_pcm_substream
*substream,
{
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+ unsigned int ofs = sai->soc_data->reg_offset;
+
+ regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
+ FSL_SAI_CR3_TRCE_MASK, 0);
if (!sai->is_slave_mode[tx] &&
sai->mclk_streams & BIT(substream->stream)) {
@@ -603,32 +614,13 @@ static int fsl_sai_trigger(struct snd_pcm_substream
*substream, int cmd,
static int fsl_sai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *cpu_dai)
{
- struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- unsigned int ofs = sai->soc_data->reg_offset;
- bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
int ret;
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
- FSL_SAI_CR3_TRCE_MASK,
- FSL_SAI_CR3_TRCE);
-
ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
SNDRV_PCM_HW_PARAM_RATE, _sai_rate_constraints);
-
return ret;
}
-static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
- struct snd_soc_dai *cpu_dai)
-{
- struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- unsigned int ofs = sai->soc_data->reg_offset;
- bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-
- regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx, ofs),
- FSL_SAI_CR3_TRCE_MASK, 0);
-}
-
static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
.set_sysclk = fsl_sai_set_dai_sysclk,
.set_fmt= fsl_sai_set_dai_fmt,
@@ -637,7 +629,6 @@ static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
.hw_free= fsl_sai_hw_free,
.trigger= fsl_sai_trigger,
.startup= fsl_sai_startup,
- .shutdown = fsl_sai_shutdown,
};
static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
@@ -881,6 +872,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
return -ENOMEM;
sai->pdev = pdev;
+
sai->soc_data = of_device_get_match_data(>dev);
sai->is_lsb_first = of_property_read_bool(np, "lsb-first");
diff --git
[PATCH] ASoC: fsl_sai: Handle slave mode per TX/RX direction
From: Viorel Suman
The SAI interface can be a clock supplier or consumer
as a function of stream direction. e.g SAI can be master
for Tx and slave for Rx.
Signed-off-by: Viorel Suman
Signed-off-by: Daniel Baluta
---
sound/soc/fsl/fsl_sai.c | 18 +-
sound/soc/fsl/fsl_sai.h | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 4a346fcb5630..69cf3678c859 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -273,18 +273,18 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai
*cpu_dai,
case SND_SOC_DAIFMT_CBS_CFS:
val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
- sai->is_slave_mode = false;
+ sai->is_slave_mode[tx] = false;
break;
case SND_SOC_DAIFMT_CBM_CFM:
- sai->is_slave_mode = true;
+ sai->is_slave_mode[tx] = true;
break;
case SND_SOC_DAIFMT_CBS_CFM:
val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
- sai->is_slave_mode = false;
+ sai->is_slave_mode[tx] = false;
break;
case SND_SOC_DAIFMT_CBM_CFS:
val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
- sai->is_slave_mode = true;
+ sai->is_slave_mode[tx] = true;
break;
default:
return -EINVAL;
@@ -326,7 +326,7 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool
tx, u32 freq)
int ret = 0;
/* Don't apply to slave mode */
- if (sai->is_slave_mode)
+ if (sai->is_slave_mode[tx])
return 0;
for (id = 0; id < FSL_SAI_MCLK_MAX; id++) {
@@ -422,7 +422,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
if (sai->slot_width)
slot_width = sai->slot_width;
- if (!sai->is_slave_mode) {
+ if (!sai->is_slave_mode[tx]) {
ret = fsl_sai_set_bclk(cpu_dai, tx,
slots * slot_width * params_rate(params));
if (ret)
@@ -458,7 +458,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream
*substream,
* error.
*/
- if (!sai->is_slave_mode) {
+ if (!sai->is_slave_mode[tx]) {
if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) {
regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs),
FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
@@ -497,7 +497,7 @@ static int fsl_sai_hw_free(struct snd_pcm_substream
*substream,
struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
- if (!sai->is_slave_mode &&
+ if (!sai->is_slave_mode[tx] &&
sai->mclk_streams & BIT(substream->stream)) {
clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[tx]]);
sai->mclk_streams &= ~BIT(substream->stream);
@@ -581,7 +581,7 @@ static int fsl_sai_trigger(struct snd_pcm_substream
*substream, int cmd,
* This is a hardware bug, and will be fix in the
* next sai version.
*/
- if (!sai->is_slave_mode) {
+ if (!sai->is_slave_mode[tx]) {
/* Software Reset for both Tx and Rx */
regmap_write(sai->regmap, FSL_SAI_TCSR(ofs),
FSL_SAI_CSR_SR);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index b89b0ca26053..c2c43a7d9ba1 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -167,7 +167,7 @@ struct fsl_sai {
struct clk *bus_clk;
struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
- bool is_slave_mode;
+ bool is_slave_mode[2];
bool is_lsb_first;
bool is_dsp_mode;
bool synchronous[2];
--
2.17.1
