linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC,v2,3/3] usb: typec: tcpm: Support for Alternate Modes
@ 2018-03-09 15:19 Heikki Krogerus
  0 siblings, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2018-03-09 15:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Guenter Roeck, Hans de Goede
  Cc: Jun Li, Regupathy, Rajaram, linux-usb, linux-kernel

This adds more complete handling of VDMs and registration of
partner alternate modes, and introduces callbacks for
alternate mode operations.

Only DFP role is supported for now.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/tcpm.c | 133 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 111 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index bfb843ebffa6..34bf5c1f4d81 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -158,13 +158,14 @@ enum pd_msg_request {
 /* Alternate mode support */
 
 #define SVID_DISCOVERY_MAX	16
+#define ALTMODE_DISCOVERY_MAX	(SVID_DISCOVERY_MAX * MODE_DISCOVERY_MAX)
 
 struct pd_mode_data {
 	int svid_index;		/* current SVID index		*/
 	int nsvids;
 	u16 svids[SVID_DISCOVERY_MAX];
 	int altmodes;		/* number of alternate modes	*/
-	struct typec_altmode_desc altmode_desc[SVID_DISCOVERY_MAX];
+	struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
 };
 
 struct tcpm_port {
@@ -280,8 +281,8 @@ struct tcpm_port {
 	/* Alternate mode data */
 
 	struct pd_mode_data mode_data;
-	struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX * 6];
-	struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX * 6];
+	struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
+	struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
 
 	/* Deadline in jiffies to exit src_try_wait state */
 	unsigned long max_wait;
@@ -985,36 +986,53 @@ static void svdm_consume_modes(struct tcpm_port *port, const __le32 *payload,
 			 pmdata->altmodes, paltmode->svid,
 			 paltmode->mode, paltmode->vdo);
 
-		port->partner_altmode[pmdata->altmodes] =
-			typec_partner_register_altmode(port->partner, paltmode);
-		if (!port->partner_altmode[pmdata->altmodes]) {
-			tcpm_log(port,
-				 "Failed to register modes for SVID 0x%04x",
-				 paltmode->svid);
-			return;
-		}
 		pmdata->altmodes++;
 	}
 }
 
+static void tcpm_register_partner_altmodes(struct tcpm_port *port)
+{
+	struct pd_mode_data *modep = &port->mode_data;
+	struct typec_altmode *altmode;
+	int i;
+
+	for (i = 0; i < modep->altmodes; i++) {
+		altmode = typec_partner_register_altmode(port->partner,
+						&modep->altmode_desc[i]);
+		if (!altmode)
+			tcpm_log(port, "Failed to register partner SVID 0x%04x",
+				 modep->altmode_desc[i].svid);
+		port->partner_altmode[i] = altmode;
+	}
+}
+
 #define supports_modal(port)	PD_IDH_MODAL_SUPP((port)->partner_ident.id_header)
 
 static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 			u32 *response)
 {
-	u32 p0 = le32_to_cpu(payload[0]);
-	int cmd_type = PD_VDO_CMDT(p0);
-	int cmd = PD_VDO_CMD(p0);
+	struct typec_altmode *altmode;
 	struct pd_mode_data *modep;
+	u32 p[PD_MAX_PAYLOAD];
 	int rlen = 0;
-	u16 svid;
+	int cmd_type;
+	int cmd;
 	int i;
 
+	for (i = 0; i < cnt; i++)
+		p[i] = le32_to_cpu(payload[i]);
+
+	cmd_type = PD_VDO_CMDT(p[0]);
+	cmd = PD_VDO_CMD(p[0]);
+
 	tcpm_log(port, "Rx VDM cmd 0x%x type %d cmd %d len %d",
-		 p0, cmd_type, cmd, cnt);
+		 p[0], cmd_type, cmd, cnt);
 
 	modep = &port->mode_data;
 
