[PATCH] remoteproc: mediatek: Don't print error when optional scp handle is missing

Nícolas F. R. A. Prado posted 1 patch 3 months, 2 weeks ago
drivers/remoteproc/mtk_scp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH] remoteproc: mediatek: Don't print error when optional scp handle is missing
Posted by Nícolas F. R. A. Prado 3 months, 2 weeks ago
The scp_get() helper has two users: the mtk-vcodec and the mtk-mdp3
drivers. mdp3 considers the mediatek,scp phandle optional, and when it's
missing mdp3 will directly look for the scp node based on compatible.

For that reason printing an error in the helper when the handle is
missing is wrong, as it's not always an actual error. Besides, the other
user, vcodec, already prints an error message on its own when the helper
fails so this message doesn't add that much information.

Remove the error message from the helper. This gets rid of the deceptive
error on MT8195-Tomato:

  mtk-mdp3 14001000.dma-controller: can't get SCP node

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/remoteproc/mtk_scp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index b885a9a041e4..f813117b6312 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -37,10 +37,8 @@ struct mtk_scp *scp_get(struct platform_device *pdev)
 	struct platform_device *scp_pdev;
 
 	scp_node = of_parse_phandle(dev->of_node, "mediatek,scp", 0);
-	if (!scp_node) {
-		dev_err(dev, "can't get SCP node\n");
+	if (!scp_node)
 		return NULL;
-	}
 
 	scp_pdev = of_find_device_by_node(scp_node);
 	of_node_put(scp_node);

---
base-commit: d97496ca23a2d4ee80b7302849404859d9058bcd
change-id: 20240605-mt8195-dma-scp-node-err-6a8dea26d4f5

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>

Re: [PATCH] remoteproc: mediatek: Don't print error when optional scp handle is missing
Posted by AngeloGioacchino Del Regno 3 months, 2 weeks ago
Il 05/06/24 21:35, Nícolas F. R. A. Prado ha scritto:
> The scp_get() helper has two users: the mtk-vcodec and the mtk-mdp3
> drivers. mdp3 considers the mediatek,scp phandle optional, and when it's
> missing mdp3 will directly look for the scp node based on compatible.
> 
> For that reason printing an error in the helper when the handle is
> missing is wrong, as it's not always an actual error. Besides, the other
> user, vcodec, already prints an error message on its own when the helper
> fails so this message doesn't add that much information.
> 
> Remove the error message from the helper. This gets rid of the deceptive
> error on MT8195-Tomato:
> 
>    mtk-mdp3 14001000.dma-controller: can't get SCP node

I'm way more for giving it a SCP handle instead of silencing this print... the
SCP handle way *is* the correct one and I prefer it, as that'd also be the only
way to support Dual-Core SCP in MDP3.

I really want to see hardcoded stuff disappear and I really really *really* want
to see more consistency across DT nodes in MediaTek platforms.

So, still a one-line change, but do it on the devicetree instead of here please.

Cheers,
Angelo

> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>   drivers/remoteproc/mtk_scp.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index b885a9a041e4..f813117b6312 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -37,10 +37,8 @@ struct mtk_scp *scp_get(struct platform_device *pdev)
>   	struct platform_device *scp_pdev;
>   
>   	scp_node = of_parse_phandle(dev->of_node, "mediatek,scp", 0);
> -	if (!scp_node) {
> -		dev_err(dev, "can't get SCP node\n");
> +	if (!scp_node)
>   		return NULL;
> -	}
>   
>   	scp_pdev = of_find_device_by_node(scp_node);
>   	of_node_put(scp_node);
> 
> ---
> base-commit: d97496ca23a2d4ee80b7302849404859d9058bcd
> change-id: 20240605-mt8195-dma-scp-node-err-6a8dea26d4f5
> 
> Best regards,

Re: [PATCH] remoteproc: mediatek: Don't print error when optional scp handle is missing
Posted by Nícolas F. R. A. Prado 3 months, 2 weeks ago
On Thu, Jun 06, 2024 at 12:50:56PM +0200, AngeloGioacchino Del Regno wrote:
> Il 05/06/24 21:35, Nícolas F. R. A. Prado ha scritto:
> > The scp_get() helper has two users: the mtk-vcodec and the mtk-mdp3
> > drivers. mdp3 considers the mediatek,scp phandle optional, and when it's
> > missing mdp3 will directly look for the scp node based on compatible.
> > 
> > For that reason printing an error in the helper when the handle is
> > missing is wrong, as it's not always an actual error. Besides, the other
> > user, vcodec, already prints an error message on its own when the helper
> > fails so this message doesn't add that much information.
> > 
> > Remove the error message from the helper. This gets rid of the deceptive
> > error on MT8195-Tomato:
> > 
> >    mtk-mdp3 14001000.dma-controller: can't get SCP node
> 
> I'm way more for giving it a SCP handle instead of silencing this print... the
> SCP handle way *is* the correct one and I prefer it, as that'd also be the only
> way to support Dual-Core SCP in MDP3.
> 
> I really want to see hardcoded stuff disappear and I really really *really* want
> to see more consistency across DT nodes in MediaTek platforms.
> 
> So, still a one-line change, but do it on the devicetree instead of here please.

Sure. So the missing scp phandle case is maintained exclusively for
backwards-compatibility. And we're ok that the old DTs will keep getting the
error.

I'll send a v2 adding the phandle in the DT instead.

Thanks,
Nícolas