Hi, On 7/25/20 4:45 PM, Guenter Roeck wrote:
On 7/24/20 10:47 AM, Hans de Goede wrote:Refactor tcpm_handle_vdm_request and its tcpm_pd_svdm helper function so that reporting the results of the vdm to the altmode-driver is separated out into a clear separate step inside tcpm_handle_vdm_request, instead of being scattered over various places inside the tcpm_pd_svdm helper. This is a preparation patch for fixing an AB BA lock inversion between the tcpm code and some altmode drivers. Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> --- Changes in v2: - Keep "if (adev && pdev)" checks as is instead of modifying them --- drivers/usb/typec/tcpm/tcpm.c | 76 ++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index ee239b54bcd8..03a0c083ee9a 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -159,6 +159,14 @@ enum pd_msg_request { PD_MSG_DATA_SOURCE_CAP, };+enum adev_actions {+ ADEV_NONE = 0, + ADEV_NOTIFY_USB_AND_QUEUE_VDM, + ADEV_QUEUE_VDM, + ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL, + ADEV_ATTENTION, +}; + /* Events from low level driver */#define TCPM_CC_EVENT BIT(0)@@ -1078,10 +1086,10 @@ static void tcpm_register_partner_altmodes(struct tcpm_port *port)#define supports_modal(port) PD_IDH_MODAL_SUPP((port)->partner_ident.id_header) -static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt,- u32 *response) +static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev, + const u32 *p, int cnt, u32 *response, + enum adev_actions *adev_action) { - struct typec_altmode *adev; struct typec_altmode *pdev; struct pd_mode_data *modep; int rlen = 0; @@ -1097,9 +1105,6 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt,modep = &port->mode_data; - adev = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,- PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0])); - pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX, PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));@@ -1125,8 +1130,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt,break; case CMD_ATTENTION: /* Attention command does not have response */ - if (adev) - typec_altmode_attention(adev, p[1]); + *adev_action = ADEV_ATTENTION; return 0; default: break; @@ -1180,23 +1184,15 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt, case CMD_ENTER_MODE: if (adev && pdev) {I must be missing something. Does this compile ? The 'adev' variable was removed above. Maybe move the call to typec_altmode_update_active() into tcpm_handle_vdm_request() instead ?
Yes it compiles. The adev variable is now a function parameter, since we already to the lookup for it in tcpm_handle_vdm_request() now, I'm simply passing it along here. Regards, Hans
typec_altmode_update_active(pdev, true); - - if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) { - response[0] = VDO(adev->svid, 1, - CMD_EXIT_MODE); - response[0] |= VDO_OPOS(adev->mode); - return 1; - } + *adev_action = ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL; } return 0; case CMD_EXIT_MODE: if (adev && pdev) { typec_altmode_update_active(pdev, false); - /* Back to USB Operation */ - WARN_ON(typec_altmode_notify(adev, - TYPEC_STATE_USB, - NULL)); + *adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM; + return 0; } break; default: @@ -1207,11 +1203,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt, switch (cmd) { case CMD_ENTER_MODE: /* Back to USB Operation */ - if (adev) - WARN_ON(typec_altmode_notify(adev, - TYPEC_STATE_USB, - NULL)); - break; + *adev_action = ADEV_NOTIFY_USB_AND_QUEUE_VDM; + return 0; default: break; } @@ -1221,15 +1214,15 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const u32 *p, int cnt, }/* Informing the alternate mode drivers about everything */- if (adev) - typec_altmode_vdm(adev, p[0], &p[1], cnt); - + *adev_action = ADEV_QUEUE_VDM; return rlen; }static void tcpm_handle_vdm_request(struct tcpm_port *port,const __le32 *payload, int cnt) { + enum adev_actions adev_action = ADEV_NONE; + struct typec_altmode *adev; u32 p[PD_MAX_PAYLOAD]; u32 response[8] = { }; int i, rlen = 0; @@ -1237,6 +1230,9 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port, for (i = 0; i < cnt; i++) p[i] = le32_to_cpu(payload[i]);+ adev = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,+ PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0])); + if (port->vdm_state == VDM_STATE_BUSY) { /* If UFP responded busy retry after timeout */ if (PD_VDO_CMDT(p[0]) == CMDT_RSP_BUSY) { @@ -1251,7 +1247,31 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port, }if (PD_VDO_SVDM(p[0]))- rlen = tcpm_pd_svdm(port, p, cnt, response); + rlen = tcpm_pd_svdm(port, adev, p, cnt, response, &adev_action); + + if (adev) { + switch (adev_action) { + case ADEV_NONE: + break; + case ADEV_NOTIFY_USB_AND_QUEUE_VDM: + WARN_ON(typec_altmode_notify(adev, TYPEC_STATE_USB, NULL)); + typec_altmode_vdm(adev, p[0], &p[1], cnt); + break; + case ADEV_QUEUE_VDM: + typec_altmode_vdm(adev, p[0], &p[1], cnt); + break; + case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL: + if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) { + response[0] = VDO(adev->svid, 1, CMD_EXIT_MODE); + response[0] |= VDO_OPOS(adev->mode); + rlen = 1; + } + break; + case ADEV_ATTENTION: + typec_altmode_attention(adev, p[1]); + break; + } + }if (rlen > 0)tcpm_queue_vdm(port, response[0], &response[1], rlen - 1);
