Messages in this thread | | | From | Robert Foss <> | Date | Tue, 30 Mar 2021 13:19:31 +0200 | Subject | Re: [PATCH v2 20/20] drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*() |
| |
Hey Lyude,
This patch looks good, but I have one question below. With it addressed, feel free to add my r-b.
Reviewed-by: Robert Foss <robert.foss@linaro.org>
> > -static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw, > +static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr, > + struct drm_dp_sideband_msg_rx *raw, > struct drm_dp_sideband_msg_req_body *msg) > { > memset(msg, 0, sizeof(*msg)); > @@ -1117,12 +1125,12 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw, > > switch (msg->req_type) { > case DP_CONNECTION_STATUS_NOTIFY: > - return drm_dp_sideband_parse_connection_status_notify(raw, msg); > + return drm_dp_sideband_parse_connection_status_notify(mgr, raw, msg); > case DP_RESOURCE_STATUS_NOTIFY: > - return drm_dp_sideband_parse_resource_status_notify(raw, msg); > + return drm_dp_sideband_parse_resource_status_notify(mgr, raw, msg); > default: > - DRM_ERROR("Got unknown request 0x%02x (%s)\n", msg->req_type, > - drm_dp_mst_req_type_str(msg->req_type)); > + drm_err(mgr->dev, "Got unknown request 0x%02x (%s)\n", > + msg->req_type, drm_dp_mst_req_type_str(msg->req_type)); > return false; > } > } >
.. snip ..
> @@ -4118,12 +4121,12 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr) > > INIT_LIST_HEAD(&up_req->next); > > - drm_dp_sideband_parse_req(&mgr->up_req_recv, &up_req->msg); > + drm_dp_sideband_parse_req(mgr, &mgr->up_req_recv, &up_req->msg);
drm_dp_sideband_parse_req() is only called here, and the function arguments could probably stand to have `&mgr->up_req_recv` removed (here and in the func. declaration) since the same data structure is accessible through the `mgr` pointer inside of drm_dp_sideband_parse_req(). I guess this is a matter of taste, so feel free to do what you want with this.
> > if (up_req->msg.req_type != DP_CONNECTION_STATUS_NOTIFY && > up_req->msg.req_type != DP_RESOURCE_STATUS_NOTIFY) { > - DRM_DEBUG_KMS("Received unknown up req type, ignoring: %x\n", > - up_req->msg.req_type); > + drm_dbg_kms(mgr->dev, "Received unknown up req type, ignoring: %x\n", > + up_req->msg.req_type); > kfree(up_req); > goto out; > }
| |