+	altmode = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,
+				      PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
+
 	switch (cmd_type) {
 	case CMDT_INIT:
 		switch (cmd) {
@@ -1036,17 +1054,21 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 		case CMD_EXIT_MODE:
 			break;
 		case CMD_ATTENTION:
-			break;
+			typec_altmode_attention(altmode, p[1]);
+			return 0;
 		default:
+			if (typec_altmode_vdm(altmode, p[0], &p[1], cnt) ==
+			    VDM_OK)
+				return 0;
 			break;
 		}
 		if (rlen >= 1) {
-			response[0] = p0 | VDO_CMDT(CMDT_RSP_ACK);
+			response[0] = p[0] | VDO_CMDT(CMDT_RSP_ACK);
 		} else if (rlen == 0) {
-			response[0] = p0 | VDO_CMDT(CMDT_RSP_NAK);
+			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
 			rlen = 1;
 		} else {
-			response[0] = p0 | VDO_CMDT(CMDT_RSP_BUSY);
+			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
 			rlen = 1;
 		}
 		break;
@@ -1079,20 +1101,26 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 			svdm_consume_modes(port, payload, cnt);
 			modep->svid_index++;
 			if (modep->svid_index < modep->nsvids) {
-				svid = modep->svids[modep->svid_index];
+				u16 svid = modep->svids[modep->svid_index];
 				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
 				rlen = 1;
 			} else {
-				/* enter alternate mode if/when implemented */
+				tcpm_register_partner_altmodes(port);
 			}
 			break;
 		case CMD_ENTER_MODE:
+			typec_altmode_enter(altmode);
+			break;
+		case CMD_EXIT_MODE:
+			typec_altmode_exit(altmode);
 			break;
 		default:
+			typec_altmode_vdm(altmode, p[0], &p[1], cnt);
 			break;
 		}
 		break;
 	default:
+		typec_altmode_vdm(altmode, p[0], &p[1], cnt);
 		break;
 	}
 
@@ -1352,6 +1380,47 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
 	return 0;
 }
 
+static void tcpm_altmode_enter(struct typec_altmode *altmode)
+{
+	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
+	u32 header;
+
+	header = VDO(altmode->svid, 1, CMD_ENTER_MODE);
+	header |= VDO_OPOS(altmode->mode);
+
+	tcpm_queue_vdm(port, header, NULL, 0);
+	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
+}
+
+static void tcpm_altmode_exit(struct typec_altmode *altmode)
+{
+	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
+	u32 header;
+
+	header = VDO(altmode->svid, 1, CMD_EXIT_MODE);
+	header |= VDO_OPOS(altmode->mode);
+
+	tcpm_queue_vdm(port, header, NULL, 0);
+	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
+}
+
+static int tcpm_altmode_vdm(struct typec_altmode *altmode,
+			    u32 header, const u32 *data, int count)
+{
+	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
+
+	tcpm_queue_vdm(port, header, data, count - 1);
+	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
+
+	return 0;
+}
+
+static const struct typec_altmode_ops tcpm_altmode_ops = {
+	.enter = tcpm_altmode_enter,
+	.exit = tcpm_altmode_exit,
+	.vdm = tcpm_altmode_vdm,
+};
+
 /*
  * PD (data, control) command handling functions
  */
@@ -3479,6 +3548,23 @@ static void tcpm_init(struct tcpm_port *port)
 	tcpm_set_state(port, PORT_RESET, 0);
 }
 
+static int tcpm_activate_mode(struct typec_altmode *alt, int activate)
+{
+	struct tcpm_port *port = typec_altmode_get_drvdata(alt);
+	u32 header;
+
+	header = VDO(cpu_to_le16(alt->svid), 1,
+		     activate ? CMD_ENTER_MODE : CMD_EXIT_MODE);
+	header |= VDO_OPOS(alt->mode);
+
+	mutex_lock(&port->lock);
+	tcpm_queue_vdm(port, header, NULL, 0);
+	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
+	mutex_unlock(&port->lock);
+
+	return 0;
+}
+
 static int tcpm_port_type_set(const struct typec_capability *cap,
 			      enum typec_port_type type)
 {
@@ -3671,6 +3757,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->typec_caps.pr_set = tcpm_pr_set;
 	port->typec_caps.vconn_set = tcpm_vconn_set;
 	port->typec_caps.try_role = tcpm_try_role;
+	port->typec_caps.activate_mode = tcpm_activate_mode;
 	port->typec_caps.port_type_set = tcpm_port_type_set;
 
 	port->partner_desc.identity = &port->partner_ident;
@@ -3703,6 +3790,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 					 dev_name(dev), paltmode->svid);
 				break;
 			}
