On Mon, 2020-04-06 at 15:23 -0400, Sean Paul wrote: > On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul <lyude@xxxxxxxxxx> wrote: > > Just add a bit more line wrapping, get rid of some extraneous > > whitespace, remove an unneeded goto label, and move around some variable > > declarations. No functional changes here. > > > > Signed-off-by: Lyude Paul <lyude@xxxxxxxxxx> > > [this isn't a fix, but it's needed for the fix that comes after this] > > Fixes: ad7f8a1f9ced ("drm/helper: add Displayport multi-stream helper > > (v0.6)") > > Cc: Sean Paul <sean@xxxxxxxxxx> > > Cc: <stable@xxxxxxxxxxxxxxx> # v3.17+ > > --- > > drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++++++++++------------ > > 1 file changed, 10 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > index 2b9ce965f044..7aaf184a2e5f 100644 > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > @@ -4473,33 +4473,31 @@ static int drm_dp_dpcd_write_payload(struct > > drm_dp_mst_topology_mgr *mgr, > > */ > > int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr) > > { > > + int count = 0, ret; > > u8 status; > > - int ret; > > - int count = 0; > > > > do { > > - ret = drm_dp_dpcd_readb(mgr->aux, > > DP_PAYLOAD_TABLE_UPDATE_STATUS, &status); > > - > > + ret = drm_dp_dpcd_readb(mgr->aux, > > + DP_PAYLOAD_TABLE_UPDATE_STATUS, > > + &status); > > if (ret < 0) { > > - DRM_DEBUG_KMS("failed to read payload table status > > %d\n", ret); > > - goto fail; > > + DRM_DEBUG_KMS("failed to read payload table status > > %d\n", > > + ret); > > + return ret; > > } > > > > if (status & DP_PAYLOAD_ACT_HANDLED) > > break; > > count++; > > udelay(100); > > - > > } while (count < 30); > > > > if (!(status & DP_PAYLOAD_ACT_HANDLED)) { > > - DRM_DEBUG_KMS("failed to get ACT bit %d after %d > > retries\n", status, count); > > - ret = -EINVAL; > > - goto fail; > > + DRM_DEBUG_KMS("failed to get ACT bit %d after %d > > retries\n", > > Should we print status in base16 here? jfyi - I realized we don't actually need to do this, because we do this in the next patch whoops. Just figured I'd point that out > > Otherwise: > > Reviewed-by: Sean Paul <sean@xxxxxxxxxx> > > > + status, count); > > + return -EINVAL; > > } > > return 0; > > -fail: > > - return ret; > > } > > EXPORT_SYMBOL(drm_dp_check_act_status); > > > > -- > > 2.25.1 > > -- Cheers, Lyude Paul (she/her) Associate Software Engineer at Red Hat