diff mbox series

[PATCHv2,01/22] drm/bridge: tc358767: fix tc_aux_get_status error handling

Message ID 20190326103146.24795-2-tomi.valkeinen@ti.com
State Superseded
Headers show
Series drm/bridge: tc358767: DP support | expand

Commit Message

Tomi Valkeinen March 26, 2019, 10:31 a.m. UTC 
tc_aux_get_status() does not report AUX_TIMEOUT correctly, as it only
checks the AUX_TIMEOUT if aux is still busy. Fix this by always checking
for AUX_TIMEOUT.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Andrzej Hajda April 15, 2019, 7:20 a.m. UTC | #1 
On 26.03.2019 11:31, Tomi Valkeinen wrote:
> tc_aux_get_status() does not report AUX_TIMEOUT correctly, as it only
> checks the AUX_TIMEOUT if aux is still busy. Fix this by always checking
> for AUX_TIMEOUT.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 888980d4bc74..7031c4f52c57 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -286,14 +286,17 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
>  	ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &value);
>  	if (ret < 0)
>  		return ret;
> +
>  	if (value & AUX_BUSY) {
> -		if (value & AUX_TIMEOUT) {
> -			dev_err(tc->dev, "i2c access timeout!\n");
> -			return -ETIMEDOUT;
> -		}
> +		dev_err(tc->dev, "aux busy!\n");
>  		return -EBUSY;
>  	}
>  
> +	if (value & AUX_TIMEOUT) {
> +		dev_err(tc->dev, "aux access timeout!\n");
> +		return -ETIMEDOUT;
> +	}
> +
>  	*reply = (value & AUX_STATUS_MASK) >> AUX_STATUS_SHIFT;
>  	return 0;
>  }
Laurent Pinchart April 20, 2019, 8:14 p.m. UTC | #2 
Hi Tomi,

Thank you for the patch.

On Tue, Mar 26, 2019 at 12:31:25PM +0200, Tomi Valkeinen wrote:
> tc_aux_get_status() does not report AUX_TIMEOUT correctly, as it only
> checks the AUX_TIMEOUT if aux is still busy. Fix this by always checking
> for AUX_TIMEOUT.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 888980d4bc74..7031c4f52c57 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -286,14 +286,17 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
>  	ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &value);
>  	if (ret < 0)
>  		return ret;
> +
>  	if (value & AUX_BUSY) {
> -		if (value & AUX_TIMEOUT) {
> -			dev_err(tc->dev, "i2c access timeout!\n");
> -			return -ETIMEDOUT;
> -		}
> +		dev_err(tc->dev, "aux busy!\n");
>  		return -EBUSY;
>  	}
>  
> +	if (value & AUX_TIMEOUT) {
> +		dev_err(tc->dev, "aux access timeout!\n");
> +		return -ETIMEDOUT;
> +	}
> +

This changes the priority between errors. Should AUX_TIMEOUT be checked
first ?

>  	*reply = (value & AUX_STATUS_MASK) >> AUX_STATUS_SHIFT;
>  	return 0;
>  }
Tomi Valkeinen April 26, 2019, 2:08 p.m. UTC | #3 
On 20/04/2019 23:14, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Tue, Mar 26, 2019 at 12:31:25PM +0200, Tomi Valkeinen wrote:
>> tc_aux_get_status() does not report AUX_TIMEOUT correctly, as it only
>> checks the AUX_TIMEOUT if aux is still busy. Fix this by always checking
>> for AUX_TIMEOUT.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/bridge/tc358767.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
>> index 888980d4bc74..7031c4f52c57 100644
>> --- a/drivers/gpu/drm/bridge/tc358767.c
>> +++ b/drivers/gpu/drm/bridge/tc358767.c
>> @@ -286,14 +286,17 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
>>  	ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &value);
>>  	if (ret < 0)
>>  		return ret;
>> +
>>  	if (value & AUX_BUSY) {
>> -		if (value & AUX_TIMEOUT) {
>> -			dev_err(tc->dev, "i2c access timeout!\n");
>> -			return -ETIMEDOUT;
>> -		}
>> +		dev_err(tc->dev, "aux busy!\n");
>>  		return -EBUSY;
>>  	}
>>  
>> +	if (value & AUX_TIMEOUT) {
>> +		dev_err(tc->dev, "aux access timeout!\n");
>> +		return -ETIMEDOUT;
>> +	}
>> +
> 
> This changes the priority between errors. Should AUX_TIMEOUT be checked
> first ?

BUSY is not an error, it indicates that the HW is not finished. I don't
think it makes sense to look at the error bits before the HW is done.

 Tomi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 888980d4bc74..7031c4f52c57 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -286,14 +286,17 @@  static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
 	ret = regmap_read(tc->regmap, DP0_AUXSTATUS, &value);
 	if (ret < 0)
 		return ret;
+
 	if (value & AUX_BUSY) {
-		if (value & AUX_TIMEOUT) {
-			dev_err(tc->dev, "i2c access timeout!\n");
-			return -ETIMEDOUT;
-		}
+		dev_err(tc->dev, "aux busy!\n");
 		return -EBUSY;
 	}
 
+	if (value & AUX_TIMEOUT) {
+		dev_err(tc->dev, "aux access timeout!\n");
+		return -ETIMEDOUT;
+	}
+
 	*reply = (value & AUX_STATUS_MASK) >> AUX_STATUS_SHIFT;
 	return 0;
 }