+			typec_altmode_set_drvdata(alt, port);
+			typec_altmode_register_ops(alt, &tcpm_altmode_ops);
 			port->port_altmode[i] = alt;
 			i++;
 			paltmode++;

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [RFC,v2,3/3] usb: typec: tcpm: Support for Alternate Modes
@ 2018-03-16 21:32 Guenter Roeck
  0 siblings, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2018-03-16 21:32 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Hans de Goede, Jun Li, Regupathy, Rajaram,
	linux-usb, linux-kernel

On Fri, Mar 09, 2018 at 06:19:18PM +0300, Heikki Krogerus wrote:
> This adds more complete handling of VDMs and registration of
> partner alternate modes, and introduces callbacks for
> alternate mode operations.
> 
> Only DFP role is supported for now.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/tcpm.c | 133 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 111 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index bfb843ebffa6..34bf5c1f4d81 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -158,13 +158,14 @@ enum pd_msg_request {
>  /* Alternate mode support */
>  
>  #define SVID_DISCOVERY_MAX	16
> +#define ALTMODE_DISCOVERY_MAX	(SVID_DISCOVERY_MAX * MODE_DISCOVERY_MAX)
>  
>  struct pd_mode_data {
>  	int svid_index;		/* current SVID index		*/
>  	int nsvids;
>  	u16 svids[SVID_DISCOVERY_MAX];
>  	int altmodes;		/* number of alternate modes	*/
> -	struct typec_altmode_desc altmode_desc[SVID_DISCOVERY_MAX];
> +	struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
>  };
>  
>  struct tcpm_port {
> @@ -280,8 +281,8 @@ struct tcpm_port {
>  	/* Alternate mode data */
>  
>  	struct pd_mode_data mode_data;
> -	struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX * 6];
> -	struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX * 6];
> +	struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
> +	struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
>  
>  	/* Deadline in jiffies to exit src_try_wait state */
>  	unsigned long max_wait;
> @@ -985,36 +986,53 @@ static void svdm_consume_modes(struct tcpm_port *port, const __le32 *payload,
>  			 pmdata->altmodes, paltmode->svid,
>  			 paltmode->mode, paltmode->vdo);
>  
> -		port->partner_altmode[pmdata->altmodes] =
> -			typec_partner_register_altmode(port->partner, paltmode);
> -		if (!port->partner_altmode[pmdata->altmodes]) {
> -			tcpm_log(port,
> -				 "Failed to register modes for SVID 0x%04x",
> -				 paltmode->svid);
> -			return;
> -		}
>  		pmdata->altmodes++;
>  	}
>  }
>  
> +static void tcpm_register_partner_altmodes(struct tcpm_port *port)
> +{
> +	struct pd_mode_data *modep = &port->mode_data;
> +	struct typec_altmode *altmode;
> +	int i;
> +
> +	for (i = 0; i < modep->altmodes; i++) {
> +		altmode = typec_partner_register_altmode(port->partner,
> +						&modep->altmode_desc[i]);
> +		if (!altmode)
> +			tcpm_log(port, "Failed to register partner SVID 0x%04x",
> +				 modep->altmode_desc[i].svid);
> +		port->partner_altmode[i] = altmode;
> +	}
> +}
> +
>  #define supports_modal(port)	PD_IDH_MODAL_SUPP((port)->partner_ident.id_header)
>  
>  static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
>  			u32 *response)
>  {
> -	u32 p0 = le32_to_cpu(payload[0]);
> -	int cmd_type = PD_VDO_CMDT(p0);
> -	int cmd = PD_VDO_CMD(p0);
> +	struct typec_altmode *altmode;
>  	struct pd_mode_data *modep;
> +	u32 p[PD_MAX_PAYLOAD];
>  	int rlen = 0;
> -	u16 svid;
> +	int cmd_type;
> +	int cmd;
>  	int i;
>  
> +	for (i = 0; i < cnt; i++)
> +		p[i] = le32_to_cpu(payload[i]);
> +
> +	cmd_type = PD_VDO_CMDT(p[0]);
> +	cmd = PD_VDO_CMD(p[0]);
> +
>  	tcpm_log(port, "Rx VDM cmd 0x%x type %d cmd %d len %d",
> -		 p0, cmd_type, cmd, cnt);
> +		 p[0], cmd_type, cmd, cnt);
>  
>  	modep = &port->mode_data;
>  
> +	altmode = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,
> +				      PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));

