Re: [PATCH 1/4] drm/dsi: Make mipi_dsi_dcs_write() return ssize_t

[Andrzej Hajda <a.hajda@samsung.com>]

Hi Thierry,

YoungJun's comment refreshed my memory about mipi_dsi_dcs_write return
value. It should be rather int than ssize_t. Why?
.transfer() returns the number of read bytes or error, but in case
of dcs write no bytes are read, so it in fact returns error or 0.
This is why return value was implemented originally as int.
So I do not think this patch is necessary.

Regards
Andrzej


On 07/22/2014 12:05 PM, Andrzej Hajda wrote:
> On 07/22/2014 11:50 AM, YoungJun Cho wrote:
>> Hi,
>>
>> On 07/22/2014 04:28 PM, Andrzej Hajda wrote:
>>> Hi Thierry,
>>>
>>> Thanks for the patch.
>>>
>>> On 07/22/2014 09:12 AM, Thierry Reding wrote:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> This function returns the value of the struct mipi_dsi_host_ops'
>>>> .transfer() so make sure the return types are consistent.
>>>>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> Acked-by: Andrzej Hajda <a.hajda@samsung.com>
>>> --
>>> Regards
>>> Andrzej
>>>> ---
>>>>   drivers/gpu/drm/drm_mipi_dsi.c        | 4 ++--
>>>>   drivers/gpu/drm/panel/panel-s6e8aa0.c | 4 ++--
>>>>   include/drm/drm_mipi_dsi.h            | 4 ++--
>>>>   3 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
>>>> index e633df2f68d8..6d2fd2077dae 100644
>>>> --- a/drivers/gpu/drm/drm_mipi_dsi.c
>>>> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
>>>> @@ -205,8 +205,8 @@ EXPORT_SYMBOL(mipi_dsi_detach);
>>>>    * @data: pointer to the command followed by parameters
>>>>    * @len: length of @data
>>>>    */
>>>> -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>>>> -		       const void *data, size_t len)
>>>> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>>>> +			   const void *data, size_t len)
>>>>   {
>>>>   	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
>>>>   	struct mipi_dsi_msg msg = {
>>>> diff --git a/drivers/gpu/drm/panel/panel-s6e8aa0.c b/drivers/gpu/drm/panel/panel-s6e8aa0.c
>>>> index 06e57a26db7a..beb43492b649 100644
>>>> --- a/drivers/gpu/drm/panel/panel-s6e8aa0.c
>>>> +++ b/drivers/gpu/drm/panel/panel-s6e8aa0.c
>>>> @@ -133,14 +133,14 @@ static int s6e8aa0_clear_error(struct s6e8aa0 *ctx)
>>>>   static void s6e8aa0_dcs_write(struct s6e8aa0 *ctx, const void *data, size_t len)
>>>>   {
>>>>   	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>>>> -	int ret;
>>>> +	ssize_t ret;
>>>>
>>>>   	if (ctx->error < 0)
>>>>   		return;
>>>>
>>>>   	ret = mipi_dsi_dcs_write(dsi, dsi->channel, data, len);
>>>>   	if (ret < 0) {
>>>> -		dev_err(ctx->dev, "error %d writing dcs seq: %*ph\n", ret, len,
>>>> +		dev_err(ctx->dev, "error %zd writing dcs seq: %*ph\n", ret, len,
>>>>   			data);
>>>>   		ctx->error = ret;
>> One more thing!
>> This 'ctx->error' type is 'int'. So it should be changed from int to 
>> ssize_t in struct s6e8aa0.
> I do not think so. ctx->error contains always error code, and this is
> guarded here
> by 'if (ret < 0)'  clause.
> 
> Regards
> Andrzej
>>
>> Thank you.
>> Best regards YJ
>>
>>>>   	}
>>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>>> index efa1b552adc5..4b0112781910 100644
>>>> --- a/include/drm/drm_mipi_dsi.h
>>>> +++ b/include/drm/drm_mipi_dsi.h
>>>> @@ -127,8 +127,8 @@ struct mipi_dsi_device {
>>>>
>>>>   int mipi_dsi_attach(struct mipi_dsi_device *dsi);
>>>>   int mipi_dsi_detach(struct mipi_dsi_device *dsi);
>>>> -int mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>>>> -		       const void *data, size_t len);
>>>> +ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, unsigned int channel,
>>>> +			   const void *data, size_t len);
>>>>   ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, unsigned int channel,
>>>>   			  u8 cmd, void *data, size_t len);
>>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>