| Message ID | 20190326103146.24795-2-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
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; > }
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; > }
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 --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; }
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(-)