This can return NULL ...

> +
>  	switch (cmd_type) {
>  	case CMDT_INIT:
>  		switch (cmd) {
> @@ -1036,17 +1054,21 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
>  		case CMD_EXIT_MODE:
>  			break;
>  		case CMD_ATTENTION:
> -			break;
> +			typec_altmode_attention(altmode, p[1]);

... and NULL is not handled by typec_altmode_attention().

> +			return 0;
>  		default:
> +			if (typec_altmode_vdm(altmode, p[0], &p[1], cnt) ==
> +			    VDM_OK)

typec_altmode_vdm() returns old fashioned error messages.
Not sure what it is supposed to return, though.

> +				return 0;
>  			break;
>  		}
>  		if (rlen >= 1) {
> -			response[0] = p0 | VDO_CMDT(CMDT_RSP_ACK);
> +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_ACK);
>  		} else if (rlen == 0) {
> -			response[0] = p0 | VDO_CMDT(CMDT_RSP_NAK);
> +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  			rlen = 1;
>  		} else {
> -			response[0] = p0 | VDO_CMDT(CMDT_RSP_BUSY);
> +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
>  			rlen = 1;
>  		}
>  		break;
> @@ -1079,20 +1101,26 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
>  			svdm_consume_modes(port, payload, cnt);
>  			modep->svid_index++;
>  			if (modep->svid_index < modep->nsvids) {
> -				svid = modep->svids[modep->svid_index];
> +				u16 svid = modep->svids[modep->svid_index];
>  				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			} else {
> -				/* enter alternate mode if/when implemented */
> +				tcpm_register_partner_altmodes(port);
>  			}
>  			break;
>  		case CMD_ENTER_MODE:
> +			typec_altmode_enter(altmode);

typec_altmode_enter() don't handle altmode == NULL.
No error handling ?

> +			break;
> +		case CMD_EXIT_MODE:
> +			typec_altmode_exit(altmode);

Doesn't handle NULL. Error handling ?

