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

2019-04-26 Thread Tomi Valkeinen 
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 
>> ---
>>  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, );
>>  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

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-04-20 Thread Laurent Pinchart 
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 
> ---
>  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, );
>   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;
>  }

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

2019-04-15 Thread Andrzej Hajda 
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 
Reviewed-by: Andrzej Hajda 

 --
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, );
>   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;
>  }


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel