| Message ID | 20230216031515.4151117-1-xu.yang_2@nxp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: typec: tcpm: fix warning when handle discover_identity message | expand |
On 2/15/23 19:15, Xu Yang wrote: > Since both source and sink device can send discover_identity message in > PD3, kernel may dump below warning: > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 169 at drivers/usb/typec/tcpm/tcpm.c:1446 tcpm_queue_vdm+0xe0/0xf0 > Modules linked in: > CPU: 0 PID: 169 Comm: 1-0050 Not tainted 6.1.1-00038-g6a3c36cf1da2-dirty #567 > Hardware name: NXP i.MX8MPlus EVK board (DT) > pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : tcpm_queue_vdm+0xe0/0xf0 > lr : tcpm_queue_vdm+0x2c/0xf0 > sp : ffff80000c19bcd0 > x29: ffff80000c19bcd0 x28: 0000000000000001 x27: ffff0000d11c8ab8 > x26: ffff0000d11cc000 x25: 0000000000000000 x24: 00000000ff008081 > x23: 0000000000000001 x22: 00000000ff00a081 x21: ffff80000c19bdbc > x20: 0000000000000000 x19: ffff0000d11c8080 x18: ffffffffffffffff > x17: 0000000000000000 x16: 0000000000000000 x15: ffff0000d716f580 > x14: 0000000000000001 x13: ffff0000d716f507 x12: 0000000000000001 > x11: 0000000000000000 x10: 0000000000000020 x9 : 00000000000ee098 > x8 : 00000000ffffffff x7 : 000000000000001c x6 : ffff0000d716f580 > x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000 > x2 : ffff80000c19bdbc x1 : 00000000ff00a081 x0 : 0000000000000004 > Call trace: > tcpm_queue_vdm+0xe0/0xf0 > tcpm_pd_rx_handler+0x340/0x1ab0 > kthread_worker_fn+0xcc/0x18c > kthread+0x10c/0x110 > ret_from_fork+0x10/0x20 > ---[ end trace 0000000000000000 ]--- > > Below sequences may trigger this warning: > > tcpm_send_discover_work(work) > tcpm_send_vdm(port, USB_SID_PD, CMD_DISCOVER_IDENT, NULL, 0); > tcpm_queue_vdm(port, header, data, count); > port->vdm_state = VDM_STATE_READY; > > vdm_state_machine_work(work); > <-- received discover_identity from partner > vdm_run_state_machine(port); > port->vdm_state = VDM_STATE_SEND_MESSAGE; > mod_vdm_delayed_work(port, x); > > tcpm_pd_rx_handler(work); > tcpm_pd_data_request(port, msg); > tcpm_handle_vdm_request(port, msg->payload, cnt); > tcpm_queue_vdm(port, response[0], &response[1], rlen - 1); > --> WARN_ON(port->vdm_state > VDM_STATE_DONE); > > For this case, the state machine could still send out discover > identity message later if we skip current discover_identity message. > So we should handle the received message firstly and override the pending > discover_identity message without warning in this case. Then, a delayed > send_discover work will send discover_identity message again. > > Fixes: e00943e91678 ("usb: typec: tcpm: PD3.0 sinks can send Discover Identity even in device mode") > cc: <stable@vger.kernel.org> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > --- > Changelogs: > v2: modify some code format and commit message > --- > drivers/usb/typec/tcpm/tcpm.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index 7f39cb9b3429..1ee774c263f0 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -1445,10 +1445,18 @@ static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams) > static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, > const u32 *data, int cnt) > { > + u32 vdo_hdr = port->vdo_data[0]; > + > WARN_ON(!mutex_is_locked(&port->lock)); > > - /* Make sure we are not still processing a previous VDM packet */ > - WARN_ON(port->vdm_state > VDM_STATE_DONE); > + /* If is sending discover_identity, handle received message first */ > + if (PD_VDO_SVDM(vdo_hdr) && PD_VDO_CMD(vdo_hdr) == CMD_DISCOVER_IDENT) { > + port->send_discover = true; > + mod_send_discover_delayed_work(port, SEND_DISCOVER_RETRY_MS); > + } else { > + /* Make sure we are not still processing a previous VDM packet */ > + WARN_ON(port->vdm_state > VDM_STATE_DONE); > + } > > port->vdo_count = cnt + 1; > port->vdo_data[0] = header; > @@ -1948,11 +1956,13 @@ static void vdm_run_state_machine(struct tcpm_port *port) > switch (PD_VDO_CMD(vdo_hdr)) { > case CMD_DISCOVER_IDENT: > res = tcpm_ams_start(port, DISCOVER_IDENTITY); > - if (res == 0) > + if (res == 0) { > port->send_discover = false; > - else if (res == -EAGAIN) > + } else if (res == -EAGAIN) { > + port->vdo_data[0] = 0; > mod_send_discover_delayed_work(port, > SEND_DISCOVER_RETRY_MS); > + } > break; > case CMD_DISCOVER_SVID: > res = tcpm_ams_start(port, DISCOVER_SVIDS); > @@ -2035,6 +2045,7 @@ static void vdm_run_state_machine(struct tcpm_port *port) > unsigned long timeout; > > port->vdm_retries = 0; > + port->vdo_data[0] = 0; > port->vdm_state = VDM_STATE_BUSY; > timeout = vdm_ready_timeout(vdo_hdr); > mod_vdm_delayed_work(port, timeout);
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 7f39cb9b3429..1ee774c263f0 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -1445,10 +1445,18 @@ static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams) static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, const u32 *data, int cnt) { + u32 vdo_hdr = port->vdo_data[0]; + WARN_ON(!mutex_is_locked(&port->lock)); - /* Make sure we are not still processing a previous VDM packet */ - WARN_ON(port->vdm_state > VDM_STATE_DONE); + /* If is sending discover_identity, handle received message first */ + if (PD_VDO_SVDM(vdo_hdr) && PD_VDO_CMD(vdo_hdr) == CMD_DISCOVER_IDENT) { + port->send_discover = true; + mod_send_discover_delayed_work(port, SEND_DISCOVER_RETRY_MS); + } else { + /* Make sure we are not still processing a previous VDM packet */ + WARN_ON(port->vdm_state > VDM_STATE_DONE); + } port->vdo_count = cnt + 1; port->vdo_data[0] = header; @@ -1948,11 +1956,13 @@ static void vdm_run_state_machine(struct tcpm_port *port) switch (PD_VDO_CMD(vdo_hdr)) { case CMD_DISCOVER_IDENT: res = tcpm_ams_start(port, DISCOVER_IDENTITY); - if (res == 0) + if (res == 0) { port->send_discover = false; - else if (res == -EAGAIN) + } else if (res == -EAGAIN) { + port->vdo_data[0] = 0; mod_send_discover_delayed_work(port, SEND_DISCOVER_RETRY_MS); + } break; case CMD_DISCOVER_SVID: res = tcpm_ams_start(port, DISCOVER_SVIDS); @@ -2035,6 +2045,7 @@ static void vdm_run_state_machine(struct tcpm_port *port) unsigned long timeout; port->vdm_retries = 0; + port->vdo_data[0] = 0; port->vdm_state = VDM_STATE_BUSY; timeout = vdm_ready_timeout(vdo_hdr); mod_vdm_delayed_work(port, timeout);
Since both source and sink device can send discover_identity message in PD3, kernel may dump below warning: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 169 at drivers/usb/typec/tcpm/tcpm.c:1446 tcpm_queue_vdm+0xe0/0xf0 Modules linked in: CPU: 0 PID: 169 Comm: 1-0050 Not tainted 6.1.1-00038-g6a3c36cf1da2-dirty #567 Hardware name: NXP i.MX8MPlus EVK board (DT) pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : tcpm_queue_vdm+0xe0/0xf0 lr : tcpm_queue_vdm+0x2c/0xf0 sp : ffff80000c19bcd0 x29: ffff80000c19bcd0 x28: 0000000000000001 x27: ffff0000d11c8ab8 x26: ffff0000d11cc000 x25: 0000000000000000 x24: 00000000ff008081 x23: 0000000000000001 x22: 00000000ff00a081 x21: ffff80000c19bdbc x20: 0000000000000000 x19: ffff0000d11c8080 x18: ffffffffffffffff x17: 0000000000000000 x16: 0000000000000000 x15: ffff0000d716f580 x14: 0000000000000001 x13: ffff0000d716f507 x12: 0000000000000001 x11: 0000000000000000 x10: 0000000000000020 x9 : 00000000000ee098 x8 : 00000000ffffffff x7 : 000000000000001c x6 : ffff0000d716f580 x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000 x2 : ffff80000c19bdbc x1 : 00000000ff00a081 x0 : 0000000000000004 Call trace: tcpm_queue_vdm+0xe0/0xf0 tcpm_pd_rx_handler+0x340/0x1ab0 kthread_worker_fn+0xcc/0x18c kthread+0x10c/0x110 ret_from_fork+0x10/0x20 ---[ end trace 0000000000000000 ]--- Below sequences may trigger this warning: tcpm_send_discover_work(work) tcpm_send_vdm(port, USB_SID_PD, CMD_DISCOVER_IDENT, NULL, 0); tcpm_queue_vdm(port, header, data, count); port->vdm_state = VDM_STATE_READY; vdm_state_machine_work(work); <-- received discover_identity from partner vdm_run_state_machine(port); port->vdm_state = VDM_STATE_SEND_MESSAGE; mod_vdm_delayed_work(port, x); tcpm_pd_rx_handler(work); tcpm_pd_data_request(port, msg); tcpm_handle_vdm_request(port, msg->payload, cnt); tcpm_queue_vdm(port, response[0], &response[1], rlen - 1); --> WARN_ON(port->vdm_state > VDM_STATE_DONE); For this case, the state machine could still send out discover identity message later if we skip current discover_identity message. So we should handle the received message firstly and override the pending discover_identity message without warning in this case. Then, a delayed send_discover work will send discover_identity message again. Fixes: e00943e91678 ("usb: typec: tcpm: PD3.0 sinks can send Discover Identity even in device mode") cc: <stable@vger.kernel.org> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- Changelogs: v2: modify some code format and commit message --- drivers/usb/typec/tcpm/tcpm.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)