>  			break;
>  		default:
> +			typec_altmode_vdm(altmode, p[0], &p[1], cnt);
>  			break;
>  		}
>  		break;
>  	default:
> +		typec_altmode_vdm(altmode, p[0], &p[1], cnt);
>  		break;
>  	}
>  
> @@ -1352,6 +1380,47 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
>  	return 0;
>  }
>  
> +static void tcpm_altmode_enter(struct typec_altmode *altmode)
> +{
> +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	u32 header;
> +
> +	header = VDO(altmode->svid, 1, CMD_ENTER_MODE);
> +	header |= VDO_OPOS(altmode->mode);
> +
> +	tcpm_queue_vdm(port, header, NULL, 0);
> +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> +}
> +
> +static void tcpm_altmode_exit(struct typec_altmode *altmode)
> +{
> +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	u32 header;
> +
> +	header = VDO(altmode->svid, 1, CMD_EXIT_MODE);
> +	header |= VDO_OPOS(altmode->mode);
> +
> +	tcpm_queue_vdm(port, header, NULL, 0);
> +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> +}
> +
> +static int tcpm_altmode_vdm(struct typec_altmode *altmode,
> +			    u32 header, const u32 *data, int count)
> +{
> +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +
> +	tcpm_queue_vdm(port, header, data, count - 1);
> +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> +
> +	return 0;
> +}
> +
> +static const struct typec_altmode_ops tcpm_altmode_ops = {
> +	.enter = tcpm_altmode_enter,
> +	.exit = tcpm_altmode_exit,
> +	.vdm = tcpm_altmode_vdm,
> +};
> +
>  /*
>   * PD (data, control) command handling functions
>   */
> @@ -3479,6 +3548,23 @@ static void tcpm_init(struct tcpm_port *port)
>  	tcpm_set_state(port, PORT_RESET, 0);
>  }
>  
> +static int tcpm_activate_mode(struct typec_altmode *alt, int activate)
> +{
> +	struct tcpm_port *port = typec_altmode_get_drvdata(alt);
> +	u32 header;
> +
> +	header = VDO(cpu_to_le16(alt->svid), 1,
> +		     activate ? CMD_ENTER_MODE : CMD_EXIT_MODE);
> +	header |= VDO_OPOS(alt->mode);
> +
> +	mutex_lock(&port->lock);
> +	tcpm_queue_vdm(port, header, NULL, 0);
> +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> +	mutex_unlock(&port->lock);

Some of the above functions are port mutex protected, some are not.
Is this on purpose ?

> +
> +	return 0;
> +}
> +
>  static int tcpm_port_type_set(const struct typec_capability *cap,
>  			      enum typec_port_type type)
>  {
> @@ -3671,6 +3757,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>  	port->typec_caps.pr_set = tcpm_pr_set;
>  	port->typec_caps.vconn_set = tcpm_vconn_set;
>  	port->typec_caps.try_role = tcpm_try_role;
> +	port->typec_caps.activate_mode = tcpm_activate_mode;
>  	port->typec_caps.port_type_set = tcpm_port_type_set;
>  
>  	port->partner_desc.identity = &port->partner_ident;
> @@ -3703,6 +3790,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>  					 dev_name(dev), paltmode->svid);
>  				break;
>  			}
> +			typec_altmode_set_drvdata(alt, port);
> +			typec_altmode_register_ops(alt, &tcpm_altmode_ops);
>  			port->port_altmode[i] = alt;
>  			i++;
>  			paltmode++;
> -- 
> 2.16.1
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [RFC,v2,3/3] usb: typec: tcpm: Support for Alternate Modes
@ 2018-03-19 12:15 Heikki Krogerus
  0 siblings, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2018-03-19 12:15 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg Kroah-Hartman, Hans de Goede, Jun Li, Regupathy, Rajaram,
	linux-usb, linux-kernel

Hi Guenter,

