| Message ID | 20240307074437.1472593-5-chancel.liu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | ASoC: fsl: Support register and unregister rpmsg sound card through remoteproc | expand |
> 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 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"); >>> + 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 > > 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"); > >>> + 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 > > > > 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
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");
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(-)