linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dsi: fix error checks and return values for DSI xmit functions
@ 2022-04-01 23:11 Dmitry Baryshkov
  2022-04-05 16:28 ` Abhinav Kumar
  2022-04-16  9:12 ` Marijn Suijten
  0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Baryshkov @ 2022-04-01 23:11 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
	dri-devel, freedreno, Dan Carpenter

As noticed by Dan ([1] an the followup thread) there are multiple issues
with the return values for MSM DSI command transmission callback. In
the error case it can easily return a positive value when it should
have returned a proper error code.

This commits attempts to fix these issues both in TX and in RX paths.

[1]: https://lore.kernel.org/linux-arm-msm/20211001123617.GH2283@kili/

Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index d51e70fab93d..8925f60fd9ec 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1341,10 +1341,10 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
 			dsi_get_bpp(msm_host->format) / 8;
 
 	len = dsi_cmd_dma_add(msm_host, msg);
-	if (!len) {
+	if (len < 0) {
 		pr_err("%s: failed to add cmd type = 0x%x\n",
 			__func__,  msg->type);
-		return -EINVAL;
+		return len;
 	}
 
 	/* for video mode, do not send cmds more than
@@ -1363,10 +1363,14 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
 	}
 
 	ret = dsi_cmd_dma_tx(msm_host, len);
-	if (ret < len) {
-		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d\n",
-			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len);
-		return -ECOMM;
+	if (ret < 0) {
+		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d, ret=%d\n",
+			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len, ret);
+		return ret;
+	} else if (ret < len) {
+		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, ret=%d len=%d\n",
+			__func__, msg->type, (*(u8 *)(msg->tx_buf)), ret, len);
+		return -EIO;
 	}
 
 	return len;
@@ -2092,9 +2096,12 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
 		}
 
 		ret = dsi_cmds2buf_tx(msm_host, msg);
-		if (ret < msg->tx_len) {
+		if (ret < 0) {
 			pr_err("%s: Read cmd Tx failed, %d\n", __func__, ret);
 			return ret;
+		} else if (ret < msg->tx_len) {
+			pr_err("%s: Read cmd Tx failed, too short: %d\n", __func__, ret);
+			return -ECOMM;
 		}
 
 		/*
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/msm/dsi: fix error checks and return values for DSI xmit functions
  2022-04-01 23:11 [PATCH] drm/msm/dsi: fix error checks and return values for DSI xmit functions Dmitry Baryshkov
@ 2022-04-05 16:28 ` Abhinav Kumar
  2022-04-16  9:12 ` Marijn Suijten
  1 sibling, 0 replies; 4+ messages in thread
From: Abhinav Kumar @ 2022-04-05 16:28 UTC (permalink / raw)
  To: Dmitry Baryshkov, Bjorn Andersson, Rob Clark, Sean Paul
  Cc: Stephen Boyd, David Airlie, Daniel Vetter, linux-arm-msm,
	dri-devel, freedreno, Dan Carpenter



On 4/1/2022 4:11 PM, Dmitry Baryshkov wrote:
> As noticed by Dan ([1] an the followup thread) there are multiple issues
> with the return values for MSM DSI command transmission callback. In
> the error case it can easily return a positive value when it should
> have returned a proper error code.
> 
> This commits attempts to fix these issues both in TX and in RX paths.
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20211001123617.GH2283@kili/
> 
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Looks reasonable to me,

Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

> ---
>   drivers/gpu/drm/msm/dsi/dsi_host.c | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d51e70fab93d..8925f60fd9ec 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1341,10 +1341,10 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
>   			dsi_get_bpp(msm_host->format) / 8;
>   
>   	len = dsi_cmd_dma_add(msm_host, msg);
> -	if (!len) {
> +	if (len < 0) {
>   		pr_err("%s: failed to add cmd type = 0x%x\n",
>   			__func__,  msg->type);
> -		return -EINVAL;
> +		return len;
>   	}
>   
>   	/* for video mode, do not send cmds more than
> @@ -1363,10 +1363,14 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
>   	}
>   
>   	ret = dsi_cmd_dma_tx(msm_host, len);
> -	if (ret < len) {
> -		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d\n",
> -			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len);
> -		return -ECOMM;
> +	if (ret < 0) {
> +		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d, ret=%d\n",
> +			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len, ret);
> +		return ret;
> +	} else if (ret < len) {
> +		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, ret=%d len=%d\n",
> +			__func__, msg->type, (*(u8 *)(msg->tx_buf)), ret, len);
> +		return -EIO;
>   	}
>   
>   	return len;
> @@ -2092,9 +2096,12 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
>   		}
>   
>   		ret = dsi_cmds2buf_tx(msm_host, msg);
> -		if (ret < msg->tx_len) {
> +		if (ret < 0) {
>   			pr_err("%s: Read cmd Tx failed, %d\n", __func__, ret);
>   			return ret;
> +		} else if (ret < msg->tx_len) {
> +			pr_err("%s: Read cmd Tx failed, too short: %d\n", __func__, ret);
> +			return -ECOMM;
>   		}
>   
>   		/*

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: drm/msm/dsi: fix error checks and return values for DSI xmit functions
  2022-04-01 23:11 [PATCH] drm/msm/dsi: fix error checks and return values for DSI xmit functions Dmitry Baryshkov
  2022-04-05 16:28 ` Abhinav Kumar
@ 2022-04-16  9:12 ` Marijn Suijten
  2022-04-16 18:10   ` Dmitry Baryshkov
  1 sibling, 1 reply; 4+ messages in thread
From: Marijn Suijten @ 2022-04-16  9:12 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar,
	David Airlie, linux-arm-msm, dri-devel, Stephen Boyd,
	Daniel Vetter, freedreno, Dan Carpenter

Hi Dmitry,

On 2022-04-02 02:11:04, Dmitry Baryshkov wrote:
> As noticed by Dan ([1] an the followup thread) there are multiple issues
> with the return values for MSM DSI command transmission callback. In
> the error case it can easily return a positive value when it should
> have returned a proper error code.
> 
> This commits attempts to fix these issues both in TX and in RX paths.
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20211001123617.GH2283@kili/
> 
> Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>

Thank you for your patience waiting for the requested tests; this patch
seems to have no adverse effect on our cmdmode panels.

Tested-by: Marijn Suijten <marijn.suijten@somainline.org>

On the following devices:
- Sony Xperia X (Loire Suzu, MSM8976), on Linux 5.17;
- Sony Xperia 10 II (Seine PDX201, SM6125), on -next 20220318;
- Sony Xperia XA2 Ultra (Nile Discovery, SDM630), on Linux 5.16.

Apologies for the older kernel versions, that's what happens when having
too many patches to dig through and too little hobby time to send them.
Let me know if there's a patch dependency that you like to be included.

- Marijn

> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index d51e70fab93d..8925f60fd9ec 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1341,10 +1341,10 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
>  			dsi_get_bpp(msm_host->format) / 8;
>  
>  	len = dsi_cmd_dma_add(msm_host, msg);
> -	if (!len) {
> +	if (len < 0) {
>  		pr_err("%s: failed to add cmd type = 0x%x\n",
>  			__func__,  msg->type);
> -		return -EINVAL;
> +		return len;
>  	}
>  
>  	/* for video mode, do not send cmds more than
> @@ -1363,10 +1363,14 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
>  	}
>  
>  	ret = dsi_cmd_dma_tx(msm_host, len);
> -	if (ret < len) {
> -		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d\n",
> -			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len);
> -		return -ECOMM;
> +	if (ret < 0) {
> +		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d, ret=%d\n",
> +			__func__, msg->type, (*(u8 *)(msg->tx_buf)), len, ret);
> +		return ret;
> +	} else if (ret < len) {
> +		pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, ret=%d len=%d\n",
> +			__func__, msg->type, (*(u8 *)(msg->tx_buf)), ret, len);
> +		return -EIO;
>  	}
>  
>  	return len;
> @@ -2092,9 +2096,12 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
>  		}
>  
>  		ret = dsi_cmds2buf_tx(msm_host, msg);
> -		if (ret < msg->tx_len) {
> +		if (ret < 0) {
>  			pr_err("%s: Read cmd Tx failed, %d\n", __func__, ret);
>  			return ret;
> +		} else if (ret < msg->tx_len) {
> +			pr_err("%s: Read cmd Tx failed, too short: %d\n", __func__, ret);
> +			return -ECOMM;
>  		}
>  
>  		/*

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: drm/msm/dsi: fix error checks and return values for DSI xmit functions
  2022-04-16  9:12 ` Marijn Suijten
@ 2022-04-16 18:10   ` Dmitry Baryshkov
  0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Baryshkov @ 2022-04-16 18:10 UTC (permalink / raw)
  To: Marijn Suijten
  Cc: Bjorn Andersson, Rob Clark, Sean Paul, Abhinav Kumar,
	David Airlie, linux-arm-msm, dri-devel, Stephen Boyd,
	Daniel Vetter, freedreno, Dan Carpenter

On Sat, 16 Apr 2022 at 12:12, Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> Hi Dmitry,
>
> On 2022-04-02 02:11:04, Dmitry Baryshkov wrote:
> > As noticed by Dan ([1] an the followup thread) there are multiple issues
> > with the return values for MSM DSI command transmission callback. In
> > the error case it can easily return a positive value when it should
> > have returned a proper error code.
> >
> > This commits attempts to fix these issues both in TX and in RX paths.
> >
> > [1]: https://lore.kernel.org/linux-arm-msm/20211001123617.GH2283@kili/
> >
> > Fixes: a689554ba6ed ("drm/msm: Initial add DSI connector support")
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> Thank you for your patience waiting for the requested tests; this patch
> seems to have no adverse effect on our cmdmode panels.
>
> Tested-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> On the following devices:
> - Sony Xperia X (Loire Suzu, MSM8976), on Linux 5.17;
> - Sony Xperia 10 II (Seine PDX201, SM6125), on -next 20220318;
> - Sony Xperia XA2 Ultra (Nile Discovery, SDM630), on Linux 5.16.
>
> Apologies for the older kernel versions, that's what happens when having
> too many patches to dig through and too little hobby time to send them.
> Let me know if there's a patch dependency that you like to be included.

Thank you for the confirmation! No, no hidden dependencies.

>
> - Marijn
>
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index d51e70fab93d..8925f60fd9ec 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -1341,10 +1341,10 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
> >                       dsi_get_bpp(msm_host->format) / 8;
> >
> >       len = dsi_cmd_dma_add(msm_host, msg);
> > -     if (!len) {
> > +     if (len < 0) {
> >               pr_err("%s: failed to add cmd type = 0x%x\n",
> >                       __func__,  msg->type);
> > -             return -EINVAL;
> > +             return len;
> >       }
> >
> >       /* for video mode, do not send cmds more than
> > @@ -1363,10 +1363,14 @@ static int dsi_cmds2buf_tx(struct msm_dsi_host *msm_host,
> >       }
> >
> >       ret = dsi_cmd_dma_tx(msm_host, len);
> > -     if (ret < len) {
> > -             pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d\n",
> > -                     __func__, msg->type, (*(u8 *)(msg->tx_buf)), len);
> > -             return -ECOMM;
> > +     if (ret < 0) {
> > +             pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, len=%d, ret=%d\n",
> > +                     __func__, msg->type, (*(u8 *)(msg->tx_buf)), len, ret);
> > +             return ret;
> > +     } else if (ret < len) {
> > +             pr_err("%s: cmd dma tx failed, type=0x%x, data0=0x%x, ret=%d len=%d\n",
> > +                     __func__, msg->type, (*(u8 *)(msg->tx_buf)), ret, len);
> > +             return -EIO;
> >       }
> >
> >       return len;
> > @@ -2092,9 +2096,12 @@ int msm_dsi_host_cmd_rx(struct mipi_dsi_host *host,
> >               }
> >
> >               ret = dsi_cmds2buf_tx(msm_host, msg);
> > -             if (ret < msg->tx_len) {
> > +             if (ret < 0) {
> >                       pr_err("%s: Read cmd Tx failed, %d\n", __func__, ret);
> >                       return ret;
> > +             } else if (ret < msg->tx_len) {
> > +                     pr_err("%s: Read cmd Tx failed, too short: %d\n", __func__, ret);
> > +                     return -ECOMM;
> >               }
> >
> >               /*



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-16 18:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 23:11 [PATCH] drm/msm/dsi: fix error checks and return values for DSI xmit functions Dmitry Baryshkov
2022-04-05 16:28 ` Abhinav Kumar
2022-04-16  9:12 ` Marijn Suijten
2022-04-16 18:10   ` Dmitry Baryshkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).