| Message ID | 20200403200757.886443-3-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/dp_mst: drm_dp_check_act_status() fixes | expand |
On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul <lyude@redhat.com> 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@redhat.com> > [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@poorly.run> > Cc: <stable@vger.kernel.org> # 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? Otherwise: Reviewed-by: Sean Paul <sean@poorly.run> > + status, count); > + return -EINVAL; > } > return 0; > -fail: > - return ret; > } > EXPORT_SYMBOL(drm_dp_check_act_status); > > -- > 2.25.1 >
On Mon, 2020-04-06 at 15:23 -0400, Sean Paul wrote: > On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul <lyude@redhat.com> 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@redhat.com> > > [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@poorly.run> > > Cc: <stable@vger.kernel.org> # 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? > > Otherwise: > > Reviewed-by: Sean Paul <sean@poorly.run> Good point - I'll make sure to fix that before I push the series > > > + status, count); > > + return -EINVAL; > > } > > return 0; > > -fail: > > - return ret; > > } > > EXPORT_SYMBOL(drm_dp_check_act_status); > > > > -- > > 2.25.1 > >
On Mon, 2020-04-06 at 15:23 -0400, Sean Paul wrote: > On Fri, Apr 3, 2020 at 4:08 PM Lyude Paul <lyude@redhat.com> 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@redhat.com> > > [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@poorly.run> > > Cc: <stable@vger.kernel.org> # 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@poorly.run> > > > + status, count); > > + return -EINVAL; > > } > > return 0; > > -fail: > > - return ret; > > } > > EXPORT_SYMBOL(drm_dp_check_act_status); > > > > -- > > 2.25.1 > >
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", + status, count); + return -EINVAL; } return 0; -fail: - return ret; } EXPORT_SYMBOL(drm_dp_check_act_status);
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@redhat.com> [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@poorly.run> Cc: <stable@vger.kernel.org> # v3.17+ --- drivers/gpu/drm/drm_dp_mst_topology.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)