[PATCH v2 0/4] ASoC: fsl: Support register and unregister rpmsg sound card through remoteproc
echo /lib/firmware/fw.elf > /sys/class/remoteproc/remoteproc0/firmware (A) echo start > /sys/class/remoteproc/remoteproc0/state (B) echo stop > /sys/class/remoteproc/remoteproc0/state
The rpmsg sound card is registered in (A) and unregistered in (B). After "start", imx-audio-rpmsg registers devices for ASoC platform driver and machine driver. Then sound card is registered. After "stop", imx-audio-rpmsg unregisters devices for ASoC platform driver and machine driver. Then sound card is unregistered.
changes in v2 - Fix build errors reported by kernel test robot
Chancel Liu (4): ASoC: fsl: imx_pcm_rpmsg: Register component with rpmsg channel name ASoC: fsl: imx-audio-rpmsg: Register device with rpmsg channel name ASoC: fsl: Let imx-audio-rpmsg register platform device for card ASoC: fsl: imx-rpmsg: Update to correct DT node
sound/soc/fsl/fsl_rpmsg.c | 11 ----------- sound/soc/fsl/imx-audio-rpmsg.c | 21 ++++++++++++++++++--- sound/soc/fsl/imx-pcm-rpmsg.c | 11 ++++++++--- sound/soc/fsl/imx-rpmsg.c | 21 ++++++++++++++++++--- 4 files changed, 44 insertions(+), 20 deletions(-)
-- 2.43.0
Machine driver uses rpmsg channel name to link this platform component. However if the component is re-registerd card will not find this new created component in snd_soc_try_rebind_card().
Explicitly register this component with rpmsg channel name so that card can always find this component.
Signed-off-by: Chancel Liu chancel.liu@nxp.com --- sound/soc/fsl/imx-pcm-rpmsg.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/imx-pcm-rpmsg.c b/sound/soc/fsl/imx-pcm-rpmsg.c index fb9244c1e9c5..2b9e4bb5e0f7 100644 --- a/sound/soc/fsl/imx-pcm-rpmsg.c +++ b/sound/soc/fsl/imx-pcm-rpmsg.c @@ -732,9 +732,6 @@ static int imx_rpmsg_pcm_probe(struct platform_device *pdev) goto fail; }
- /* platform component name is used by machine driver to link with */ - component->name = info->rpdev->id.name; - #ifdef CONFIG_DEBUG_FS component->debugfs_prefix = "rpmsg"; #endif @@ -822,9 +819,17 @@ static const struct dev_pm_ops imx_rpmsg_pcm_pm_ops = { imx_rpmsg_pcm_resume) };
+static const struct platform_device_id imx_rpmsg_pcm_id_table[] = { + { .name = "rpmsg-audio-channel" }, + { .name = "rpmsg-micfil-channel" }, + { } +}; +MODULE_DEVICE_TABLE(platform, imx_rpmsg_pcm_id_table); + static struct platform_driver imx_pcm_rpmsg_driver = { .probe = imx_rpmsg_pcm_probe, .remove_new = imx_rpmsg_pcm_remove, + .id_table = imx_rpmsg_pcm_id_table, .driver = { .name = IMX_PCM_DRV_NAME, .pm = &imx_rpmsg_pcm_pm_ops,
This rpmsg driver registers device for ASoC platform driver. To align with platform driver use rpmsg channel name to create device.
Signed-off-by: Chancel Liu chancel.liu@nxp.com --- sound/soc/fsl/imx-audio-rpmsg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/imx-audio-rpmsg.c b/sound/soc/fsl/imx-audio-rpmsg.c index 289e47c03d40..40820d5ad92d 100644 --- a/sound/soc/fsl/imx-audio-rpmsg.c +++ b/sound/soc/fsl/imx-audio-rpmsg.c @@ -87,8 +87,8 @@ static int imx_audio_rpmsg_probe(struct rpmsg_device *rpdev)
/* Register platform driver for rpmsg routine */ data->rpmsg_pdev = platform_device_register_data(&rpdev->dev, - IMX_PCM_DRV_NAME, - PLATFORM_DEVID_AUTO, + rpdev->id.name, + PLATFORM_DEVID_NONE, NULL, 0); if (IS_ERR(data->rpmsg_pdev)) { dev_err(&rpdev->dev, "failed to register rpmsg platform.\n");
Let imx-audio-rpmsg register platform device for card. So that card register and unregister can be controlled by rpmsg driver's register and unregister.
Signed-off-by: Chancel Liu chancel.liu@nxp.com --- sound/soc/fsl/fsl_rpmsg.c | 11 ----------- sound/soc/fsl/imx-audio-rpmsg.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/sound/soc/fsl/fsl_rpmsg.c b/sound/soc/fsl/fsl_rpmsg.c index 00852f174a69..53bd517e59d6 100644 --- a/sound/soc/fsl/fsl_rpmsg.c +++ b/sound/soc/fsl/fsl_rpmsg.c @@ -240,17 +240,6 @@ static int fsl_rpmsg_probe(struct platform_device *pdev) if (ret) goto err_pm_disable;
- rpmsg->card_pdev = platform_device_register_data(&pdev->dev, - "imx-audio-rpmsg", - PLATFORM_DEVID_AUTO, - NULL, - 0); - if (IS_ERR(rpmsg->card_pdev)) { - dev_err(&pdev->dev, "failed to register rpmsg card\n"); - ret = PTR_ERR(rpmsg->card_pdev); - goto err_pm_disable; - } - return 0;
err_pm_disable: diff --git a/sound/soc/fsl/imx-audio-rpmsg.c b/sound/soc/fsl/imx-audio-rpmsg.c index 40820d5ad92d..60f27b0a2530 100644 --- a/sound/soc/fsl/imx-audio-rpmsg.c +++ b/sound/soc/fsl/imx-audio-rpmsg.c @@ -12,6 +12,7 @@ */ struct imx_audio_rpmsg { struct platform_device *rpmsg_pdev; + struct platform_device *card_pdev; };
static int imx_audio_rpmsg_cb(struct rpmsg_device *rpdev, void *data, int len, @@ -95,6 +96,16 @@ static int imx_audio_rpmsg_probe(struct rpmsg_device *rpdev) ret = PTR_ERR(data->rpmsg_pdev); }
+ data->card_pdev = platform_device_register_data(&rpdev->dev, + "imx-audio-rpmsg", + PLATFORM_DEVID_AUTO, + rpdev->id.name, + strlen(rpdev->id.name)); + if (IS_ERR(data->card_pdev)) { + dev_err(&rpdev->dev, "failed to register rpmsg card.\n"); + ret = PTR_ERR(data->card_pdev); + } + return ret; }
@@ -105,6 +116,9 @@ static void imx_audio_rpmsg_remove(struct rpmsg_device *rpdev) if (data->rpmsg_pdev) platform_device_unregister(data->rpmsg_pdev);
+ if (data->card_pdev) + platform_device_unregister(data->card_pdev); + dev_info(&rpdev->dev, "audio rpmsg driver is removed\n"); }
@@ -113,6 +127,7 @@ static struct rpmsg_device_id imx_audio_rpmsg_id_table[] = { { .name = "rpmsg-micfil-channel" }, { }, }; +MODULE_DEVICE_TABLE(rpmsg, imx_audio_rpmsg_id_table);
static struct rpmsg_driver imx_audio_rpmsg_driver = { .drv.name = "imx_audio_rpmsg", @@ -126,5 +141,5 @@ module_rpmsg_driver(imx_audio_rpmsg_driver);
MODULE_DESCRIPTION("Freescale SoC Audio RPMSG interface"); MODULE_AUTHOR("Shengjiu Wang shengjiu.wang@nxp.com"); -MODULE_ALIAS("platform:imx_audio_rpmsg"); +MODULE_ALIAS("rpmsg:imx_audio_rpmsg"); MODULE_LICENSE("GPL v2");
Platform device for card to probe is registered in imx-audio-rpmsg. According to this change DT node of ASoC CPU DAI device is updated.
Signed-off-by: Chancel Liu chancel.liu@nxp.com --- sound/soc/fsl/imx-rpmsg.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c index e5bd63dab10c..2686125b3043 100644 --- a/sound/soc/fsl/imx-rpmsg.c +++ b/sound/soc/fsl/imx-rpmsg.c @@ -108,10 +108,9 @@ static int imx_rpmsg_late_probe(struct snd_soc_card *card) static int imx_rpmsg_probe(struct platform_device *pdev) { struct snd_soc_dai_link_component *dlc; - struct device *dev = pdev->dev.parent; /* rpmsg_pdev is the platform device for the rpmsg node that probed us */ - struct platform_device *rpmsg_pdev = to_platform_device(dev); - struct device_node *np = rpmsg_pdev->dev.of_node; + struct platform_device *rpmsg_pdev = NULL; + struct device_node *np = NULL; struct of_phandle_args args; const char *platform_name; struct imx_rpmsg *data; @@ -127,6 +126,22 @@ static int imx_rpmsg_probe(struct platform_device *pdev) goto fail; }
+ if (!strcmp(pdev->dev.platform_data, "rpmsg-micfil-channel")) + np = of_find_node_by_name(NULL, "rpmsg_micfil"); + else + np = of_find_node_by_name(NULL, "rpmsg_audio"); + if (!np) { + dev_err(&pdev->dev, "failed to parse node\n"); + ret = -EINVAL; + goto fail; + } + rpmsg_pdev = of_find_device_by_node(np); + if (!rpmsg_pdev) { + dev_err(&pdev->dev, "failed to parse platform device\n"); + ret = -EINVAL; + goto fail; + } + ret = of_reserved_mem_device_init_by_idx(&pdev->dev, np, 0); if (ret) dev_warn(&pdev->dev, "no reserved DMA memory\n");
On 07/03/2024 08:44, Chancel Liu wrote:
Platform device for card to probe is registered in imx-audio-rpmsg. According to this change DT node of ASoC CPU DAI device is updated.
Signed-off-by: Chancel Liu chancel.liu@nxp.com
sound/soc/fsl/imx-rpmsg.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c index e5bd63dab10c..2686125b3043 100644 --- a/sound/soc/fsl/imx-rpmsg.c +++ b/sound/soc/fsl/imx-rpmsg.c @@ -108,10 +108,9 @@ static int imx_rpmsg_late_probe(struct snd_soc_card *card) static int imx_rpmsg_probe(struct platform_device *pdev) { struct snd_soc_dai_link_component *dlc;
- struct device *dev = pdev->dev.parent; /* rpmsg_pdev is the platform device for the rpmsg node that probed us */
- struct platform_device *rpmsg_pdev = to_platform_device(dev);
- struct device_node *np = rpmsg_pdev->dev.of_node;
- struct platform_device *rpmsg_pdev = NULL;
- struct device_node *np = NULL; struct of_phandle_args args; const char *platform_name; struct imx_rpmsg *data;
@@ -127,6 +126,22 @@ static int imx_rpmsg_probe(struct platform_device *pdev) goto fail; }
- if (!strcmp(pdev->dev.platform_data, "rpmsg-micfil-channel"))
np = of_find_node_by_name(NULL, "rpmsg_micfil");- else
np = of_find_node_by_name(NULL, "rpmsg_audio");
Why do you create ABI on node names? Where is it documented? Why can't you use phandles?
Best regards, Krzysztof
On 07/03/2024 08:44, Chancel Liu wrote:
Platform device for card to probe is registered in imx-audio-rpmsg. According to this change DT node of ASoC CPU DAI device is updated.
Signed-off-by: Chancel Liu chancel.liu@nxp.com
sound/soc/fsl/imx-rpmsg.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/sound/soc/fsl/imx-rpmsg.c b/sound/soc/fsl/imx-rpmsg.c index e5bd63dab10c..2686125b3043 100644 --- a/sound/soc/fsl/imx-rpmsg.c +++ b/sound/soc/fsl/imx-rpmsg.c @@ -108,10 +108,9 @@ static int imx_rpmsg_late_probe(struct
snd_soc_card *card)
static int imx_rpmsg_probe(struct platform_device *pdev) { struct snd_soc_dai_link_component *dlc;
struct device *dev = pdev->dev.parent; /* rpmsg_pdev is the platform device for the rpmsg node that probedus */
struct platform_device *rpmsg_pdev = to_platform_device(dev);struct device_node *np = rpmsg_pdev->dev.of_node;
struct platform_device *rpmsg_pdev = NULL;struct device_node *np = NULL; struct of_phandle_args args; const char *platform_name; struct imx_rpmsg *data;@@ -127,6 +126,22 @@ static int imx_rpmsg_probe(struct platform_device
*pdev)
goto fail; }
if (!strcmp(pdev->dev.platform_data, "rpmsg-micfil-channel"))np = of_find_node_by_name(NULL, "rpmsg_micfil");elsenp = of_find_node_by_name(NULL, "rpmsg_audio");Why do you create ABI on node names? Where is it documented? Why can't you use phandles?
Best regards, Krzysztof
Thanks for your reminder. Truly I shouldn't use undocumented bindings. I will use “fsl,rpmsg-channel-name” to refine patch set. Please help review next version.
Regards, Chancel Liu
On 11/03/2024 08:33, Chancel Liu wrote:
@@ -127,6 +126,22 @@ static int imx_rpmsg_probe(struct platform_device
*pdev)
goto fail; }
if (!strcmp(pdev->dev.platform_data, "rpmsg-micfil-channel"))np = of_find_node_by_name(NULL, "rpmsg_micfil");elsenp = of_find_node_by_name(NULL, "rpmsg_audio");Why do you create ABI on node names? Where is it documented? Why can't you use phandles?
Best regards, Krzysztof
Thanks for your reminder. Truly I shouldn't use undocumented bindings. I will use “fsl,rpmsg-channel-name” to refine patch set. Please help review next version.
Instead of hard-coding node names in the driver you want to put it in "fsl,rpmsg-channel-name" property? I don't follow. I recommended instead using phandles, care to address that?
Best regards, Krzysztof
On 11/03/2024 08:33, Chancel Liu wrote:
@@ -127,6 +126,22 @@ static int imx_rpmsg_probe(struct
platform_device
*pdev)
goto fail; }
if (!strcmp(pdev->dev.platform_data, "rpmsg-micfil-channel"))np = of_find_node_by_name(NULL, "rpmsg_micfil");elsenp = of_find_node_by_name(NULL, "rpmsg_audio");Why do you create ABI on node names? Where is it documented? Why
can't
you use phandles?
Best regards, Krzysztof
Thanks for your reminder. Truly I shouldn't use undocumented bindings. I
will
use “fsl,rpmsg-channel-name” to refine patch set. Please help review next version.
Instead of hard-coding node names in the driver you want to put it in "fsl,rpmsg-channel-name" property? I don't follow. I recommended instead using phandles, care to address that?
imx-rpmsg is ASoC machine driver and fsl_rpmsg is ASoC CPU DAI driver. In imx-rpmsg, driver needs to get CPU DAI DT node for hardware configuration. So imx-rpmsg needs some "information" to find the correct DT node. As you recommended, it's not wise to use hard-coding node name. Also the device of imx-rpmsg is created by imx-audio-rpmsg so it can't directly get phandle of CPU DAI node.
Sorry for unclear statement. "fsl,rpmsg-channel-name" is the property of rpmsg channel name. Each rpmsg sound card sits on one rpmsg channel. So I decide to use rpmsg channel name to connect all parts of this sound card. If the CPU DAI is registerd with rpmsg channel name then imx-rpmsg can easily get the DT node by this name.
Regards, Chancel Liu
participants (2)
-
Chancel Liu -
Krzysztof Kozlowski