[PATCH v2 0/2] drm/bridge/synopsys: dsi: Add fix & warning in dsi_host_transfer()

[PATCH v2 0/2] drm/bridge/synopsys: dsi: Add fix & warning in dsi_host_transfer()

[Philippe Cornu]

Add a fix & a warning in the dsi_host_transfer().

Version 2:
- Simplify the 2 patches following comments from Brian Norris.
- Swap the 2 patches as the return value is only on tx and
  in case of rx requests the warning is there.

Version 1:
- Initial commit

Philippe Cornu (2):
  drm/bridge/synopsys: dsi: Add a warning msg on dsi read requests
  drm/bridge/synopsys: dsi: Fix dsi_host_transfer() return value

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

-- 
2.15.1

[PATCH v2 1/2] drm/bridge/synopsys: dsi: Add a warning msg on dsi read requests

[Philippe Cornu]

The dcs/generic dsi read feature is not yet implemented so it
is important to warn the host_transfer() caller in case of
read operation requests.

Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index daec7881be6d..72ecaeb40822 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -405,6 +405,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
 	struct mipi_dsi_packet packet;
 	int ret;
 
+	if (msg->rx_buf || msg->rx_len) {
+		/* TODO dw drv improvements: implement read feature */
+		dev_warn(dsi->dev, "read operations not yet implemented\n");
+		return -EINVAL;
+	}
+
 	ret = mipi_dsi_create_packet(&packet, msg);
 	if (ret) {
 		dev_err(dsi->dev, "failed to create packet: %d\n", ret);
-- 
2.15.1

Re: [PATCH v2 1/2] drm/bridge/synopsys: dsi: Add a warning msg on dsi read requests

[Brian Norris]

On Thu, Jan 25, 2018 at 11:37:59AM +0100, Philippe Cornu wrote:
> The dcs/generic dsi read feature is not yet implemented so it
> is important to warn the host_transfer() caller in case of
> read operation requests.
> 
> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>

Awesome, thanks.

Reviewed-by: Brian Norris <briannorris@chromium.org>

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index daec7881be6d..72ecaeb40822 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -405,6 +405,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>  	struct mipi_dsi_packet packet;
>  	int ret;
>  
> +	if (msg->rx_buf || msg->rx_len) {
> +		/* TODO dw drv improvements: implement read feature */
> +		dev_warn(dsi->dev, "read operations not yet implemented\n");
> +		return -EINVAL;
> +	}
> +
>  	ret = mipi_dsi_create_packet(&packet, msg);
>  	if (ret) {
>  		dev_err(dsi->dev, "failed to create packet: %d\n", ret);
> -- 
> 2.15.1
> 

Re: [PATCH v2 1/2] drm/bridge/synopsys: dsi: Add a warning msg on dsi read requests

[Archit Taneja]

On 01/26/2018 06:14 AM, Brian Norris wrote:
> On Thu, Jan 25, 2018 at 11:37:59AM +0100, Philippe Cornu wrote:
>> The dcs/generic dsi read feature is not yet implemented so it
>> is important to warn the host_transfer() caller in case of
>> read operation requests.
>>
>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
> 
> Awesome, thanks.
> 
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> 

Queued to drm-misc-next.

Thanks,
Archit

>> ---
>>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index daec7881be6d..72ecaeb40822 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> @@ -405,6 +405,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>>   	struct mipi_dsi_packet packet;
>>   	int ret;
>>   
>> +	if (msg->rx_buf || msg->rx_len) {
>> +		/* TODO dw drv improvements: implement read feature */
>> +		dev_warn(dsi->dev, "read operations not yet implemented\n");
>> +		return -EINVAL;
>> +	}
>> +
>>   	ret = mipi_dsi_create_packet(&packet, msg);
>>   	if (ret) {
>>   		dev_err(dsi->dev, "failed to create packet: %d\n", ret);
>> -- 
>> 2.15.1
>>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/2] drm/bridge/synopsys: dsi: Add a warning msg on dsi read requests

[Philippe CORNU]

Hi Archit,

And many thanks,
Philippe :-)

On 01/30/2018 03:09 PM, Archit Taneja wrote:
> 
> 
> On 01/26/2018 06:14 AM, Brian Norris wrote:
>> On Thu, Jan 25, 2018 at 11:37:59AM +0100, Philippe Cornu wrote:
>>> The dcs/generic dsi read feature is not yet implemented so it
>>> is important to warn the host_transfer() caller in case of
>>> read operation requests.
>>>
>>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
>>
>> Awesome, thanks.
>>
>> Reviewed-by: Brian Norris <briannorris@chromium.org>
>>
> 
> Queued to drm-misc-next.
> 
> Thanks,
> Archit
> 
>>> ---
>>>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
>>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> index daec7881be6d..72ecaeb40822 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> @@ -405,6 +405,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct 
>>> mipi_dsi_host *host,
>>>       struct mipi_dsi_packet packet;
>>>       int ret;
>>> +    if (msg->rx_buf || msg->rx_len) {
>>> +        /* TODO dw drv improvements: implement read feature */
>>> +        dev_warn(dsi->dev, "read operations not yet implemented\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>>       ret = mipi_dsi_create_packet(&packet, msg);
>>>       if (ret) {
>>>           dev_err(dsi->dev, "failed to create packet: %d\n", ret);
>>> -- 
>>> 2.15.1
>>>
> 

[PATCH v2 2/2] drm/bridge/synopsys: dsi: Fix dsi_host_transfer() return value

[Philippe Cornu]

The dw_mipi_dsi_host_transfer() must return the number of
bytes transmitted/received on success instead of 0.
Note: As the read feature is not implemented, only the
transmitted number of bytes is returned for the moment.

Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 72ecaeb40822..090bf62d1ea8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -419,7 +419,16 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
 
 	dw_mipi_message_config(dsi, msg);
 
-	return dw_mipi_dsi_write(dsi, &packet);
+	ret = dw_mipi_dsi_write(dsi, &packet);
+	if (ret)
+		return ret;
+
+	/*
+	 * TODO Only transmitted size is returned as actual driver does
+	 * not support dcs/generic reads. Please update return value when
+	 * delivering the read feature.
+	 */
+	return packet.size;
 }
 
 static const struct mipi_dsi_host_ops dw_mipi_dsi_host_ops = {
-- 
2.15.1

Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: Fix dsi_host_transfer() return value

[Brian Norris]

On Thu, Jan 25, 2018 at 11:38:00AM +0100, Philippe Cornu wrote:
> The dw_mipi_dsi_host_transfer() must return the number of
> bytes transmitted/received on success instead of 0.
> Note: As the read feature is not implemented, only the
> transmitted number of bytes is returned for the moment.
> 
> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>

Assuming we're going with the current documented semantics (where we
return # of TX bytes for writes), then:

Reviewed-by: Brian Norris <briannorris@chromium.org>

I believe Archit was suggesting maybe changing that sometime, but that's
no excuse for not matching documentation now.

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 72ecaeb40822..090bf62d1ea8 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -419,7 +419,16 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>  
>  	dw_mipi_message_config(dsi, msg);
>  
> -	return dw_mipi_dsi_write(dsi, &packet);
> +	ret = dw_mipi_dsi_write(dsi, &packet);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * TODO Only transmitted size is returned as actual driver does
> +	 * not support dcs/generic reads. Please update return value when
> +	 * delivering the read feature.
> +	 */
> +	return packet.size;

You're really holding my hand here, I see :) Thanks I guess.

>  }
>  
>  static const struct mipi_dsi_host_ops dw_mipi_dsi_host_ops = {
> -- 
> 2.15.1
> 

Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: Fix dsi_host_transfer() return value

[Archit Taneja]

On 01/26/2018 06:16 AM, Brian Norris wrote:
> On Thu, Jan 25, 2018 at 11:38:00AM +0100, Philippe Cornu wrote:
>> The dw_mipi_dsi_host_transfer() must return the number of
>> bytes transmitted/received on success instead of 0.
>> Note: As the read feature is not implemented, only the
>> transmitted number of bytes is returned for the moment.
>>
>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
> 
> Assuming we're going with the current documented semantics (where we
> return # of TX bytes for writes), then:
> 
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> 
> I believe Archit was suggesting maybe changing that sometime, but that's
> no excuse for not matching documentation now.

Queued to drm-misc-next.

Thanks,
Archit

> 
>> ---
>>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index 72ecaeb40822..090bf62d1ea8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> @@ -419,7 +419,16 @@ static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>>   
>>   	dw_mipi_message_config(dsi, msg);
>>   
>> -	return dw_mipi_dsi_write(dsi, &packet);
>> +	ret = dw_mipi_dsi_write(dsi, &packet);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * TODO Only transmitted size is returned as actual driver does
>> +	 * not support dcs/generic reads. Please update return value when
>> +	 * delivering the read feature.
>> +	 */
>> +	return packet.size;
> 
> You're really holding my hand here, I see :) Thanks I guess.
> 
>>   }
>>   
>>   static const struct mipi_dsi_host_ops dw_mipi_dsi_host_ops = {
>> -- 
>> 2.15.1
>>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project