On Fri, Mar 16, 2018 at 02:32:06PM -0700, Guenter Roeck wrote:
> On Fri, Mar 09, 2018 at 06:19:18PM +0300, Heikki Krogerus wrote:
> > This adds more complete handling of VDMs and registration of
> > partner alternate modes, and introduces callbacks for
> > alternate mode operations.
> > 
> > Only DFP role is supported for now.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/usb/typec/tcpm.c | 133 +++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 111 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > index bfb843ebffa6..34bf5c1f4d81 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -158,13 +158,14 @@ enum pd_msg_request {
> >  /* Alternate mode support */
> >  
> >  #define SVID_DISCOVERY_MAX	16
> > +#define ALTMODE_DISCOVERY_MAX	(SVID_DISCOVERY_MAX * MODE_DISCOVERY_MAX)
> >  
> >  struct pd_mode_data {
> >  	int svid_index;		/* current SVID index		*/
> >  	int nsvids;
> >  	u16 svids[SVID_DISCOVERY_MAX];
> >  	int altmodes;		/* number of alternate modes	*/
> > -	struct typec_altmode_desc altmode_desc[SVID_DISCOVERY_MAX];
> > +	struct typec_altmode_desc altmode_desc[ALTMODE_DISCOVERY_MAX];
> >  };
> >  
> >  struct tcpm_port {
> > @@ -280,8 +281,8 @@ struct tcpm_port {
> >  	/* Alternate mode data */
> >  
> >  	struct pd_mode_data mode_data;
> > -	struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX * 6];
> > -	struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX * 6];
> > +	struct typec_altmode *partner_altmode[ALTMODE_DISCOVERY_MAX];
> > +	struct typec_altmode *port_altmode[ALTMODE_DISCOVERY_MAX];
> >  
> >  	/* Deadline in jiffies to exit src_try_wait state */
> >  	unsigned long max_wait;
> > @@ -985,36 +986,53 @@ static void svdm_consume_modes(struct tcpm_port *port, const __le32 *payload,
> >  			 pmdata->altmodes, paltmode->svid,
> >  			 paltmode->mode, paltmode->vdo);
> >  
> > -		port->partner_altmode[pmdata->altmodes] =
> > -			typec_partner_register_altmode(port->partner, paltmode);
> > -		if (!port->partner_altmode[pmdata->altmodes]) {
> > -			tcpm_log(port,
> > -				 "Failed to register modes for SVID 0x%04x",
> > -				 paltmode->svid);
> > -			return;
> > -		}
> >  		pmdata->altmodes++;
> >  	}
> >  }
> >  
> > +static void tcpm_register_partner_altmodes(struct tcpm_port *port)
> > +{
> > +	struct pd_mode_data *modep = &port->mode_data;
> > +	struct typec_altmode *altmode;
> > +	int i;
> > +
> > +	for (i = 0; i < modep->altmodes; i++) {
> > +		altmode = typec_partner_register_altmode(port->partner,
> > +						&modep->altmode_desc[i]);
> > +		if (!altmode)
> > +			tcpm_log(port, "Failed to register partner SVID 0x%04x",
> > +				 modep->altmode_desc[i].svid);
> > +		port->partner_altmode[i] = altmode;
> > +	}
> > +}
> > +
> >  #define supports_modal(port)	PD_IDH_MODAL_SUPP((port)->partner_ident.id_header)
> >  
> >  static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
> >  			u32 *response)
> >  {
> > -	u32 p0 = le32_to_cpu(payload[0]);
> > -	int cmd_type = PD_VDO_CMDT(p0);
> > -	int cmd = PD_VDO_CMD(p0);
> > +	struct typec_altmode *altmode;
> >  	struct pd_mode_data *modep;
> > +	u32 p[PD_MAX_PAYLOAD];
> >  	int rlen = 0;
> > -	u16 svid;
> > +	int cmd_type;
> > +	int cmd;
> >  	int i;
> >  
> > +	for (i = 0; i < cnt; i++)
> > +		p[i] = le32_to_cpu(payload[i]);
> > +
> > +	cmd_type = PD_VDO_CMDT(p[0]);
> > +	cmd = PD_VDO_CMD(p[0]);
> > +
> >  	tcpm_log(port, "Rx VDM cmd 0x%x type %d cmd %d len %d",
> > -		 p0, cmd_type, cmd, cnt);
> > +		 p[0], cmd_type, cmd, cnt);
> >  
> >  	modep = &port->mode_data;
> >  
> > +	altmode = typec_match_altmode(port->port_altmode, ALTMODE_DISCOVERY_MAX,
> > +				      PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
> 
> This can return NULL ...
> 
> > +
> >  	switch (cmd_type) {
> >  	case CMDT_INIT:
> >  		switch (cmd) {
> > @@ -1036,17 +1054,21 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
> >  		case CMD_EXIT_MODE:
> >  			break;
> >  		case CMD_ATTENTION:
> > -			break;
> > +			typec_altmode_attention(altmode, p[1]);
> 
> ... and NULL is not handled by typec_altmode_attention().

True.

> > +			return 0;
> >  		default:
> > +			if (typec_altmode_vdm(altmode, p[0], &p[1], cnt) ==
> > +			    VDM_OK)
> 
> typec_altmode_vdm() returns old fashioned error messages.
> Not sure what it is supposed to return, though.

The idea was that the alternate mode driver can inform the port driver
that it not support a specific VDM. In that case we need to NAK.

I'll think about this a bit more.

> > +				return 0;
> >  			break;
> >  		}
> >  		if (rlen >= 1) {
> > -			response[0] = p0 | VDO_CMDT(CMDT_RSP_ACK);
> > +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_ACK);
> >  		} else if (rlen == 0) {
> > -			response[0] = p0 | VDO_CMDT(CMDT_RSP_NAK);
> > +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> >  			rlen = 1;
> >  		} else {
> > -			response[0] = p0 | VDO_CMDT(CMDT_RSP_BUSY);
> > +			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
> >  			rlen = 1;
> >  		}
> >  		break;
> > @@ -1079,20 +1101,26 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
> >  			svdm_consume_modes(port, payload, cnt);
> >  			modep->svid_index++;
> >  			if (modep->svid_index < modep->nsvids) {
> > -				svid = modep->svids[modep->svid_index];
> > +				u16 svid = modep->svids[modep->svid_index];
> >  				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
> >  				rlen = 1;
> >  			} else {
> > -				/* enter alternate mode if/when implemented */
> > +				tcpm_register_partner_altmodes(port);
> >  			}
> >  			break;
> >  		case CMD_ENTER_MODE:
> > +			typec_altmode_enter(altmode);
> 
> typec_altmode_enter() don't handle altmode == NULL.

True. I'll fix that as well.

> No error handling ?
> 
> > +			break;
> > +		case CMD_EXIT_MODE:
> > +			typec_altmode_exit(altmode);
> 
> Doesn't handle NULL. Error handling ?

True, and I'll add error handling for there.

> >  			break;
> >  		default:
> > +			typec_altmode_vdm(altmode, p[0], &p[1], cnt);
> >  			break;
> >  		}
> >  		break;
> >  	default:
> > +		typec_altmode_vdm(altmode, p[0], &p[1], cnt);
> >  		break;
> >  	}
> >  
> > @@ -1352,6 +1380,47 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
> >  	return 0;
> >  }
> >  
> > +static void tcpm_altmode_enter(struct typec_altmode *altmode)
> > +{
> > +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > +	u32 header;
> > +
> > +	header = VDO(altmode->svid, 1, CMD_ENTER_MODE);
> > +	header |= VDO_OPOS(altmode->mode);
> > +
> > +	tcpm_queue_vdm(port, header, NULL, 0);
> > +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> > +}
> > +
> > +static void tcpm_altmode_exit(struct typec_altmode *altmode)
> > +{
> > +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > +	u32 header;
> > +
> > +	header = VDO(altmode->svid, 1, CMD_EXIT_MODE);
> > +	header |= VDO_OPOS(altmode->mode);
> > +
> > +	tcpm_queue_vdm(port, header, NULL, 0);
> > +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> > +}
> > +
> > +static int tcpm_altmode_vdm(struct typec_altmode *altmode,
> > +			    u32 header, const u32 *data, int count)
> > +{
> > +	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > +
> > +	tcpm_queue_vdm(port, header, data, count - 1);
> > +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct typec_altmode_ops tcpm_altmode_ops = {
> > +	.enter = tcpm_altmode_enter,
> > +	.exit = tcpm_altmode_exit,
> > +	.vdm = tcpm_altmode_vdm,
> > +};
> > +
> >  /*
> >   * PD (data, control) command handling functions
> >   */
> > @@ -3479,6 +3548,23 @@ static void tcpm_init(struct tcpm_port *port)
> >  	tcpm_set_state(port, PORT_RESET, 0);
> >  }
> >  
> > +static int tcpm_activate_mode(struct typec_altmode *alt, int activate)
> > +{
> > +	struct tcpm_port *port = typec_altmode_get_drvdata(alt);
> > +	u32 header;
> > +
> > +	header = VDO(cpu_to_le16(alt->svid), 1,
> > +		     activate ? CMD_ENTER_MODE : CMD_EXIT_MODE);
> > +	header |= VDO_OPOS(alt->mode);
> > +
> > +	mutex_lock(&port->lock);
> > +	tcpm_queue_vdm(port, header, NULL, 0);
> > +	mod_delayed_work(port->wq, &port->vdm_state_machine, 0);
> > +	mutex_unlock(&port->lock);
> 
> Some of the above functions are port mutex protected, some are not.
> Is this on purpose ?

No. The plan is to always use the mutex.

I can't call typec_altmode_enter/exit() directly from tcpm_pd_svdm()
because the alternate mode drivers may call typec_altmode_enter() from
their probe drivers. I guess the solution is that instead of directly
calling typec_altmode_enter/exit() from tcpm_pm_svdm(), we schedule a
work where we call them. Or would you have some better ideas for this?

For this RFC I just hacked it and didn't use the mutex in
tcpm_altmode_enter/exit().


Thanks for reviewing these,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-03-19 12:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-16 21:32 [RFC,v2,3/3] usb: typec: tcpm: Support for Alternate Modes Guenter Roeck
  -- strict thread matches above, loose matches on Subject: below --
2018-03-19 12:15 Heikki Krogerus
2018-03-09 15:19 Heikki Krogerus

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).