* [PATCH v3] drm/dp_mst: Fix W=1 warnings
@ 2019-11-28 13:50 Benjamin Gaignard
2019-12-03 16:58 ` Benjamin Gaignard
2019-12-04 16:47 ` Jani Nikula
0 siblings, 2 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2019-11-28 13:50 UTC (permalink / raw)
To: maarten.lankhorst, mripard, sean, airlied, daniel
Cc: dri-devel, linux-kernel, Benjamin Gaignard, Jani Nikula
Fix the warnings that show up with W=1.
They are all about unused but set variables.
If functions returns are not used anymore make them void.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
CC: Jani Nikula <jani.nikula@linux.intel.com>
changes in version 3:
- remove the hunk that may conflict with c485e2c97dae
("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
changes in version 2:
- fix indentations
- when possible change functions prototype to void
drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
1 file changed, 31 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 1437bc46368b..d5cb5688b5dd 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
u8 *replybuf, u8 replybuflen, bool hdr)
{
int ret;
- u8 crc4;
if (hdr) {
u8 hdrlen;
@@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
}
if (msg->curchunk_idx >= msg->curchunk_len) {
- /* do CRC */
- crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
/* copy chunk into bigger msg */
memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
msg->curlen += msg->curchunk_len - 1;
@@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
}
}
-static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
+static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
{
struct drm_dp_sideband_msg_req_body req;
@@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
req.u.dpcd_write.num_bytes = num_bytes;
req.u.dpcd_write.bytes = bytes;
drm_dp_encode_sideband_req(&req, msg);
-
- return 0;
}
-static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
+static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
{
struct drm_dp_sideband_msg_req_body req;
req.req_type = DP_LINK_ADDRESS;
drm_dp_encode_sideband_req(&req, msg);
- return 0;
}
static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
@@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
return 0;
}
-static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
+static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
u8 vcpi, uint16_t pbn,
u8 number_sdp_streams,
u8 *sdp_stream_sink)
@@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
number_sdp_streams);
drm_dp_encode_sideband_req(&req, msg);
msg->path_msg = true;
- return 0;
}
-static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
+static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
int port_num, bool power_up)
{
struct drm_dp_sideband_msg_req_body req;
@@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
req.u.port_num.port_number = port_num;
drm_dp_encode_sideband_req(&req, msg);
msg->path_msg = true;
- return 0;
}
static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
@@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
*/
static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
{
- int ret;
u8 rad[6], lct;
bool send_link = false;
switch (port->pdt) {
case DP_PEER_DEVICE_DP_LEGACY_CONV:
case DP_PEER_DEVICE_SST_SINK:
/* add i2c over sideband */
- ret = drm_dp_mst_register_i2c_bus(&port->aux);
+ drm_dp_mst_register_i2c_bus(&port->aux);
break;
case DP_PEER_DEVICE_MST_BRANCHING:
lct = drm_dp_calculate_rad(port, rad);
@@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
{
- int ret;
-
memcpy(mstb->guid, guid, 16);
if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
if (mstb->port_parent) {
- ret = drm_dp_send_dpcd_write(
- mstb->mgr,
- mstb->port_parent,
- DP_GUID,
- 16,
- mstb->guid);
+ drm_dp_send_dpcd_write(mstb->mgr,
+ mstb->port_parent,
+ DP_GUID,
+ 16,
+ mstb->guid);
} else {
-
- ret = drm_dp_dpcd_write(
- mstb->mgr->aux,
- DP_GUID,
- mstb->guid,
- 16);
+ drm_dp_dpcd_write(mstb->mgr->aux,
+ DP_GUID,
+ mstb->guid,
+ 16);
}
}
}
@@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
return false;
}
-static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
+static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
{
struct drm_dp_sideband_msg_req_body req;
@@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
req.u.dpcd_read.dpcd_address = offset;
req.u.dpcd_read.num_bytes = num_bytes;
drm_dp_encode_sideband_req(&req, msg);
-
- return 0;
}
static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
@@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
{
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_link_address_ack_reply *reply;
- int i, len, ret;
+ int i, ret;
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
if (!txmsg)
return;
txmsg->dst = mstb;
- len = build_link_address(txmsg);
+ build_link_address(txmsg);
mstb->link_address_sent = true;
drm_dp_queue_down_tx(mgr, txmsg);
@@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
{
struct drm_dp_enum_path_resources_ack_reply *path_res;
struct drm_dp_sideband_msg_tx *txmsg;
- int len;
int ret;
txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
@@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
return -ENOMEM;
txmsg->dst = mstb;
- len = build_enum_path_resources(txmsg, port->port_num);
+ build_enum_path_resources(txmsg, port->port_num);
drm_dp_queue_down_tx(mgr, txmsg);
@@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
{
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_mst_branch *mstb;
- int len, ret, port_num;
+ int ret, port_num;
u8 sinks[DRM_DP_MAX_SDP_STREAMS];
int i;
@@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
sinks[i] = i;
txmsg->dst = mstb;
- len = build_allocate_payload(txmsg, port_num,
- id,
- pbn, port->num_sdp_streams, sinks);
+ build_allocate_payload(txmsg, port_num,
+ id,
+ pbn, port->num_sdp_streams, sinks);
drm_dp_queue_down_tx(mgr, txmsg);
@@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, bool power_up)
{
struct drm_dp_sideband_msg_tx *txmsg;
- int len, ret;
+ int ret;
port = drm_dp_mst_topology_get_port_validated(mgr, port);
if (!port)
@@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
}
txmsg->dst = port->parent;
- len = build_power_updown_phy(txmsg, port->port_num, power_up);
+ build_power_updown_phy(txmsg, port->port_num, power_up);
drm_dp_queue_down_tx(mgr, txmsg);
ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
@@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port,
int offset, int size, u8 *bytes)
{
- int len;
int ret = 0;
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_mst_branch *mstb;
@@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
goto fail_put;
}
- len = build_dpcd_read(txmsg, port->port_num, offset, size);
+ build_dpcd_read(txmsg, port->port_num, offset, size);
txmsg->dst = port->parent;
drm_dp_queue_down_tx(mgr, txmsg);
@@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port,
int offset, int size, u8 *bytes)
{
- int len;
int ret;
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_mst_branch *mstb;
@@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
goto fail_put;
}
- len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
+ build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
txmsg->dst = mstb;
drm_dp_queue_down_tx(mgr, txmsg);
@@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
{
int len;
u8 replyblock[32];
- int replylen, origlen, curreply;
+ int replylen, curreply;
int ret;
struct drm_dp_sideband_msg_rx *msg;
int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
@@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
}
replylen = msg->curchunk_len + msg->curchunk_hdrlen;
- origlen = replylen;
replylen -= len;
curreply = len;
while (replylen > 0) {
@@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
mutex_lock(&mgr->lock);
if (mgr->mst_primary) {
u8 buf[DP_PAYLOAD_TABLE_SIZE];
- int ret;
- ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
+ drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
- ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
+ drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
seq_printf(m, "faux/mst: %*ph\n", 2, buf);
- ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
+ drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
/* dump the standard OUI branch header */
- ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
+ drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
for (i = 0x3; i < 0x8 && buf[i]; i++)
seq_printf(m, "%c", buf[i]);
--
2.15.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2019-11-28 13:50 [PATCH v3] drm/dp_mst: Fix W=1 warnings Benjamin Gaignard
@ 2019-12-03 16:58 ` Benjamin Gaignard
2019-12-04 16:47 ` Jani Nikula
1 sibling, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2019-12-03 16:58 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
Daniel Vetter, Linux Kernel Mailing List, ML dri-devel
Le sam. 30 nov. 2019 à 14:24, Benjamin Gaignard
<benjamin.gaignard@st.com> a écrit :
>
> Fix the warnings that show up with W=1.
> They are all about unused but set variables.
> If functions returns are not used anymore make them void.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> CC: Jani Nikula <jani.nikula@linux.intel.com>
>
> changes in version 3:
> - remove the hunk that may conflict with c485e2c97dae
> ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
I have test today this patch on drm-misc-next where the other patch is
present, it apply fine and I don't have warning anymore.
Benjamin
>
> changes in version 2:
> - fix indentations
> - when possible change functions prototype to void
>
> drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
> 1 file changed, 31 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1437bc46368b..d5cb5688b5dd 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> u8 *replybuf, u8 replybuflen, bool hdr)
> {
> int ret;
> - u8 crc4;
>
> if (hdr) {
> u8 hdrlen;
> @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> }
>
> if (msg->curchunk_idx >= msg->curchunk_len) {
> - /* do CRC */
> - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
> /* copy chunk into bigger msg */
> memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
> msg->curlen += msg->curchunk_len - 1;
> @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> }
> }
>
> -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> req.u.dpcd_write.num_bytes = num_bytes;
> req.u.dpcd_write.bytes = bytes;
> drm_dp_encode_sideband_req(&req, msg);
> -
> - return 0;
> }
>
> -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> req.req_type = DP_LINK_ADDRESS;
> drm_dp_encode_sideband_req(&req, msg);
> - return 0;
> }
>
> static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
> @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
> return 0;
> }
>
> -static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> +static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> u8 vcpi, uint16_t pbn,
> u8 number_sdp_streams,
> u8 *sdp_stream_sink)
> @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
> number_sdp_streams);
> drm_dp_encode_sideband_req(&req, msg);
> msg->path_msg = true;
> - return 0;
> }
>
> -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> int port_num, bool power_up)
> {
> struct drm_dp_sideband_msg_req_body req;
> @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> req.u.port_num.port_number = port_num;
> drm_dp_encode_sideband_req(&req, msg);
> msg->path_msg = true;
> - return 0;
> }
>
> static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
> @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
> */
> static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> {
> - int ret;
> u8 rad[6], lct;
> bool send_link = false;
> switch (port->pdt) {
> case DP_PEER_DEVICE_DP_LEGACY_CONV:
> case DP_PEER_DEVICE_SST_SINK:
> /* add i2c over sideband */
> - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> + drm_dp_mst_register_i2c_bus(&port->aux);
> break;
> case DP_PEER_DEVICE_MST_BRANCHING:
> lct = drm_dp_calculate_rad(port, rad);
> @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
>
> static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> {
> - int ret;
> -
> memcpy(mstb->guid, guid, 16);
>
> if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> if (mstb->port_parent) {
> - ret = drm_dp_send_dpcd_write(
> - mstb->mgr,
> - mstb->port_parent,
> - DP_GUID,
> - 16,
> - mstb->guid);
> + drm_dp_send_dpcd_write(mstb->mgr,
> + mstb->port_parent,
> + DP_GUID,
> + 16,
> + mstb->guid);
> } else {
> -
> - ret = drm_dp_dpcd_write(
> - mstb->mgr->aux,
> - DP_GUID,
> - mstb->guid,
> - 16);
> + drm_dp_dpcd_write(mstb->mgr->aux,
> + DP_GUID,
> + mstb->guid,
> + 16);
> }
> }
> }
> @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> return false;
> }
>
> -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> req.u.dpcd_read.dpcd_address = offset;
> req.u.dpcd_read.num_bytes = num_bytes;
> drm_dp_encode_sideband_req(&req, msg);
> -
> - return 0;
> }
>
> static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_link_address_ack_reply *reply;
> - int i, len, ret;
> + int i, ret;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> if (!txmsg)
> return;
>
> txmsg->dst = mstb;
> - len = build_link_address(txmsg);
> + build_link_address(txmsg);
>
> mstb->link_address_sent = true;
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_enum_path_resources_ack_reply *path_res;
> struct drm_dp_sideband_msg_tx *txmsg;
> - int len;
> int ret;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> return -ENOMEM;
>
> txmsg->dst = mstb;
> - len = build_enum_path_resources(txmsg, port->port_num);
> + build_enum_path_resources(txmsg, port->port_num);
>
> drm_dp_queue_down_tx(mgr, txmsg);
>
> @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> - int len, ret, port_num;
> + int ret, port_num;
> u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> int i;
>
> @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> sinks[i] = i;
>
> txmsg->dst = mstb;
> - len = build_allocate_payload(txmsg, port_num,
> - id,
> - pbn, port->num_sdp_streams, sinks);
> + build_allocate_payload(txmsg, port_num,
> + id,
> + pbn, port->num_sdp_streams, sinks);
>
> drm_dp_queue_down_tx(mgr, txmsg);
>
> @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port, bool power_up)
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> - int len, ret;
> + int ret;
>
> port = drm_dp_mst_topology_get_port_validated(mgr, port);
> if (!port)
> @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> }
>
> txmsg->dst = port->parent;
> - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> + build_power_updown_phy(txmsg, port->port_num, power_up);
> drm_dp_queue_down_tx(mgr, txmsg);
>
> ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> int offset, int size, u8 *bytes)
> {
> - int len;
> int ret = 0;
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> goto fail_put;
> }
>
> - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> + build_dpcd_read(txmsg, port->port_num, offset, size);
> txmsg->dst = port->parent;
>
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> int offset, int size, u8 *bytes)
> {
> - int len;
> int ret;
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> goto fail_put;
> }
>
> - len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> txmsg->dst = mstb;
>
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> {
> int len;
> u8 replyblock[32];
> - int replylen, origlen, curreply;
> + int replylen, curreply;
> int ret;
> struct drm_dp_sideband_msg_rx *msg;
> int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
> @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> }
> replylen = msg->curchunk_len + msg->curchunk_hdrlen;
>
> - origlen = replylen;
> replylen -= len;
> curreply = len;
> while (replylen > 0) {
> @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> mutex_lock(&mgr->lock);
> if (mgr->mst_primary) {
> u8 buf[DP_PAYLOAD_TABLE_SIZE];
> - int ret;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
> - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
>
> /* dump the standard OUI branch header */
> - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> for (i = 0x3; i < 0x8 && buf[i]; i++)
> seq_printf(m, "%c", buf[i]);
> --
> 2.15.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2019-11-28 13:50 [PATCH v3] drm/dp_mst: Fix W=1 warnings Benjamin Gaignard
2019-12-03 16:58 ` Benjamin Gaignard
@ 2019-12-04 16:47 ` Jani Nikula
2019-12-16 8:28 ` Benjamin Gaignard
1 sibling, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2019-12-04 16:47 UTC (permalink / raw)
To: Benjamin Gaignard, maarten.lankhorst, mripard, sean, airlied, daniel
Cc: dri-devel, linux-kernel, Benjamin Gaignard
On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com> wrote:
> Fix the warnings that show up with W=1.
> They are all about unused but set variables.
> If functions returns are not used anymore make them void.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> CC: Jani Nikula <jani.nikula@linux.intel.com>
>
> changes in version 3:
> - remove the hunk that may conflict with c485e2c97dae
> ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
>
> changes in version 2:
> - fix indentations
> - when possible change functions prototype to void
>
> drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
> 1 file changed, 31 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1437bc46368b..d5cb5688b5dd 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> u8 *replybuf, u8 replybuflen, bool hdr)
> {
> int ret;
> - u8 crc4;
>
> if (hdr) {
> u8 hdrlen;
> @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> }
>
> if (msg->curchunk_idx >= msg->curchunk_len) {
> - /* do CRC */
> - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
Again, someone needs to check if crc4 should be *used* instead of thrown
away. Blindly throwing stuff out is not the way to go.
BR,
Jani.
> /* copy chunk into bigger msg */
> memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
> msg->curlen += msg->curchunk_len - 1;
> @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> }
> }
>
> -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> req.u.dpcd_write.num_bytes = num_bytes;
> req.u.dpcd_write.bytes = bytes;
> drm_dp_encode_sideband_req(&req, msg);
> -
> - return 0;
> }
>
> -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> req.req_type = DP_LINK_ADDRESS;
> drm_dp_encode_sideband_req(&req, msg);
> - return 0;
> }
>
> static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
> @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
> return 0;
> }
>
> -static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> +static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> u8 vcpi, uint16_t pbn,
> u8 number_sdp_streams,
> u8 *sdp_stream_sink)
> @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
> number_sdp_streams);
> drm_dp_encode_sideband_req(&req, msg);
> msg->path_msg = true;
> - return 0;
> }
>
> -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> int port_num, bool power_up)
> {
> struct drm_dp_sideband_msg_req_body req;
> @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> req.u.port_num.port_number = port_num;
> drm_dp_encode_sideband_req(&req, msg);
> msg->path_msg = true;
> - return 0;
> }
>
> static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
> @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
> */
> static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> {
> - int ret;
> u8 rad[6], lct;
> bool send_link = false;
> switch (port->pdt) {
> case DP_PEER_DEVICE_DP_LEGACY_CONV:
> case DP_PEER_DEVICE_SST_SINK:
> /* add i2c over sideband */
> - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> + drm_dp_mst_register_i2c_bus(&port->aux);
> break;
> case DP_PEER_DEVICE_MST_BRANCHING:
> lct = drm_dp_calculate_rad(port, rad);
> @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
>
> static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> {
> - int ret;
> -
> memcpy(mstb->guid, guid, 16);
>
> if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> if (mstb->port_parent) {
> - ret = drm_dp_send_dpcd_write(
> - mstb->mgr,
> - mstb->port_parent,
> - DP_GUID,
> - 16,
> - mstb->guid);
> + drm_dp_send_dpcd_write(mstb->mgr,
> + mstb->port_parent,
> + DP_GUID,
> + 16,
> + mstb->guid);
> } else {
> -
> - ret = drm_dp_dpcd_write(
> - mstb->mgr->aux,
> - DP_GUID,
> - mstb->guid,
> - 16);
> + drm_dp_dpcd_write(mstb->mgr->aux,
> + DP_GUID,
> + mstb->guid,
> + 16);
> }
> }
> }
> @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> return false;
> }
>
> -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> {
> struct drm_dp_sideband_msg_req_body req;
>
> @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> req.u.dpcd_read.dpcd_address = offset;
> req.u.dpcd_read.num_bytes = num_bytes;
> drm_dp_encode_sideband_req(&req, msg);
> -
> - return 0;
> }
>
> static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_link_address_ack_reply *reply;
> - int i, len, ret;
> + int i, ret;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> if (!txmsg)
> return;
>
> txmsg->dst = mstb;
> - len = build_link_address(txmsg);
> + build_link_address(txmsg);
>
> mstb->link_address_sent = true;
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_enum_path_resources_ack_reply *path_res;
> struct drm_dp_sideband_msg_tx *txmsg;
> - int len;
> int ret;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> return -ENOMEM;
>
> txmsg->dst = mstb;
> - len = build_enum_path_resources(txmsg, port->port_num);
> + build_enum_path_resources(txmsg, port->port_num);
>
> drm_dp_queue_down_tx(mgr, txmsg);
>
> @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> - int len, ret, port_num;
> + int ret, port_num;
> u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> int i;
>
> @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> sinks[i] = i;
>
> txmsg->dst = mstb;
> - len = build_allocate_payload(txmsg, port_num,
> - id,
> - pbn, port->num_sdp_streams, sinks);
> + build_allocate_payload(txmsg, port_num,
> + id,
> + pbn, port->num_sdp_streams, sinks);
>
> drm_dp_queue_down_tx(mgr, txmsg);
>
> @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port, bool power_up)
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> - int len, ret;
> + int ret;
>
> port = drm_dp_mst_topology_get_port_validated(mgr, port);
> if (!port)
> @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> }
>
> txmsg->dst = port->parent;
> - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> + build_power_updown_phy(txmsg, port->port_num, power_up);
> drm_dp_queue_down_tx(mgr, txmsg);
>
> ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> int offset, int size, u8 *bytes)
> {
> - int len;
> int ret = 0;
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> goto fail_put;
> }
>
> - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> + build_dpcd_read(txmsg, port->port_num, offset, size);
> txmsg->dst = port->parent;
>
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> struct drm_dp_mst_port *port,
> int offset, int size, u8 *bytes)
> {
> - int len;
> int ret;
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> goto fail_put;
> }
>
> - len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> txmsg->dst = mstb;
>
> drm_dp_queue_down_tx(mgr, txmsg);
> @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> {
> int len;
> u8 replyblock[32];
> - int replylen, origlen, curreply;
> + int replylen, curreply;
> int ret;
> struct drm_dp_sideband_msg_rx *msg;
> int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
> @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> }
> replylen = msg->curchunk_len + msg->curchunk_hdrlen;
>
> - origlen = replylen;
> replylen -= len;
> curreply = len;
> while (replylen > 0) {
> @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> mutex_lock(&mgr->lock);
> if (mgr->mst_primary) {
> u8 buf[DP_PAYLOAD_TABLE_SIZE];
> - int ret;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
> - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
>
> /* dump the standard OUI branch header */
> - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> for (i = 0x3; i < 0x8 && buf[i]; i++)
> seq_printf(m, "%c", buf[i]);
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2019-12-04 16:47 ` Jani Nikula
@ 2019-12-16 8:28 ` Benjamin Gaignard
2019-12-20 14:03 ` Benjamin Gaignard
0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Gaignard @ 2019-12-16 8:28 UTC (permalink / raw)
To: Jani Nikula
Cc: Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Linux Kernel Mailing List,
ML dri-devel
Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a écrit :
>
> On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com> wrote:
> > Fix the warnings that show up with W=1.
> > They are all about unused but set variables.
> > If functions returns are not used anymore make them void.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> > CC: Jani Nikula <jani.nikula@linux.intel.com>
> >
> > changes in version 3:
> > - remove the hunk that may conflict with c485e2c97dae
> > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> >
> > changes in version 2:
> > - fix indentations
> > - when possible change functions prototype to void
> >
> > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
> > 1 file changed, 31 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 1437bc46368b..d5cb5688b5dd 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > u8 *replybuf, u8 replybuflen, bool hdr)
> > {
> > int ret;
> > - u8 crc4;
> >
> > if (hdr) {
> > u8 hdrlen;
> > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > }
> >
> > if (msg->curchunk_idx >= msg->curchunk_len) {
> > - /* do CRC */
> > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
>
> Again, someone needs to check if crc4 should be *used* instead of thrown
> away. Blindly throwing stuff out is not the way to go.
Hi Dave,
Your are the original writer of this code, could you tell us if we can drop crc4
ao if there is something else that I have misunderstood ?
Thanks,
Benjamin
>
> BR,
> Jani.
>
> > /* copy chunk into bigger msg */
> > memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
> > msg->curlen += msg->curchunk_len - 1;
> > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> > }
> > }
> >
> > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > {
> > struct drm_dp_sideband_msg_req_body req;
> >
> > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > req.u.dpcd_write.num_bytes = num_bytes;
> > req.u.dpcd_write.bytes = bytes;
> > drm_dp_encode_sideband_req(&req, msg);
> > -
> > - return 0;
> > }
> >
> > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > {
> > struct drm_dp_sideband_msg_req_body req;
> >
> > req.req_type = DP_LINK_ADDRESS;
> > drm_dp_encode_sideband_req(&req, msg);
> > - return 0;
> > }
> >
> > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
> > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
> > return 0;
> > }
> >
> > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > u8 vcpi, uint16_t pbn,
> > u8 number_sdp_streams,
> > u8 *sdp_stream_sink)
> > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
> > number_sdp_streams);
> > drm_dp_encode_sideband_req(&req, msg);
> > msg->path_msg = true;
> > - return 0;
> > }
> >
> > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > int port_num, bool power_up)
> > {
> > struct drm_dp_sideband_msg_req_body req;
> > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > req.u.port_num.port_number = port_num;
> > drm_dp_encode_sideband_req(&req, msg);
> > msg->path_msg = true;
> > - return 0;
> > }
> >
> > static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
> > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
> > */
> > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > {
> > - int ret;
> > u8 rad[6], lct;
> > bool send_link = false;
> > switch (port->pdt) {
> > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > case DP_PEER_DEVICE_SST_SINK:
> > /* add i2c over sideband */
> > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > + drm_dp_mst_register_i2c_bus(&port->aux);
> > break;
> > case DP_PEER_DEVICE_MST_BRANCHING:
> > lct = drm_dp_calculate_rad(port, rad);
> > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> >
> > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> > {
> > - int ret;
> > -
> > memcpy(mstb->guid, guid, 16);
> >
> > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > if (mstb->port_parent) {
> > - ret = drm_dp_send_dpcd_write(
> > - mstb->mgr,
> > - mstb->port_parent,
> > - DP_GUID,
> > - 16,
> > - mstb->guid);
> > + drm_dp_send_dpcd_write(mstb->mgr,
> > + mstb->port_parent,
> > + DP_GUID,
> > + 16,
> > + mstb->guid);
> > } else {
> > -
> > - ret = drm_dp_dpcd_write(
> > - mstb->mgr->aux,
> > - DP_GUID,
> > - mstb->guid,
> > - 16);
> > + drm_dp_dpcd_write(mstb->mgr->aux,
> > + DP_GUID,
> > + mstb->guid,
> > + 16);
> > }
> > }
> > }
> > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> > return false;
> > }
> >
> > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > {
> > struct drm_dp_sideband_msg_req_body req;
> >
> > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > req.u.dpcd_read.dpcd_address = offset;
> > req.u.dpcd_read.num_bytes = num_bytes;
> > drm_dp_encode_sideband_req(&req, msg);
> > -
> > - return 0;
> > }
> >
> > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> > {
> > struct drm_dp_sideband_msg_tx *txmsg;
> > struct drm_dp_link_address_ack_reply *reply;
> > - int i, len, ret;
> > + int i, ret;
> >
> > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > if (!txmsg)
> > return;
> >
> > txmsg->dst = mstb;
> > - len = build_link_address(txmsg);
> > + build_link_address(txmsg);
> >
> > mstb->link_address_sent = true;
> > drm_dp_queue_down_tx(mgr, txmsg);
> > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > {
> > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > struct drm_dp_sideband_msg_tx *txmsg;
> > - int len;
> > int ret;
> >
> > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > return -ENOMEM;
> >
> > txmsg->dst = mstb;
> > - len = build_enum_path_resources(txmsg, port->port_num);
> > + build_enum_path_resources(txmsg, port->port_num);
> >
> > drm_dp_queue_down_tx(mgr, txmsg);
> >
> > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > {
> > struct drm_dp_sideband_msg_tx *txmsg;
> > struct drm_dp_mst_branch *mstb;
> > - int len, ret, port_num;
> > + int ret, port_num;
> > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > int i;
> >
> > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > sinks[i] = i;
> >
> > txmsg->dst = mstb;
> > - len = build_allocate_payload(txmsg, port_num,
> > - id,
> > - pbn, port->num_sdp_streams, sinks);
> > + build_allocate_payload(txmsg, port_num,
> > + id,
> > + pbn, port->num_sdp_streams, sinks);
> >
> > drm_dp_queue_down_tx(mgr, txmsg);
> >
> > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > struct drm_dp_mst_port *port, bool power_up)
> > {
> > struct drm_dp_sideband_msg_tx *txmsg;
> > - int len, ret;
> > + int ret;
> >
> > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > if (!port)
> > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > }
> >
> > txmsg->dst = port->parent;
> > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > drm_dp_queue_down_tx(mgr, txmsg);
> >
> > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > struct drm_dp_mst_port *port,
> > int offset, int size, u8 *bytes)
> > {
> > - int len;
> > int ret = 0;
> > struct drm_dp_sideband_msg_tx *txmsg;
> > struct drm_dp_mst_branch *mstb;
> > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > goto fail_put;
> > }
> >
> > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > txmsg->dst = port->parent;
> >
> > drm_dp_queue_down_tx(mgr, txmsg);
> > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > struct drm_dp_mst_port *port,
> > int offset, int size, u8 *bytes)
> > {
> > - int len;
> > int ret;
> > struct drm_dp_sideband_msg_tx *txmsg;
> > struct drm_dp_mst_branch *mstb;
> > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > goto fail_put;
> > }
> >
> > - len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > txmsg->dst = mstb;
> >
> > drm_dp_queue_down_tx(mgr, txmsg);
> > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > {
> > int len;
> > u8 replyblock[32];
> > - int replylen, origlen, curreply;
> > + int replylen, curreply;
> > int ret;
> > struct drm_dp_sideband_msg_rx *msg;
> > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > }
> > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> >
> > - origlen = replylen;
> > replylen -= len;
> > curreply = len;
> > while (replylen > 0) {
> > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> > mutex_lock(&mgr->lock);
> > if (mgr->mst_primary) {
> > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > - int ret;
> >
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> >
> > /* dump the standard OUI branch header */
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > seq_printf(m, "%c", buf[i]);
>
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2019-12-16 8:28 ` Benjamin Gaignard
@ 2019-12-20 14:03 ` Benjamin Gaignard
2020-01-07 13:11 ` Benjamin Gaignard
0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Gaignard @ 2019-12-20 14:03 UTC (permalink / raw)
To: Jani Nikula
Cc: Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Linux Kernel Mailing List,
ML dri-devel, lyude
Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
<benjamin.gaignard@linaro.org> a écrit :
>
> Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a écrit :
> >
> > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com> wrote:
> > > Fix the warnings that show up with W=1.
> > > They are all about unused but set variables.
> > > If functions returns are not used anymore make them void.
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > >
> > > changes in version 3:
> > > - remove the hunk that may conflict with c485e2c97dae
> > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > >
> > > changes in version 2:
> > > - fix indentations
> > > - when possible change functions prototype to void
> > >
> > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
> > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > index 1437bc46368b..d5cb5688b5dd 100644
> > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > > u8 *replybuf, u8 replybuflen, bool hdr)
> > > {
> > > int ret;
> > > - u8 crc4;
> > >
> > > if (hdr) {
> > > u8 hdrlen;
> > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > > }
> > >
> > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > - /* do CRC */
> > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
> >
> > Again, someone needs to check if crc4 should be *used* instead of thrown
> > away. Blindly throwing stuff out is not the way to go.
>
> Hi Dave,
>
+ Lyude who has been hacking in this code recently
> Your are the original writer of this code, could you tell us if we can drop crc4
> ao if there is something else that I have misunderstood ?
>
> Thanks,
> Benjamin
>
> >
> > BR,
> > Jani.
> >
> > > /* copy chunk into bigger msg */
> > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
> > > msg->curlen += msg->curchunk_len - 1;
> > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> > > }
> > > }
> > >
> > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > {
> > > struct drm_dp_sideband_msg_req_body req;
> > >
> > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > req.u.dpcd_write.num_bytes = num_bytes;
> > > req.u.dpcd_write.bytes = bytes;
> > > drm_dp_encode_sideband_req(&req, msg);
> > > -
> > > - return 0;
> > > }
> > >
> > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > {
> > > struct drm_dp_sideband_msg_req_body req;
> > >
> > > req.req_type = DP_LINK_ADDRESS;
> > > drm_dp_encode_sideband_req(&req, msg);
> > > - return 0;
> > > }
> > >
> > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
> > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
> > > return 0;
> > > }
> > >
> > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > > u8 vcpi, uint16_t pbn,
> > > u8 number_sdp_streams,
> > > u8 *sdp_stream_sink)
> > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
> > > number_sdp_streams);
> > > drm_dp_encode_sideband_req(&req, msg);
> > > msg->path_msg = true;
> > > - return 0;
> > > }
> > >
> > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > int port_num, bool power_up)
> > > {
> > > struct drm_dp_sideband_msg_req_body req;
> > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > req.u.port_num.port_number = port_num;
> > > drm_dp_encode_sideband_req(&req, msg);
> > > msg->path_msg = true;
> > > - return 0;
> > > }
> > >
> > > static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
> > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
> > > */
> > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > {
> > > - int ret;
> > > u8 rad[6], lct;
> > > bool send_link = false;
> > > switch (port->pdt) {
> > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > case DP_PEER_DEVICE_SST_SINK:
> > > /* add i2c over sideband */
> > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > break;
> > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > lct = drm_dp_calculate_rad(port, rad);
> > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> > >
> > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> > > {
> > > - int ret;
> > > -
> > > memcpy(mstb->guid, guid, 16);
> > >
> > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > if (mstb->port_parent) {
> > > - ret = drm_dp_send_dpcd_write(
> > > - mstb->mgr,
> > > - mstb->port_parent,
> > > - DP_GUID,
> > > - 16,
> > > - mstb->guid);
> > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > + mstb->port_parent,
> > > + DP_GUID,
> > > + 16,
> > > + mstb->guid);
> > > } else {
> > > -
> > > - ret = drm_dp_dpcd_write(
> > > - mstb->mgr->aux,
> > > - DP_GUID,
> > > - mstb->guid,
> > > - 16);
> > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > + DP_GUID,
> > > + mstb->guid,
> > > + 16);
> > > }
> > > }
> > > }
> > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> > > return false;
> > > }
> > >
> > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > > {
> > > struct drm_dp_sideband_msg_req_body req;
> > >
> > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > req.u.dpcd_read.dpcd_address = offset;
> > > req.u.dpcd_read.num_bytes = num_bytes;
> > > drm_dp_encode_sideband_req(&req, msg);
> > > -
> > > - return 0;
> > > }
> > >
> > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> > > {
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > struct drm_dp_link_address_ack_reply *reply;
> > > - int i, len, ret;
> > > + int i, ret;
> > >
> > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > if (!txmsg)
> > > return;
> > >
> > > txmsg->dst = mstb;
> > > - len = build_link_address(txmsg);
> > > + build_link_address(txmsg);
> > >
> > > mstb->link_address_sent = true;
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > > {
> > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > - int len;
> > > int ret;
> > >
> > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > > return -ENOMEM;
> > >
> > > txmsg->dst = mstb;
> > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > + build_enum_path_resources(txmsg, port->port_num);
> > >
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > >
> > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > {
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > struct drm_dp_mst_branch *mstb;
> > > - int len, ret, port_num;
> > > + int ret, port_num;
> > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > int i;
> > >
> > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > sinks[i] = i;
> > >
> > > txmsg->dst = mstb;
> > > - len = build_allocate_payload(txmsg, port_num,
> > > - id,
> > > - pbn, port->num_sdp_streams, sinks);
> > > + build_allocate_payload(txmsg, port_num,
> > > + id,
> > > + pbn, port->num_sdp_streams, sinks);
> > >
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > >
> > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > > struct drm_dp_mst_port *port, bool power_up)
> > > {
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > - int len, ret;
> > > + int ret;
> > >
> > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > if (!port)
> > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > > }
> > >
> > > txmsg->dst = port->parent;
> > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > >
> > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > > struct drm_dp_mst_port *port,
> > > int offset, int size, u8 *bytes)
> > > {
> > > - int len;
> > > int ret = 0;
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > struct drm_dp_mst_branch *mstb;
> > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > > goto fail_put;
> > > }
> > >
> > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > txmsg->dst = port->parent;
> > >
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > > struct drm_dp_mst_port *port,
> > > int offset, int size, u8 *bytes)
> > > {
> > > - int len;
> > > int ret;
> > > struct drm_dp_sideband_msg_tx *txmsg;
> > > struct drm_dp_mst_branch *mstb;
> > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > > goto fail_put;
> > > }
> > >
> > > - len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > txmsg->dst = mstb;
> > >
> > > drm_dp_queue_down_tx(mgr, txmsg);
> > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > > {
> > > int len;
> > > u8 replyblock[32];
> > > - int replylen, origlen, curreply;
> > > + int replylen, curreply;
> > > int ret;
> > > struct drm_dp_sideband_msg_rx *msg;
> > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > > }
> > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > >
> > > - origlen = replylen;
> > > replylen -= len;
> > > curreply = len;
> > > while (replylen > 0) {
> > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> > > mutex_lock(&mgr->lock);
> > > if (mgr->mst_primary) {
> > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > - int ret;
> > >
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > >
> > > /* dump the standard OUI branch header */
> > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > seq_printf(m, "%c", buf[i]);
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2019-12-20 14:03 ` Benjamin Gaignard
@ 2020-01-07 13:11 ` Benjamin Gaignard
2020-01-18 0:55 ` Lyude Paul
2020-01-24 22:08 ` Lyude Paul
0 siblings, 2 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-01-07 13:11 UTC (permalink / raw)
To: Jani Nikula
Cc: Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Linux Kernel Mailing List,
ML dri-devel, lyude
Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
<benjamin.gaignard@linaro.org> a écrit :
>
> Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> a écrit :
> >
> > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a écrit :
> > >
> > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com> wrote:
> > > > Fix the warnings that show up with W=1.
> > > > They are all about unused but set variables.
> > > > If functions returns are not used anymore make them void.
> > > >
> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > ---
> > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > >
> > > > changes in version 3:
> > > > - remove the hunk that may conflict with c485e2c97dae
> > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > >
> > > > changes in version 2:
> > > > - fix indentations
> > > > - when possible change functions prototype to void
> > > >
> > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------------------
> > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > > > u8 *replybuf, u8 replybuflen, bool hdr)
> > > > {
> > > > int ret;
> > > > - u8 crc4;
> > > >
> > > > if (hdr) {
> > > > u8 hdrlen;
> > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct drm_dp_sideband_msg_rx *msg,
> > > > }
> > > >
> > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > - /* do CRC */
> > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg->curchunk_len - 1);
> > >
> > > Again, someone needs to check if crc4 should be *used* instead of thrown
> > > away. Blindly throwing stuff out is not the way to go.
> >
> > Hi Dave,
> >
>
> + Lyude who has been hacking in this code recently
gentle ping for the reviewers,
Thanks,
Benjamin
>
> > Your are the original writer of this code, could you tell us if we can drop crc4
> > ao if there is something else that I have misunderstood ?
> >
> > Thanks,
> > Benjamin
> >
> > >
> > > BR,
> > > Jani.
> > >
> > > > /* copy chunk into bigger msg */
> > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg->curchunk_len - 1);
> > > > msg->curlen += msg->curchunk_len - 1;
> > > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct drm_dp_sideband_msg_rx *raw,
> > > > }
> > > > }
> > > >
> > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > {
> > > > struct drm_dp_sideband_msg_req_body req;
> > > >
> > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > req.u.dpcd_write.bytes = bytes;
> > > > drm_dp_encode_sideband_req(&req, msg);
> > > > -
> > > > - return 0;
> > > > }
> > > >
> > > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > {
> > > > struct drm_dp_sideband_msg_req_body req;
> > > >
> > > > req.req_type = DP_LINK_ADDRESS;
> > > > drm_dp_encode_sideband_req(&req, msg);
> > > > - return 0;
> > > > }
> > > >
> > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int port_num)
> > > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct drm_dp_sideband_msg_tx *msg, int por
> > > > return 0;
> > > > }
> > > >
> > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_num,
> > > > u8 vcpi, uint16_t pbn,
> > > > u8 number_sdp_streams,
> > > > u8 *sdp_stream_sink)
> > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct drm_dp_sideband_msg_tx *msg, int port_n
> > > > number_sdp_streams);
> > > > drm_dp_encode_sideband_req(&req, msg);
> > > > msg->path_msg = true;
> > > > - return 0;
> > > > }
> > > >
> > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > > int port_num, bool power_up)
> > > > {
> > > > struct drm_dp_sideband_msg_req_body req;
> > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct drm_dp_sideband_msg_tx *msg,
> > > > req.u.port_num.port_number = port_num;
> > > > drm_dp_encode_sideband_req(&req, msg);
> > > > msg->path_msg = true;
> > > > - return 0;
> > > > }
> > > >
> > > > static int drm_dp_mst_assign_payload_id(struct drm_dp_mst_topology_mgr *mgr,
> > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port *port,
> > > > */
> > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > {
> > > > - int ret;
> > > > u8 rad[6], lct;
> > > > bool send_link = false;
> > > > switch (port->pdt) {
> > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > case DP_PEER_DEVICE_SST_SINK:
> > > > /* add i2c over sideband */
> > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > break;
> > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > lct = drm_dp_calculate_rad(port, rad);
> > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> > > >
> > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid)
> > > > {
> > > > - int ret;
> > > > -
> > > > memcpy(mstb->guid, guid, 16);
> > > >
> > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > if (mstb->port_parent) {
> > > > - ret = drm_dp_send_dpcd_write(
> > > > - mstb->mgr,
> > > > - mstb->port_parent,
> > > > - DP_GUID,
> > > > - 16,
> > > > - mstb->guid);
> > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > + mstb->port_parent,
> > > > + DP_GUID,
> > > > + 16,
> > > > + mstb->guid);
> > > > } else {
> > > > -
> > > > - ret = drm_dp_dpcd_write(
> > > > - mstb->mgr->aux,
> > > > - DP_GUID,
> > > > - mstb->guid,
> > > > - 16);
> > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > + DP_GUID,
> > > > + mstb->guid,
> > > > + 16);
> > > > }
> > > > }
> > > > }
> > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
> > > > return false;
> > > > }
> > > >
> > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32 offset, u8 num_bytes)
> > > > {
> > > > struct drm_dp_sideband_msg_req_body req;
> > > >
> > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > req.u.dpcd_read.dpcd_address = offset;
> > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > drm_dp_encode_sideband_req(&req, msg);
> > > > -
> > > > - return 0;
> > > > }
> > > >
> > > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct drm_dp_mst_topology_mgr *mgr,
> > > > {
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > struct drm_dp_link_address_ack_reply *reply;
> > > > - int i, len, ret;
> > > > + int i, ret;
> > > >
> > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > if (!txmsg)
> > > > return;
> > > >
> > > > txmsg->dst = mstb;
> > > > - len = build_link_address(txmsg);
> > > > + build_link_address(txmsg);
> > > >
> > > > mstb->link_address_sent = true;
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > > > {
> > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > - int len;
> > > > int ret;
> > > >
> > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct drm_dp_mst_topology_mgr *mgr,
> > > > return -ENOMEM;
> > > >
> > > > txmsg->dst = mstb;
> > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > + build_enum_path_resources(txmsg, port->port_num);
> > > >
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > >
> > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > > {
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > struct drm_dp_mst_branch *mstb;
> > > > - int len, ret, port_num;
> > > > + int ret, port_num;
> > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > int i;
> > > >
> > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> > > > sinks[i] = i;
> > > >
> > > > txmsg->dst = mstb;
> > > > - len = build_allocate_payload(txmsg, port_num,
> > > > - id,
> > > > - pbn, port->num_sdp_streams, sinks);
> > > > + build_allocate_payload(txmsg, port_num,
> > > > + id,
> > > > + pbn, port->num_sdp_streams, sinks);
> > > >
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > >
> > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > > > struct drm_dp_mst_port *port, bool power_up)
> > > > {
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > - int len, ret;
> > > > + int ret;
> > > >
> > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > if (!port)
> > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
> > > > }
> > > >
> > > > txmsg->dst = port->parent;
> > > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > >
> > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > > > struct drm_dp_mst_port *port,
> > > > int offset, int size, u8 *bytes)
> > > > {
> > > > - int len;
> > > > int ret = 0;
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > struct drm_dp_mst_branch *mstb;
> > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct drm_dp_mst_topology_mgr *mgr,
> > > > goto fail_put;
> > > > }
> > > >
> > > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > txmsg->dst = port->parent;
> > > >
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > > > struct drm_dp_mst_port *port,
> > > > int offset, int size, u8 *bytes)
> > > > {
> > > > - int len;
> > > > int ret;
> > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > struct drm_dp_mst_branch *mstb;
> > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct drm_dp_mst_topology_mgr *mgr,
> > > > goto fail_put;
> > > > }
> > > >
> > > > - len = build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > txmsg->dst = mstb;
> > > >
> > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > > > {
> > > > int len;
> > > > u8 replyblock[32];
> > > > - int replylen, origlen, curreply;
> > > > + int replylen, curreply;
> > > > int ret;
> > > > struct drm_dp_sideband_msg_rx *msg;
> > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE : DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up)
> > > > }
> > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > >
> > > > - origlen = replylen;
> > > > replylen -= len;
> > > > curreply = len;
> > > > while (replylen > 0) {
> > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> > > > mutex_lock(&mgr->lock);
> > > > if (mgr->mst_primary) {
> > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > - int ret;
> > > >
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf, DP_RECEIVER_CAP_SIZE);
> > > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE, buf);
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > >
> > > > /* dump the standard OUI branch header */
> > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf, DP_BRANCH_OUI_HEADER_SIZE);
> > > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > seq_printf(m, "%c", buf[i]);
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-07 13:11 ` Benjamin Gaignard
@ 2020-01-18 0:55 ` Lyude Paul
2020-01-18 8:31 ` Benjamin Gaignard
2020-01-24 22:08 ` Lyude Paul
1 sibling, 1 reply; 14+ messages in thread
From: Lyude Paul @ 2020-01-18 0:55 UTC (permalink / raw)
To: Benjamin Gaignard, Jani Nikula
Cc: Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Linux Kernel Mailing List,
ML dri-devel
Hey, does this still need review?
On Tue, 2020-01-07 at 14:11 +0100, Benjamin Gaignard wrote:
> Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> a écrit :
> > Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> > <benjamin.gaignard@linaro.org> a écrit :
> > > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a
> > > écrit :
> > > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > wrote:
> > > > > Fix the warnings that show up with W=1.
> > > > > They are all about unused but set variables.
> > > > > If functions returns are not used anymore make them void.
> > > > >
> > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > ---
> > > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > >
> > > > > changes in version 3:
> > > > > - remove the hunk that may conflict with c485e2c97dae
> > > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > > >
> > > > > changes in version 2:
> > > > > - fix indentations
> > > > > - when possible change functions prototype to void
> > > > >
> > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------
> > > > > ------------
> > > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > drm_dp_sideband_msg_rx *msg,
> > > > > u8 *replybuf, u8 replybuflen,
> > > > > bool hdr)
> > > > > {
> > > > > int ret;
> > > > > - u8 crc4;
> > > > >
> > > > > if (hdr) {
> > > > > u8 hdrlen;
> > > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > drm_dp_sideband_msg_rx *msg,
> > > > > }
> > > > >
> > > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > > - /* do CRC */
> > > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg-
> > > > > >curchunk_len - 1);
> > > >
> > > > Again, someone needs to check if crc4 should be *used* instead of
> > > > thrown
> > > > away. Blindly throwing stuff out is not the way to go.
> > >
> > > Hi Dave,
> > >
> >
> > + Lyude who has been hacking in this code recently
>
> gentle ping for the reviewers,
>
> Thanks,
> Benjamin
> > > Your are the original writer of this code, could you tell us if we can
> > > drop crc4
> > > ao if there is something else that I have misunderstood ?
> > >
> > > Thanks,
> > > Benjamin
> > >
> > > > BR,
> > > > Jani.
> > > >
> > > > > /* copy chunk into bigger msg */
> > > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> > > > > >curchunk_len - 1);
> > > > > msg->curlen += msg->curchunk_len - 1;
> > > > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct
> > > > > drm_dp_sideband_msg_rx *raw,
> > > > > }
> > > > > }
> > > > >
> > > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct
> > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > > req.u.dpcd_write.bytes = bytes;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > -
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > req.req_type = DP_LINK_ADDRESS;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num)
> > > > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct
> > > > > drm_dp_sideband_msg_tx *msg, int por
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num,
> > > > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num,
> > > > > u8 vcpi, uint16_t pbn,
> > > > > u8 number_sdp_streams,
> > > > > u8 *sdp_stream_sink)
> > > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct
> > > > > drm_dp_sideband_msg_tx *msg, int port_n
> > > > > number_sdp_streams);
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > msg->path_msg = true;
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > *msg,
> > > > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > *msg,
> > > > > int port_num, bool power_up)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct
> > > > > drm_dp_sideband_msg_tx *msg,
> > > > > req.u.port_num.port_number = port_num;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > msg->path_msg = true;
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int drm_dp_mst_assign_payload_id(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct
> > > > > drm_dp_mst_port *port,
> > > > > */
> > > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > > {
> > > > > - int ret;
> > > > > u8 rad[6], lct;
> > > > > bool send_link = false;
> > > > > switch (port->pdt) {
> > > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > > case DP_PEER_DEVICE_SST_SINK:
> > > > > /* add i2c over sideband */
> > > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > break;
> > > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > > lct = drm_dp_calculate_rad(port, rad);
> > > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct
> > > > > drm_dp_aux *aux,
> > > > >
> > > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb,
> > > > > u8 *guid)
> > > > > {
> > > > > - int ret;
> > > > > -
> > > > > memcpy(mstb->guid, guid, 16);
> > > > >
> > > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > > if (mstb->port_parent) {
> > > > > - ret = drm_dp_send_dpcd_write(
> > > > > - mstb->mgr,
> > > > > - mstb->port_parent,
> > > > > - DP_GUID,
> > > > > - 16,
> > > > > - mstb->guid);
> > > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > > + mstb->port_parent,
> > > > > + DP_GUID,
> > > > > + 16,
> > > > > + mstb->guid);
> > > > > } else {
> > > > > -
> > > > > - ret = drm_dp_dpcd_write(
> > > > > - mstb->mgr->aux,
> > > > > - DP_GUID,
> > > > > - mstb->guid,
> > > > > - 16);
> > > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > > + DP_GUID,
> > > > > + mstb->guid,
> > > > > + 16);
> > > > > }
> > > > > }
> > > > > }
> > > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > return false;
> > > > > }
> > > > >
> > > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes)
> > > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct
> > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > req.u.dpcd_read.dpcd_address = offset;
> > > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > -
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr
> > > > > *mgr,
> > > > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_link_address_ack_reply *reply;
> > > > > - int i, len, ret;
> > > > > + int i, ret;
> > > > >
> > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > if (!txmsg)
> > > > > return;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_link_address(txmsg);
> > > > > + build_link_address(txmsg);
> > > > >
> > > > > mstb->link_address_sent = true;
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > - int len;
> > > > > int ret;
> > > > >
> > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > return -ENOMEM;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > > + build_enum_path_resources(txmsg, port->port_num);
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > - int len, ret, port_num;
> > > > > + int ret, port_num;
> > > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > > int i;
> > > > >
> > > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > sinks[i] = i;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_allocate_payload(txmsg, port_num,
> > > > > - id,
> > > > > - pbn, port->num_sdp_streams,
> > > > > sinks);
> > > > > + build_allocate_payload(txmsg, port_num,
> > > > > + id,
> > > > > + pbn, port->num_sdp_streams, sinks);
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port, bool
> > > > > power_up)
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > - int len, ret;
> > > > > + int ret;
> > > > >
> > > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > > if (!port)
> > > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > }
> > > > >
> > > > > txmsg->dst = port->parent;
> > > > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port,
> > > > > int offset, int size, u8 *bytes)
> > > > > {
> > > > > - int len;
> > > > > int ret = 0;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > goto fail_put;
> > > > > }
> > > > >
> > > > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > txmsg->dst = port->parent;
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port,
> > > > > int offset, int size, u8 *bytes)
> > > > > {
> > > > > - int len;
> > > > > int ret;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > goto fail_put;
> > > > > }
> > > > >
> > > > > - len = build_dpcd_write(txmsg, port->port_num, offset, size,
> > > > > bytes);
> > > > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > > txmsg->dst = mstb;
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > {
> > > > > int len;
> > > > > u8 replyblock[32];
> > > > > - int replylen, origlen, curreply;
> > > > > + int replylen, curreply;
> > > > > int ret;
> > > > > struct drm_dp_sideband_msg_rx *msg;
> > > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
> > > > > DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > }
> > > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > > >
> > > > > - origlen = replylen;
> > > > > replylen -= len;
> > > > > curreply = len;
> > > > > while (replylen > 0) {
> > > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct
> > > > > seq_file *m,
> > > > > mutex_lock(&mgr->lock);
> > > > > if (mgr->mst_primary) {
> > > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > > - int ret;
> > > > >
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE,
> > > > > buf);
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf,
> > > > > 1);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > > >
> > > > > /* dump the standard OUI branch header */
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > > seq_printf(m, "%c", buf[i]);
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Lyude Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-18 0:55 ` Lyude Paul
@ 2020-01-18 8:31 ` Benjamin Gaignard
0 siblings, 0 replies; 14+ messages in thread
From: Benjamin Gaignard @ 2020-01-18 8:31 UTC (permalink / raw)
To: Lyude Paul
Cc: Jani Nikula, Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard,
Sean Paul, David Airlie, Daniel Vetter,
Linux Kernel Mailing List, ML dri-devel
Le sam. 18 janv. 2020 à 01:55, Lyude Paul <lyude@redhat.com> a écrit :
>
> Hey, does this still need review?
yes please.
There is a question about what we should do of crc4 variable below.
Thanks,
Benjamin
>
> On Tue, 2020-01-07 at 14:11 +0100, Benjamin Gaignard wrote:
> > Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
> > <benjamin.gaignard@linaro.org> a écrit :
> > > Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> > > <benjamin.gaignard@linaro.org> a écrit :
> > > > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a
> > > > écrit :
> > > > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > wrote:
> > > > > > Fix the warnings that show up with W=1.
> > > > > > They are all about unused but set variables.
> > > > > > If functions returns are not used anymore make them void.
> > > > > >
> > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > ---
> > > > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > >
> > > > > > changes in version 3:
> > > > > > - remove the hunk that may conflict with c485e2c97dae
> > > > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > > > >
> > > > > > changes in version 2:
> > > > > > - fix indentations
> > > > > > - when possible change functions prototype to void
> > > > > >
> > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------
> > > > > > ------------
> > > > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > u8 *replybuf, u8 replybuflen,
> > > > > > bool hdr)
> > > > > > {
> > > > > > int ret;
> > > > > > - u8 crc4;
> > > > > >
> > > > > > if (hdr) {
> > > > > > u8 hdrlen;
> > > > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > }
> > > > > >
> > > > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > > > - /* do CRC */
> > > > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg-
> > > > > > >curchunk_len - 1);
> > > > >
> > > > > Again, someone needs to check if crc4 should be *used* instead of
> > > > > thrown
> > > > > away. Blindly throwing stuff out is not the way to go.
> > > >
> > > > Hi Dave,
> > > >
> > >
> > > + Lyude who has been hacking in this code recently
> >
> > gentle ping for the reviewers,
> >
> > Thanks,
> > Benjamin
> > > > Your are the original writer of this code, could you tell us if we can
> > > > drop crc4
> > > > ao if there is something else that I have misunderstood ?
> > > >
> > > > Thanks,
> > > > Benjamin
> > > >
> > > > > BR,
> > > > > Jani.
> > > > >
> > > > > > /* copy chunk into bigger msg */
> > > > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> > > > > > >curchunk_len - 1);
> > > > > > msg->curlen += msg->curchunk_len - 1;
> > > > > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct
> > > > > > drm_dp_sideband_msg_rx *raw,
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct
> > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > > > req.u.dpcd_write.bytes = bytes;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > -
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > req.req_type = DP_LINK_ADDRESS;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num)
> > > > > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct
> > > > > > drm_dp_sideband_msg_tx *msg, int por
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num,
> > > > > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num,
> > > > > > u8 vcpi, uint16_t pbn,
> > > > > > u8 number_sdp_streams,
> > > > > > u8 *sdp_stream_sink)
> > > > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct
> > > > > > drm_dp_sideband_msg_tx *msg, int port_n
> > > > > > number_sdp_streams);
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > msg->path_msg = true;
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > > *msg,
> > > > > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > > *msg,
> > > > > > int port_num, bool power_up)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct
> > > > > > drm_dp_sideband_msg_tx *msg,
> > > > > > req.u.port_num.port_number = port_num;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > msg->path_msg = true;
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int drm_dp_mst_assign_payload_id(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct
> > > > > > drm_dp_mst_port *port,
> > > > > > */
> > > > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > > > {
> > > > > > - int ret;
> > > > > > u8 rad[6], lct;
> > > > > > bool send_link = false;
> > > > > > switch (port->pdt) {
> > > > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > > > case DP_PEER_DEVICE_SST_SINK:
> > > > > > /* add i2c over sideband */
> > > > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > break;
> > > > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > > > lct = drm_dp_calculate_rad(port, rad);
> > > > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct
> > > > > > drm_dp_aux *aux,
> > > > > >
> > > > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb,
> > > > > > u8 *guid)
> > > > > > {
> > > > > > - int ret;
> > > > > > -
> > > > > > memcpy(mstb->guid, guid, 16);
> > > > > >
> > > > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > > > if (mstb->port_parent) {
> > > > > > - ret = drm_dp_send_dpcd_write(
> > > > > > - mstb->mgr,
> > > > > > - mstb->port_parent,
> > > > > > - DP_GUID,
> > > > > > - 16,
> > > > > > - mstb->guid);
> > > > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > > > + mstb->port_parent,
> > > > > > + DP_GUID,
> > > > > > + 16,
> > > > > > + mstb->guid);
> > > > > > } else {
> > > > > > -
> > > > > > - ret = drm_dp_dpcd_write(
> > > > > > - mstb->mgr->aux,
> > > > > > - DP_GUID,
> > > > > > - mstb->guid,
> > > > > > - 16);
> > > > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > > > + DP_GUID,
> > > > > > + mstb->guid,
> > > > > > + 16);
> > > > > > }
> > > > > > }
> > > > > > }
> > > > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > return false;
> > > > > > }
> > > > > >
> > > > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct
> > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > req.u.dpcd_read.dpcd_address = offset;
> > > > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > -
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr
> > > > > > *mgr,
> > > > > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_link_address_ack_reply *reply;
> > > > > > - int i, len, ret;
> > > > > > + int i, ret;
> > > > > >
> > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > if (!txmsg)
> > > > > > return;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_link_address(txmsg);
> > > > > > + build_link_address(txmsg);
> > > > > >
> > > > > > mstb->link_address_sent = true;
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > - int len;
> > > > > > int ret;
> > > > > >
> > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > > > + build_enum_path_resources(txmsg, port->port_num);
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > - int len, ret, port_num;
> > > > > > + int ret, port_num;
> > > > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > > > int i;
> > > > > >
> > > > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > sinks[i] = i;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_allocate_payload(txmsg, port_num,
> > > > > > - id,
> > > > > > - pbn, port->num_sdp_streams,
> > > > > > sinks);
> > > > > > + build_allocate_payload(txmsg, port_num,
> > > > > > + id,
> > > > > > + pbn, port->num_sdp_streams, sinks);
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port, bool
> > > > > > power_up)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > - int len, ret;
> > > > > > + int ret;
> > > > > >
> > > > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > > > if (!port)
> > > > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > }
> > > > > >
> > > > > > txmsg->dst = port->parent;
> > > > > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port,
> > > > > > int offset, int size, u8 *bytes)
> > > > > > {
> > > > > > - int len;
> > > > > > int ret = 0;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > goto fail_put;
> > > > > > }
> > > > > >
> > > > > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > > txmsg->dst = port->parent;
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port,
> > > > > > int offset, int size, u8 *bytes)
> > > > > > {
> > > > > > - int len;
> > > > > > int ret;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > goto fail_put;
> > > > > > }
> > > > > >
> > > > > > - len = build_dpcd_write(txmsg, port->port_num, offset, size,
> > > > > > bytes);
> > > > > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > > > txmsg->dst = mstb;
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > {
> > > > > > int len;
> > > > > > u8 replyblock[32];
> > > > > > - int replylen, origlen, curreply;
> > > > > > + int replylen, curreply;
> > > > > > int ret;
> > > > > > struct drm_dp_sideband_msg_rx *msg;
> > > > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
> > > > > > DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > }
> > > > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > > > >
> > > > > > - origlen = replylen;
> > > > > > replylen -= len;
> > > > > > curreply = len;
> > > > > > while (replylen > 0) {
> > > > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct
> > > > > > seq_file *m,
> > > > > > mutex_lock(&mgr->lock);
> > > > > > if (mgr->mst_primary) {
> > > > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > > > - int ret;
> > > > > >
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE,
> > > > > > buf);
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf,
> > > > > > 1);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > > > >
> > > > > > /* dump the standard OUI branch header */
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > > > seq_printf(m, "%c", buf[i]);
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel Open Source Graphics Center
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> Cheers,
> Lyude Paul
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-07 13:11 ` Benjamin Gaignard
2020-01-18 0:55 ` Lyude Paul
@ 2020-01-24 22:08 ` Lyude Paul
2020-01-27 13:08 ` Benjamin Gaignard
1 sibling, 1 reply; 14+ messages in thread
From: Lyude Paul @ 2020-01-24 22:08 UTC (permalink / raw)
To: Benjamin Gaignard, Jani Nikula
Cc: Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard, Sean Paul,
David Airlie, Daniel Vetter, Linux Kernel Mailing List,
ML dri-devel
On Tue, 2020-01-07 at 14:11 +0100, Benjamin Gaignard wrote:
> Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
> <benjamin.gaignard@linaro.org> a écrit :
> > Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> > <benjamin.gaignard@linaro.org> a écrit :
> > > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a
> > > écrit :
> > > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > wrote:
> > > > > Fix the warnings that show up with W=1.
> > > > > They are all about unused but set variables.
> > > > > If functions returns are not used anymore make them void.
> > > > >
> > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > ---
> > > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > >
> > > > > changes in version 3:
> > > > > - remove the hunk that may conflict with c485e2c97dae
> > > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > > >
> > > > > changes in version 2:
> > > > > - fix indentations
> > > > > - when possible change functions prototype to void
> > > > >
> > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------
> > > > > ------------
> > > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > drm_dp_sideband_msg_rx *msg,
> > > > > u8 *replybuf, u8 replybuflen,
> > > > > bool hdr)
> > > > > {
> > > > > int ret;
> > > > > - u8 crc4;
> > > > >
> > > > > if (hdr) {
> > > > > u8 hdrlen;
> > > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > drm_dp_sideband_msg_rx *msg,
> > > > > }
> > > > >
> > > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > > - /* do CRC */
> > > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg-
> > > > > >curchunk_len - 1);
> > > >
> > > > Again, someone needs to check if crc4 should be *used* instead of
> > > > thrown
> > > > away. Blindly throwing stuff out is not the way to go.
> > >
> > > Hi Dave,
> > >
> >
> > + Lyude who has been hacking in this code recently
>
> gentle ping for the reviewers,
>
hi-actually yes, we should probably be using this instead of just dropping
this. Also, I didn't write this code originally I just refactored a bunch of
it - Dave Airlied is the original author, but the original version of this
code was written ages ago. tbh, I think it's a safe bet to say that they
probably did mean to use this but forgot to and no one noticed until now.
> Thanks,
> Benjamin
> > > Your are the original writer of this code, could you tell us if we can
> > > drop crc4
> > > ao if there is something else that I have misunderstood ?
> > >
> > > Thanks,
> > > Benjamin
> > >
> > > > BR,
> > > > Jani.
> > > >
> > > > > /* copy chunk into bigger msg */
> > > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> > > > > >curchunk_len - 1);
> > > > > msg->curlen += msg->curchunk_len - 1;
> > > > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct
> > > > > drm_dp_sideband_msg_rx *raw,
> > > > > }
> > > > > }
> > > > >
> > > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct
> > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > > req.u.dpcd_write.bytes = bytes;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > -
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > req.req_type = DP_LINK_ADDRESS;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num)
> > > > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct
> > > > > drm_dp_sideband_msg_tx *msg, int por
> > > > > return 0;
> > > > > }
> > > > >
> > > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num,
> > > > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > *msg, int port_num,
> > > > > u8 vcpi, uint16_t pbn,
> > > > > u8 number_sdp_streams,
> > > > > u8 *sdp_stream_sink)
> > > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct
> > > > > drm_dp_sideband_msg_tx *msg, int port_n
> > > > > number_sdp_streams);
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > msg->path_msg = true;
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > *msg,
> > > > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > *msg,
> > > > > int port_num, bool power_up)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct
> > > > > drm_dp_sideband_msg_tx *msg,
> > > > > req.u.port_num.port_number = port_num;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > msg->path_msg = true;
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int drm_dp_mst_assign_payload_id(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct
> > > > > drm_dp_mst_port *port,
> > > > > */
> > > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > > {
> > > > > - int ret;
> > > > > u8 rad[6], lct;
> > > > > bool send_link = false;
> > > > > switch (port->pdt) {
> > > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > > case DP_PEER_DEVICE_SST_SINK:
> > > > > /* add i2c over sideband */
> > > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > break;
> > > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > > lct = drm_dp_calculate_rad(port, rad);
> > > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct
> > > > > drm_dp_aux *aux,
> > > > >
> > > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb,
> > > > > u8 *guid)
> > > > > {
> > > > > - int ret;
> > > > > -
> > > > > memcpy(mstb->guid, guid, 16);
> > > > >
> > > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > > if (mstb->port_parent) {
> > > > > - ret = drm_dp_send_dpcd_write(
> > > > > - mstb->mgr,
> > > > > - mstb->port_parent,
> > > > > - DP_GUID,
> > > > > - 16,
> > > > > - mstb->guid);
> > > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > > + mstb->port_parent,
> > > > > + DP_GUID,
> > > > > + 16,
> > > > > + mstb->guid);
> > > > > } else {
> > > > > -
> > > > > - ret = drm_dp_dpcd_write(
> > > > > - mstb->mgr->aux,
> > > > > - DP_GUID,
> > > > > - mstb->guid,
> > > > > - 16);
> > > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > > + DP_GUID,
> > > > > + mstb->guid,
> > > > > + 16);
> > > > > }
> > > > > }
> > > > > }
> > > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > return false;
> > > > > }
> > > > >
> > > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes)
> > > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > port_num, u32 offset, u8 num_bytes)
> > > > > {
> > > > > struct drm_dp_sideband_msg_req_body req;
> > > > >
> > > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct
> > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > req.u.dpcd_read.dpcd_address = offset;
> > > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > -
> > > > > - return 0;
> > > > > }
> > > > >
> > > > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr
> > > > > *mgr,
> > > > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_link_address_ack_reply *reply;
> > > > > - int i, len, ret;
> > > > > + int i, ret;
> > > > >
> > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > if (!txmsg)
> > > > > return;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_link_address(txmsg);
> > > > > + build_link_address(txmsg);
> > > > >
> > > > > mstb->link_address_sent = true;
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > - int len;
> > > > > int ret;
> > > > >
> > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > return -ENOMEM;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > > + build_enum_path_resources(txmsg, port->port_num);
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > - int len, ret, port_num;
> > > > > + int ret, port_num;
> > > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > > int i;
> > > > >
> > > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > sinks[i] = i;
> > > > >
> > > > > txmsg->dst = mstb;
> > > > > - len = build_allocate_payload(txmsg, port_num,
> > > > > - id,
> > > > > - pbn, port->num_sdp_streams,
> > > > > sinks);
> > > > > + build_allocate_payload(txmsg, port_num,
> > > > > + id,
> > > > > + pbn, port->num_sdp_streams, sinks);
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port, bool
> > > > > power_up)
> > > > > {
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > - int len, ret;
> > > > > + int ret;
> > > > >
> > > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > > if (!port)
> > > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > }
> > > > >
> > > > > txmsg->dst = port->parent;
> > > > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > >
> > > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port,
> > > > > int offset, int size, u8 *bytes)
> > > > > {
> > > > > - int len;
> > > > > int ret = 0;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > goto fail_put;
> > > > > }
> > > > >
> > > > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > txmsg->dst = port->parent;
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > struct drm_dp_mst_port *port,
> > > > > int offset, int size, u8 *bytes)
> > > > > {
> > > > > - int len;
> > > > > int ret;
> > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > struct drm_dp_mst_branch *mstb;
> > > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct
> > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > goto fail_put;
> > > > > }
> > > > >
> > > > > - len = build_dpcd_write(txmsg, port->port_num, offset, size,
> > > > > bytes);
> > > > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > > txmsg->dst = mstb;
> > > > >
> > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > {
> > > > > int len;
> > > > > u8 replyblock[32];
> > > > > - int replylen, origlen, curreply;
> > > > > + int replylen, curreply;
> > > > > int ret;
> > > > > struct drm_dp_sideband_msg_rx *msg;
> > > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
> > > > > DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > }
> > > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > > >
> > > > > - origlen = replylen;
> > > > > replylen -= len;
> > > > > curreply = len;
> > > > > while (replylen > 0) {
> > > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct
> > > > > seq_file *m,
> > > > > mutex_lock(&mgr->lock);
> > > > > if (mgr->mst_primary) {
> > > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > > - int ret;
> > > > >
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > DP_RECEIVER_CAP_SIZE);
> > > > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE,
> > > > > buf);
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf,
> > > > > 1);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > > >
> > > > > /* dump the standard OUI branch header */
> > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > > seq_printf(m, "%c", buf[i]);
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> > > > _______________________________________________
> > > > dri-devel mailing list
> > > > dri-devel@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Cheers,
Lyude Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-24 22:08 ` Lyude Paul
@ 2020-01-27 13:08 ` Benjamin Gaignard
2020-01-30 22:22 ` Lyude Paul
0 siblings, 1 reply; 14+ messages in thread
From: Benjamin Gaignard @ 2020-01-27 13:08 UTC (permalink / raw)
To: Lyude Paul
Cc: Jani Nikula, Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard,
Sean Paul, David Airlie, Daniel Vetter,
Linux Kernel Mailing List, ML dri-devel
Le ven. 24 janv. 2020 à 23:08, Lyude Paul <lyude@redhat.com> a écrit :
>
> On Tue, 2020-01-07 at 14:11 +0100, Benjamin Gaignard wrote:
> > Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
> > <benjamin.gaignard@linaro.org> a écrit :
> > > Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> > > <benjamin.gaignard@linaro.org> a écrit :
> > > > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <jani.nikula@linux.intel.com> a
> > > > écrit :
> > > > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > wrote:
> > > > > > Fix the warnings that show up with W=1.
> > > > > > They are all about unused but set variables.
> > > > > > If functions returns are not used anymore make them void.
> > > > > >
> > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > ---
> > > > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > >
> > > > > > changes in version 3:
> > > > > > - remove the hunk that may conflict with c485e2c97dae
> > > > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > > > >
> > > > > > changes in version 2:
> > > > > > - fix indentations
> > > > > > - when possible change functions prototype to void
> > > > > >
> > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++----------
> > > > > > ------------
> > > > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > u8 *replybuf, u8 replybuflen,
> > > > > > bool hdr)
> > > > > > {
> > > > > > int ret;
> > > > > > - u8 crc4;
> > > > > >
> > > > > > if (hdr) {
> > > > > > u8 hdrlen;
> > > > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > }
> > > > > >
> > > > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > > > - /* do CRC */
> > > > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg-
> > > > > > >curchunk_len - 1);
> > > > >
> > > > > Again, someone needs to check if crc4 should be *used* instead of
> > > > > thrown
> > > > > away. Blindly throwing stuff out is not the way to go.
> > > >
> > > > Hi Dave,
> > > >
> > >
> > > + Lyude who has been hacking in this code recently
> >
> > gentle ping for the reviewers,
> >
> hi-actually yes, we should probably be using this instead of just dropping
> this. Also, I didn't write this code originally I just refactored a bunch of
> it - Dave Airlied is the original author, but the original version of this
> code was written ages ago. tbh, I think it's a safe bet to say that they
> probably did mean to use this but forgot to and no one noticed until now.
Hi,
Any clue about how to use crc value ? Does it have to be checked
against something else ?
If crc are not matching what should we do of the data copied just before ?
Benjamin
>
> > Thanks,
> > Benjamin
> > > > Your are the original writer of this code, could you tell us if we can
> > > > drop crc4
> > > > ao if there is something else that I have misunderstood ?
> > > >
> > > > Thanks,
> > > > Benjamin
> > > >
> > > > > BR,
> > > > > Jani.
> > > > >
> > > > > > /* copy chunk into bigger msg */
> > > > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> > > > > > >curchunk_len - 1);
> > > > > > msg->curlen += msg->curchunk_len - 1;
> > > > > > @@ -1014,7 +1011,7 @@ static bool drm_dp_sideband_parse_req(struct
> > > > > > drm_dp_sideband_msg_rx *raw,
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct
> > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > > > req.u.dpcd_write.bytes = bytes;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > -
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > > +static void build_link_address(struct drm_dp_sideband_msg_tx *msg)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > req.req_type = DP_LINK_ADDRESS;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int build_enum_path_resources(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num)
> > > > > > @@ -1048,7 +1042,7 @@ static int build_enum_path_resources(struct
> > > > > > drm_dp_sideband_msg_tx *msg, int por
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num,
> > > > > > +static void build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > > *msg, int port_num,
> > > > > > u8 vcpi, uint16_t pbn,
> > > > > > u8 number_sdp_streams,
> > > > > > u8 *sdp_stream_sink)
> > > > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct
> > > > > > drm_dp_sideband_msg_tx *msg, int port_n
> > > > > > number_sdp_streams);
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > msg->path_msg = true;
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > > *msg,
> > > > > > +static void build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > > *msg,
> > > > > > int port_num, bool power_up)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct
> > > > > > drm_dp_sideband_msg_tx *msg,
> > > > > > req.u.port_num.port_number = port_num;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > msg->path_msg = true;
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int drm_dp_mst_assign_payload_id(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct
> > > > > > drm_dp_mst_port *port,
> > > > > > */
> > > > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > > > {
> > > > > > - int ret;
> > > > > > u8 rad[6], lct;
> > > > > > bool send_link = false;
> > > > > > switch (port->pdt) {
> > > > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > > > case DP_PEER_DEVICE_SST_SINK:
> > > > > > /* add i2c over sideband */
> > > > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > break;
> > > > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > > > lct = drm_dp_calculate_rad(port, rad);
> > > > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct
> > > > > > drm_dp_aux *aux,
> > > > > >
> > > > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb,
> > > > > > u8 *guid)
> > > > > > {
> > > > > > - int ret;
> > > > > > -
> > > > > > memcpy(mstb->guid, guid, 16);
> > > > > >
> > > > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > > > if (mstb->port_parent) {
> > > > > > - ret = drm_dp_send_dpcd_write(
> > > > > > - mstb->mgr,
> > > > > > - mstb->port_parent,
> > > > > > - DP_GUID,
> > > > > > - 16,
> > > > > > - mstb->guid);
> > > > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > > > + mstb->port_parent,
> > > > > > + DP_GUID,
> > > > > > + 16,
> > > > > > + mstb->guid);
> > > > > > } else {
> > > > > > -
> > > > > > - ret = drm_dp_dpcd_write(
> > > > > > - mstb->mgr->aux,
> > > > > > - DP_GUID,
> > > > > > - mstb->guid,
> > > > > > - 16);
> > > > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > > > + DP_GUID,
> > > > > > + mstb->guid,
> > > > > > + 16);
> > > > > > }
> > > > > > }
> > > > > > }
> > > > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > return false;
> > > > > > }
> > > > > >
> > > > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg, u8
> > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > >
> > > > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct
> > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > req.u.dpcd_read.dpcd_address = offset;
> > > > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > -
> > > > > > - return 0;
> > > > > > }
> > > > > >
> > > > > > static int drm_dp_send_sideband_msg(struct drm_dp_mst_topology_mgr
> > > > > > *mgr,
> > > > > > @@ -2429,14 +2413,14 @@ static void drm_dp_send_link_address(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_link_address_ack_reply *reply;
> > > > > > - int i, len, ret;
> > > > > > + int i, ret;
> > > > > >
> > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > if (!txmsg)
> > > > > > return;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_link_address(txmsg);
> > > > > > + build_link_address(txmsg);
> > > > > >
> > > > > > mstb->link_address_sent = true;
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > - int len;
> > > > > > int ret;
> > > > > >
> > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > > > + build_enum_path_resources(txmsg, port->port_num);
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > - int len, ret, port_num;
> > > > > > + int ret, port_num;
> > > > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > > > int i;
> > > > > >
> > > > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > sinks[i] = i;
> > > > > >
> > > > > > txmsg->dst = mstb;
> > > > > > - len = build_allocate_payload(txmsg, port_num,
> > > > > > - id,
> > > > > > - pbn, port->num_sdp_streams,
> > > > > > sinks);
> > > > > > + build_allocate_payload(txmsg, port_num,
> > > > > > + id,
> > > > > > + pbn, port->num_sdp_streams, sinks);
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port, bool
> > > > > > power_up)
> > > > > > {
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > - int len, ret;
> > > > > > + int ret;
> > > > > >
> > > > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > > > if (!port)
> > > > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > }
> > > > > >
> > > > > > txmsg->dst = port->parent;
> > > > > > - len = build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > >
> > > > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port,
> > > > > > int offset, int size, u8 *bytes)
> > > > > > {
> > > > > > - int len;
> > > > > > int ret = 0;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > goto fail_put;
> > > > > > }
> > > > > >
> > > > > > - len = build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > > txmsg->dst = port->parent;
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > struct drm_dp_mst_port *port,
> > > > > > int offset, int size, u8 *bytes)
> > > > > > {
> > > > > > - int len;
> > > > > > int ret;
> > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > goto fail_put;
> > > > > > }
> > > > > >
> > > > > > - len = build_dpcd_write(txmsg, port->port_num, offset, size,
> > > > > > bytes);
> > > > > > + build_dpcd_write(txmsg, port->port_num, offset, size, bytes);
> > > > > > txmsg->dst = mstb;
> > > > > >
> > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > {
> > > > > > int len;
> > > > > > u8 replyblock[32];
> > > > > > - int replylen, origlen, curreply;
> > > > > > + int replylen, curreply;
> > > > > > int ret;
> > > > > > struct drm_dp_sideband_msg_rx *msg;
> > > > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
> > > > > > DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > }
> > > > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > > > >
> > > > > > - origlen = replylen;
> > > > > > replylen -= len;
> > > > > > curreply = len;
> > > > > > while (replylen > 0) {
> > > > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct
> > > > > > seq_file *m,
> > > > > > mutex_lock(&mgr->lock);
> > > > > > if (mgr->mst_primary) {
> > > > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > > > - int ret;
> > > > > >
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > seq_printf(m, "dpcd: %*ph\n", DP_RECEIVER_CAP_SIZE,
> > > > > > buf);
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf,
> > > > > > 1);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > > > >
> > > > > > /* dump the standard OUI branch header */
> > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > seq_printf(m, "branch oui: %*phN devid: ", 3, buf);
> > > > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > > > seq_printf(m, "%c", buf[i]);
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel Open Source Graphics Center
> > > > > _______________________________________________
> > > > > dri-devel mailing list
> > > > > dri-devel@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> Cheers,
> Lyude Paul
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-27 13:08 ` Benjamin Gaignard
@ 2020-01-30 22:22 ` Lyude Paul
2020-01-30 23:22 ` Dave Airlie
0 siblings, 1 reply; 14+ messages in thread
From: Lyude Paul @ 2020-01-30 22:22 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Jani Nikula, Benjamin Gaignard, Maarten Lankhorst, Maxime Ripard,
Sean Paul, David Airlie, Daniel Vetter,
Linux Kernel Mailing List, ML dri-devel
On Mon, 2020-01-27 at 14:08 +0100, Benjamin Gaignard wrote:
> Le ven. 24 janv. 2020 à 23:08, Lyude Paul <lyude@redhat.com> a écrit :
> > On Tue, 2020-01-07 at 14:11 +0100, Benjamin Gaignard wrote:
> > > Le ven. 20 déc. 2019 à 15:03, Benjamin Gaignard
> > > <benjamin.gaignard@linaro.org> a écrit :
> > > > Le lun. 16 déc. 2019 à 09:28, Benjamin Gaignard
> > > > <benjamin.gaignard@linaro.org> a écrit :
> > > > > Le mer. 4 déc. 2019 à 17:47, Jani Nikula <
> > > > > jani.nikula@linux.intel.com> a
> > > > > écrit :
> > > > > > On Thu, 28 Nov 2019, Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > wrote:
> > > > > > > Fix the warnings that show up with W=1.
> > > > > > > They are all about unused but set variables.
> > > > > > > If functions returns are not used anymore make them void.
> > > > > > >
> > > > > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > > > > ---
> > > > > > > CC: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > > >
> > > > > > > changes in version 3:
> > > > > > > - remove the hunk that may conflict with c485e2c97dae
> > > > > > > ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
> > > > > > >
> > > > > > > changes in version 2:
> > > > > > > - fix indentations
> > > > > > > - when possible change functions prototype to void
> > > > > > >
> > > > > > > drivers/gpu/drm/drm_dp_mst_topology.c | 83 +++++++++++++------
> > > > > > > ----
> > > > > > > ------------
> > > > > > > 1 file changed, 31 insertions(+), 52 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > > index 1437bc46368b..d5cb5688b5dd 100644
> > > > > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > > > > > > @@ -674,7 +674,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > > u8 *replybuf, u8
> > > > > > > replybuflen,
> > > > > > > bool hdr)
> > > > > > > {
> > > > > > > int ret;
> > > > > > > - u8 crc4;
> > > > > > >
> > > > > > > if (hdr) {
> > > > > > > u8 hdrlen;
> > > > > > > @@ -716,8 +715,6 @@ static bool drm_dp_sideband_msg_build(struct
> > > > > > > drm_dp_sideband_msg_rx *msg,
> > > > > > > }
> > > > > > >
> > > > > > > if (msg->curchunk_idx >= msg->curchunk_len) {
> > > > > > > - /* do CRC */
> > > > > > > - crc4 = drm_dp_msg_data_crc4(msg->chunk, msg-
> > > > > > > > curchunk_len - 1);
> > > > > >
> > > > > > Again, someone needs to check if crc4 should be *used* instead of
> > > > > > thrown
> > > > > > away. Blindly throwing stuff out is not the way to go.
> > > > >
> > > > > Hi Dave,
> > > > >
> > > >
> > > > + Lyude who has been hacking in this code recently
> > >
> > > gentle ping for the reviewers,
> > >
> > hi-actually yes, we should probably be using this instead of just dropping
> > this. Also, I didn't write this code originally I just refactored a bunch
> > of
> > it - Dave Airlied is the original author, but the original version of this
> > code was written ages ago. tbh, I think it's a safe bet to say that they
> > probably did mean to use this but forgot to and no one noticed until now.
>
> Hi,
>
> Any clue about how to use crc value ? Does it have to be checked
> against something else ?
> If crc are not matching what should we do of the data copied just before ?
We should be able to just take the CRC value from the sideband message and
then generate our own CRC value using the sideband message contents, and check
if the two are equal. If they aren't, something went wrong and we didn't
receive the message properly.
Now as to what we should do when we have CRC mismatches? That's a bit more
difficult. If you have access to the DP MST spec, I suppose a place to start
figuring that out would be checking if there's a way for us to request that a
branch device resend whatever message it sent previously. If there isn't, I
guess we should just print an error in dmesg (possibly with a hexdump of the
failed message as well) and not forward the message to the driver. Not sure of
any better way of handling it then that
>
> Benjamin
>
> > > Thanks,
> > > Benjamin
> > > > > Your are the original writer of this code, could you tell us if we
> > > > > can
> > > > > drop crc4
> > > > > ao if there is something else that I have misunderstood ?
> > > > >
> > > > > Thanks,
> > > > > Benjamin
> > > > >
> > > > > > BR,
> > > > > > Jani.
> > > > > >
> > > > > > > /* copy chunk into bigger msg */
> > > > > > > memcpy(&msg->msg[msg->curlen], msg->chunk, msg-
> > > > > > > > curchunk_len - 1);
> > > > > > > msg->curlen += msg->curchunk_len - 1;
> > > > > > > @@ -1014,7 +1011,7 @@ static bool
> > > > > > > drm_dp_sideband_parse_req(struct
> > > > > > > drm_dp_sideband_msg_rx *raw,
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > -static int build_dpcd_write(struct drm_dp_sideband_msg_tx *msg,
> > > > > > > u8
> > > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > > +static void build_dpcd_write(struct drm_dp_sideband_msg_tx
> > > > > > > *msg, u8
> > > > > > > port_num, u32 offset, u8 num_bytes, u8 *bytes)
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > >
> > > > > > > @@ -1024,17 +1021,14 @@ static int build_dpcd_write(struct
> > > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > > req.u.dpcd_write.num_bytes = num_bytes;
> > > > > > > req.u.dpcd_write.bytes = bytes;
> > > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > > -
> > > > > > > - return 0;
> > > > > > > }
> > > > > > >
> > > > > > > -static int build_link_address(struct drm_dp_sideband_msg_tx
> > > > > > > *msg)
> > > > > > > +static void build_link_address(struct drm_dp_sideband_msg_tx
> > > > > > > *msg)
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > >
> > > > > > > req.req_type = DP_LINK_ADDRESS;
> > > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > > - return 0;
> > > > > > > }
> > > > > > >
> > > > > > > static int build_enum_path_resources(struct
> > > > > > > drm_dp_sideband_msg_tx
> > > > > > > *msg, int port_num)
> > > > > > > @@ -1048,7 +1042,7 @@ static int
> > > > > > > build_enum_path_resources(struct
> > > > > > > drm_dp_sideband_msg_tx *msg, int por
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > -static int build_allocate_payload(struct drm_dp_sideband_msg_tx
> > > > > > > *msg, int port_num,
> > > > > > > +static void build_allocate_payload(struct
> > > > > > > drm_dp_sideband_msg_tx
> > > > > > > *msg, int port_num,
> > > > > > > u8 vcpi, uint16_t pbn,
> > > > > > > u8 number_sdp_streams,
> > > > > > > u8 *sdp_stream_sink)
> > > > > > > @@ -1064,10 +1058,9 @@ static int build_allocate_payload(struct
> > > > > > > drm_dp_sideband_msg_tx *msg, int port_n
> > > > > > > number_sdp_streams);
> > > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > > msg->path_msg = true;
> > > > > > > - return 0;
> > > > > > > }
> > > > > > >
> > > > > > > -static int build_power_updown_phy(struct drm_dp_sideband_msg_tx
> > > > > > > *msg,
> > > > > > > +static void build_power_updown_phy(struct
> > > > > > > drm_dp_sideband_msg_tx
> > > > > > > *msg,
> > > > > > > int port_num, bool power_up)
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > > @@ -1080,7 +1073,6 @@ static int build_power_updown_phy(struct
> > > > > > > drm_dp_sideband_msg_tx *msg,
> > > > > > > req.u.port_num.port_number = port_num;
> > > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > > msg->path_msg = true;
> > > > > > > - return 0;
> > > > > > > }
> > > > > > >
> > > > > > > static int drm_dp_mst_assign_payload_id(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > @@ -1746,14 +1738,13 @@ static u8 drm_dp_calculate_rad(struct
> > > > > > > drm_dp_mst_port *port,
> > > > > > > */
> > > > > > > static bool drm_dp_port_setup_pdt(struct drm_dp_mst_port *port)
> > > > > > > {
> > > > > > > - int ret;
> > > > > > > u8 rad[6], lct;
> > > > > > > bool send_link = false;
> > > > > > > switch (port->pdt) {
> > > > > > > case DP_PEER_DEVICE_DP_LEGACY_CONV:
> > > > > > > case DP_PEER_DEVICE_SST_SINK:
> > > > > > > /* add i2c over sideband */
> > > > > > > - ret = drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > > + drm_dp_mst_register_i2c_bus(&port->aux);
> > > > > > > break;
> > > > > > > case DP_PEER_DEVICE_MST_BRANCHING:
> > > > > > > lct = drm_dp_calculate_rad(port, rad);
> > > > > > > @@ -1823,25 +1814,20 @@ ssize_t drm_dp_mst_dpcd_write(struct
> > > > > > > drm_dp_aux *aux,
> > > > > > >
> > > > > > > static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch
> > > > > > > *mstb,
> > > > > > > u8 *guid)
> > > > > > > {
> > > > > > > - int ret;
> > > > > > > -
> > > > > > > memcpy(mstb->guid, guid, 16);
> > > > > > >
> > > > > > > if (!drm_dp_validate_guid(mstb->mgr, mstb->guid)) {
> > > > > > > if (mstb->port_parent) {
> > > > > > > - ret = drm_dp_send_dpcd_write(
> > > > > > > - mstb->mgr,
> > > > > > > - mstb->port_parent,
> > > > > > > - DP_GUID,
> > > > > > > - 16,
> > > > > > > - mstb->guid);
> > > > > > > + drm_dp_send_dpcd_write(mstb->mgr,
> > > > > > > + mstb->port_parent,
> > > > > > > + DP_GUID,
> > > > > > > + 16,
> > > > > > > + mstb->guid);
> > > > > > > } else {
> > > > > > > -
> > > > > > > - ret = drm_dp_dpcd_write(
> > > > > > > - mstb->mgr->aux,
> > > > > > > - DP_GUID,
> > > > > > > - mstb->guid,
> > > > > > > - 16);
> > > > > > > + drm_dp_dpcd_write(mstb->mgr->aux,
> > > > > > > + DP_GUID,
> > > > > > > + mstb->guid,
> > > > > > > + 16);
> > > > > > > }
> > > > > > > }
> > > > > > > }
> > > > > > > @@ -2197,7 +2183,7 @@ static bool drm_dp_validate_guid(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > return false;
> > > > > > > }
> > > > > > >
> > > > > > > -static int build_dpcd_read(struct drm_dp_sideband_msg_tx *msg,
> > > > > > > u8
> > > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > > +static void build_dpcd_read(struct drm_dp_sideband_msg_tx *msg,
> > > > > > > u8
> > > > > > > port_num, u32 offset, u8 num_bytes)
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_req_body req;
> > > > > > >
> > > > > > > @@ -2206,8 +2192,6 @@ static int build_dpcd_read(struct
> > > > > > > drm_dp_sideband_msg_tx *msg, u8 port_num, u32
> > > > > > > req.u.dpcd_read.dpcd_address = offset;
> > > > > > > req.u.dpcd_read.num_bytes = num_bytes;
> > > > > > > drm_dp_encode_sideband_req(&req, msg);
> > > > > > > -
> > > > > > > - return 0;
> > > > > > > }
> > > > > > >
> > > > > > > static int drm_dp_send_sideband_msg(struct
> > > > > > > drm_dp_mst_topology_mgr
> > > > > > > *mgr,
> > > > > > > @@ -2429,14 +2413,14 @@ static void
> > > > > > > drm_dp_send_link_address(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > struct drm_dp_link_address_ack_reply *reply;
> > > > > > > - int i, len, ret;
> > > > > > > + int i, ret;
> > > > > > >
> > > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > > if (!txmsg)
> > > > > > > return;
> > > > > > >
> > > > > > > txmsg->dst = mstb;
> > > > > > > - len = build_link_address(txmsg);
> > > > > > > + build_link_address(txmsg);
> > > > > > >
> > > > > > > mstb->link_address_sent = true;
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > > @@ -2478,7 +2462,6 @@ drm_dp_send_enum_path_resources(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > {
> > > > > > > struct drm_dp_enum_path_resources_ack_reply *path_res;
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > - int len;
> > > > > > > int ret;
> > > > > > >
> > > > > > > txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> > > > > > > @@ -2486,7 +2469,7 @@ drm_dp_send_enum_path_resources(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > return -ENOMEM;
> > > > > > >
> > > > > > > txmsg->dst = mstb;
> > > > > > > - len = build_enum_path_resources(txmsg, port->port_num);
> > > > > > > + build_enum_path_resources(txmsg, port->port_num);
> > > > > > >
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > >
> > > > > > > @@ -2569,7 +2552,7 @@ static int drm_dp_payload_send_msg(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > > - int len, ret, port_num;
> > > > > > > + int ret, port_num;
> > > > > > > u8 sinks[DRM_DP_MAX_SDP_STREAMS];
> > > > > > > int i;
> > > > > > >
> > > > > > > @@ -2594,9 +2577,9 @@ static int drm_dp_payload_send_msg(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > sinks[i] = i;
> > > > > > >
> > > > > > > txmsg->dst = mstb;
> > > > > > > - len = build_allocate_payload(txmsg, port_num,
> > > > > > > - id,
> > > > > > > - pbn, port->num_sdp_streams,
> > > > > > > sinks);
> > > > > > > + build_allocate_payload(txmsg, port_num,
> > > > > > > + id,
> > > > > > > + pbn, port->num_sdp_streams, sinks);
> > > > > > >
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > >
> > > > > > > @@ -2625,7 +2608,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > struct drm_dp_mst_port *port,
> > > > > > > bool
> > > > > > > power_up)
> > > > > > > {
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > - int len, ret;
> > > > > > > + int ret;
> > > > > > >
> > > > > > > port = drm_dp_mst_topology_get_port_validated(mgr, port);
> > > > > > > if (!port)
> > > > > > > @@ -2638,7 +2621,7 @@ int drm_dp_send_power_updown_phy(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > }
> > > > > > >
> > > > > > > txmsg->dst = port->parent;
> > > > > > > - len = build_power_updown_phy(txmsg, port->port_num,
> > > > > > > power_up);
> > > > > > > + build_power_updown_phy(txmsg, port->port_num, power_up);
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > >
> > > > > > > ret = drm_dp_mst_wait_tx_reply(port->parent, txmsg);
> > > > > > > @@ -2858,7 +2841,6 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > struct drm_dp_mst_port *port,
> > > > > > > int offset, int size, u8 *bytes)
> > > > > > > {
> > > > > > > - int len;
> > > > > > > int ret = 0;
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > > @@ -2873,7 +2855,7 @@ static int drm_dp_send_dpcd_read(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > goto fail_put;
> > > > > > > }
> > > > > > >
> > > > > > > - len = build_dpcd_read(txmsg, port->port_num, offset,
> > > > > > > size);
> > > > > > > + build_dpcd_read(txmsg, port->port_num, offset, size);
> > > > > > > txmsg->dst = port->parent;
> > > > > > >
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > > @@ -2911,7 +2893,6 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > struct drm_dp_mst_port *port,
> > > > > > > int offset, int size, u8 *bytes)
> > > > > > > {
> > > > > > > - int len;
> > > > > > > int ret;
> > > > > > > struct drm_dp_sideband_msg_tx *txmsg;
> > > > > > > struct drm_dp_mst_branch *mstb;
> > > > > > > @@ -2926,7 +2907,7 @@ static int drm_dp_send_dpcd_write(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr,
> > > > > > > goto fail_put;
> > > > > > > }
> > > > > > >
> > > > > > > - len = build_dpcd_write(txmsg, port->port_num, offset,
> > > > > > > size,
> > > > > > > bytes);
> > > > > > > + build_dpcd_write(txmsg, port->port_num, offset, size,
> > > > > > > bytes);
> > > > > > > txmsg->dst = mstb;
> > > > > > >
> > > > > > > drm_dp_queue_down_tx(mgr, txmsg);
> > > > > > > @@ -3149,7 +3130,7 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > > {
> > > > > > > int len;
> > > > > > > u8 replyblock[32];
> > > > > > > - int replylen, origlen, curreply;
> > > > > > > + int replylen, curreply;
> > > > > > > int ret;
> > > > > > > struct drm_dp_sideband_msg_rx *msg;
> > > > > > > int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
> > > > > > > DP_SIDEBAND_MSG_DOWN_REP_BASE;
> > > > > > > @@ -3169,7 +3150,6 @@ static bool drm_dp_get_one_sb_msg(struct
> > > > > > > drm_dp_mst_topology_mgr *mgr, bool up)
> > > > > > > }
> > > > > > > replylen = msg->curchunk_len + msg->curchunk_hdrlen;
> > > > > > >
> > > > > > > - origlen = replylen;
> > > > > > > replylen -= len;
> > > > > > > curreply = len;
> > > > > > > while (replylen > 0) {
> > > > > > > @@ -3961,17 +3941,16 @@ void drm_dp_mst_dump_topology(struct
> > > > > > > seq_file *m,
> > > > > > > mutex_lock(&mgr->lock);
> > > > > > > if (mgr->mst_primary) {
> > > > > > > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > > > > > > - int ret;
> > > > > > >
> > > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > > + drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > > > > > > DP_RECEIVER_CAP_SIZE);
> > > > > > > seq_printf(m, "dpcd: %*ph\n",
> > > > > > > DP_RECEIVER_CAP_SIZE,
> > > > > > > buf);
> > > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf,
> > > > > > > 2);
> > > > > > > + drm_dp_dpcd_read(mgr->aux, DP_FAUX_CAP, buf, 2);
> > > > > > > seq_printf(m, "faux/mst: %*ph\n", 2, buf);
> > > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL,
> > > > > > > buf,
> > > > > > > 1);
> > > > > > > + drm_dp_dpcd_read(mgr->aux, DP_MSTM_CTRL, buf, 1);
> > > > > > > seq_printf(m, "mst ctrl: %*ph\n", 1, buf);
> > > > > > >
> > > > > > > /* dump the standard OUI branch header */
> > > > > > > - ret = drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI,
> > > > > > > buf,
> > > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > > + drm_dp_dpcd_read(mgr->aux, DP_BRANCH_OUI, buf,
> > > > > > > DP_BRANCH_OUI_HEADER_SIZE);
> > > > > > > seq_printf(m, "branch oui: %*phN devid: ", 3,
> > > > > > > buf);
> > > > > > > for (i = 0x3; i < 0x8 && buf[i]; i++)
> > > > > > > seq_printf(m, "%c", buf[i]);
> > > > > >
> > > > > > --
> > > > > > Jani Nikula, Intel Open Source Graphics Center
> > > > > > _______________________________________________
> > > > > > dri-devel mailing list
> > > > > > dri-devel@lists.freedesktop.org
> > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > --
> > Cheers,
> > Lyude Paul
> >
--
Cheers,
Lyude Paul
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-30 22:22 ` Lyude Paul
@ 2020-01-30 23:22 ` Dave Airlie
2020-01-31 8:08 ` Benjamin GAIGNARD
0 siblings, 1 reply; 14+ messages in thread
From: Dave Airlie @ 2020-01-30 23:22 UTC (permalink / raw)
To: Lyude Paul
Cc: Benjamin Gaignard, Benjamin Gaignard, David Airlie, ML dri-devel,
Linux Kernel Mailing List, Sean Paul
> > > >
> > > hi-actually yes, we should probably be using this instead of just dropping
> > > this. Also, I didn't write this code originally I just refactored a bunch
> > > of
> > > it - Dave Airlied is the original author, but the original version of this
> > > code was written ages ago. tbh, I think it's a safe bet to say that they
> > > probably did mean to use this but forgot to and no one noticed until now.
> >
> > Hi,
> >
> > Any clue about how to use crc value ? Does it have to be checked
> > against something else ?
> > If crc are not matching what should we do of the data copied just before ?
>
> We should be able to just take the CRC value from the sideband message and
> then generate our own CRC value using the sideband message contents, and check
> if the two are equal. If they aren't, something went wrong and we didn't
> receive the message properly.
>
> Now as to what we should do when we have CRC mismatches? That's a bit more
> difficult. If you have access to the DP MST spec, I suppose a place to start
> figuring that out would be checking if there's a way for us to request that a
> branch device resend whatever message it sent previously. If there isn't, I
> guess we should just print an error in dmesg (possibly with a hexdump of the
> failed message as well) and not forward the message to the driver. Not sure of
> any better way of handling it then that
Yeah I think this reflects what I wanted to do, I've no memory of a
retransmit option in the spec, but I've away from it for a while. But
we'd want to compare the CRC with what we got to make sure the are the
same.
Dave.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-30 23:22 ` Dave Airlie
@ 2020-01-31 8:08 ` Benjamin GAIGNARD
2020-02-03 9:40 ` Daniel Vetter
0 siblings, 1 reply; 14+ messages in thread
From: Benjamin GAIGNARD @ 2020-01-31 8:08 UTC (permalink / raw)
To: Dave Airlie, Lyude Paul
Cc: Benjamin Gaignard, David Airlie, ML dri-devel,
Linux Kernel Mailing List, Sean Paul
On 1/31/20 12:22 AM, Dave Airlie wrote:
>>>> hi-actually yes, we should probably be using this instead of just dropping
>>>> this. Also, I didn't write this code originally I just refactored a bunch
>>>> of
>>>> it - Dave Airlied is the original author, but the original version of this
>>>> code was written ages ago. tbh, I think it's a safe bet to say that they
>>>> probably did mean to use this but forgot to and no one noticed until now.
>>> Hi,
>>>
>>> Any clue about how to use crc value ? Does it have to be checked
>>> against something else ?
>>> If crc are not matching what should we do of the data copied just before ?
>> We should be able to just take the CRC value from the sideband message and
>> then generate our own CRC value using the sideband message contents, and check
>> if the two are equal. If they aren't, something went wrong and we didn't
>> receive the message properly.
>>
>> Now as to what we should do when we have CRC mismatches? That's a bit more
>> difficult. If you have access to the DP MST spec, I suppose a place to start
>> figuring that out would be checking if there's a way for us to request that a
>> branch device resend whatever message it sent previously. If there isn't, I
>> guess we should just print an error in dmesg (possibly with a hexdump of the
>> failed message as well) and not forward the message to the driver. Not sure of
>> any better way of handling it then that
> Yeah I think this reflects what I wanted to do, I've no memory of a
> retransmit option in the spec, but I've away from it for a while. But
> we'd want to compare the CRC with what we got to make sure the are the
> same.
Hmm, that far more complex than just fix compilation warnings :)
I will split the patch in two:
- one for of all other warnings, hopefully it can get reviewed
- one for this crc4 variable. Does checking crc value and print an error
should be acceptable ?
Something like:
if (crc4 != msg->chunk[msg->curchunk_len - 1])
print_hex_dump(KERN_DEBUG, "wrong crc", DUMP_PREFIX_NONE, 16, 1,
msg->chunk, msg->curchunk_len, false);
Benjamin
>
> Dave.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] drm/dp_mst: Fix W=1 warnings
2020-01-31 8:08 ` Benjamin GAIGNARD
@ 2020-02-03 9:40 ` Daniel Vetter
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2020-02-03 9:40 UTC (permalink / raw)
To: Benjamin GAIGNARD
Cc: Dave Airlie, Lyude Paul, David Airlie, Sean Paul, ML dri-devel,
Linux Kernel Mailing List
On Fri, Jan 31, 2020 at 08:08:34AM +0000, Benjamin GAIGNARD wrote:
>
> On 1/31/20 12:22 AM, Dave Airlie wrote:
> >>>> hi-actually yes, we should probably be using this instead of just dropping
> >>>> this. Also, I didn't write this code originally I just refactored a bunch
> >>>> of
> >>>> it - Dave Airlied is the original author, but the original version of this
> >>>> code was written ages ago. tbh, I think it's a safe bet to say that they
> >>>> probably did mean to use this but forgot to and no one noticed until now.
> >>> Hi,
> >>>
> >>> Any clue about how to use crc value ? Does it have to be checked
> >>> against something else ?
> >>> If crc are not matching what should we do of the data copied just before ?
> >> We should be able to just take the CRC value from the sideband message and
> >> then generate our own CRC value using the sideband message contents, and check
> >> if the two are equal. If they aren't, something went wrong and we didn't
> >> receive the message properly.
> >>
> >> Now as to what we should do when we have CRC mismatches? That's a bit more
> >> difficult. If you have access to the DP MST spec, I suppose a place to start
> >> figuring that out would be checking if there's a way for us to request that a
> >> branch device resend whatever message it sent previously. If there isn't, I
> >> guess we should just print an error in dmesg (possibly with a hexdump of the
> >> failed message as well) and not forward the message to the driver. Not sure of
> >> any better way of handling it then that
> > Yeah I think this reflects what I wanted to do, I've no memory of a
> > retransmit option in the spec, but I've away from it for a while. But
> > we'd want to compare the CRC with what we got to make sure the are the
> > same.
>
> Hmm, that far more complex than just fix compilation warnings :)
>
> I will split the patch in two:
>
> - one for of all other warnings, hopefully it can get reviewed
>
> - one for this crc4 variable. Does checking crc value and print an error
> should be acceptable ?
>
> Something like:
>
> if (crc4 != msg->chunk[msg->curchunk_len - 1])
>
> print_hex_dump(KERN_DEBUG, "wrong crc", DUMP_PREFIX_NONE, 16, 1,
> msg->chunk, msg->curchunk_len, false);
Yeah I think that should be reasonable as a start. Then we'll see how much
the bug reports start flowing in ...
-Daniel
>
>
> Benjamin
>
>
> >
> > Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-03 9:40 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 13:50 [PATCH v3] drm/dp_mst: Fix W=1 warnings Benjamin Gaignard
2019-12-03 16:58 ` Benjamin Gaignard
2019-12-04 16:47 ` Jani Nikula
2019-12-16 8:28 ` Benjamin Gaignard
2019-12-20 14:03 ` Benjamin Gaignard
2020-01-07 13:11 ` Benjamin Gaignard
2020-01-18 0:55 ` Lyude Paul
2020-01-18 8:31 ` Benjamin Gaignard
2020-01-24 22:08 ` Lyude Paul
2020-01-27 13:08 ` Benjamin Gaignard
2020-01-30 22:22 ` Lyude Paul
2020-01-30 23:22 ` Dave Airlie
2020-01-31 8:08 ` Benjamin GAIGNARD
2020-02-03 9:40 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).