[PATCH v1 1/2] usb: typec: class: add typec_get_data_role symbol

RD Babiera posted 2 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v1 1/2] usb: typec: class: add typec_get_data_role symbol
Posted by RD Babiera 1 month, 1 week ago
Alt Mode drivers are responsible for sending Enter Mode through the TCPM,
but only a DFP is allowed to send Enter Mode. typec_get_data_role gets
the port's data role, which can then be used in altmode drivers via
typec_altmode_get_data_role to know if Enter Mode should be sent.

Signed-off-by: RD Babiera <rdbabiera@google.com>
---
 drivers/usb/typec/class.c         | 13 +++++++++++++
 include/linux/usb/typec.h         |  1 +
 include/linux/usb/typec_altmode.h | 12 ++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 67a533e35150..9b2647cb199b 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -2120,6 +2120,19 @@ void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
 }
 EXPORT_SYMBOL_GPL(typec_set_data_role);
 
+/**
+ * typec_get_data_role - Get port data role
+ * @port: The USB Type-C Port to query
+ *
+ * This routine is used by the altmode drivers to determine if the port is the
+ * DFP before issuing Enter Mode
+ */
+enum typec_data_role typec_get_data_role(struct typec_port *port)
+{
+	return port->data_role;
+}
+EXPORT_SYMBOL_GPL(typec_get_data_role);
+
 /**
  * typec_set_pwr_role - Report power role change
  * @port: The USB Type-C Port where the role was changed
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 252af3f77039..309251572e2e 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -337,6 +337,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
 void typec_unregister_plug(struct typec_plug *plug);
 
 void typec_set_data_role(struct typec_port *port, enum typec_data_role role);
+enum typec_data_role typec_get_data_role(struct typec_port *port);
 void typec_set_pwr_role(struct typec_port *port, enum typec_role role);
 void typec_set_vconn_role(struct typec_port *port, enum typec_role role);
 void typec_set_pwr_opmode(struct typec_port *port, enum typec_pwr_opmode mode);
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index b3c0866ea70f..559cd6865ba1 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -172,6 +172,18 @@ typec_altmode_get_svdm_version(struct typec_altmode *altmode)
 	return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
 }
 
+/**
+ * typec_altmode_get_data_role - Get port data role. Alt Mode drivers should only
+ * issue Enter Mode through the port if they are the DFP.
+ *
+ * @altmode Handle to the alternate mode
+ */
+static inline enum typec_data_role
+typec_altmode_get_data_role(struct typec_altmode *altmode)
+{
+	return typec_get_data_role(typec_altmode2port(altmode));
+}
+
 /**
  * struct typec_altmode_driver - USB Type-C alternate mode device driver
  * @id_table: Null terminated array of SVIDs
-- 
2.51.0.261.g7ce5a0a67e-goog
Re: [PATCH v1 1/2] usb: typec: class: add typec_get_data_role symbol
Posted by Heikki Krogerus 1 month ago
On Thu, Aug 21, 2025 at 08:38:34PM +0000, RD Babiera wrote:
> Alt Mode drivers are responsible for sending Enter Mode through the TCPM,
> but only a DFP is allowed to send Enter Mode. typec_get_data_role gets
> the port's data role, which can then be used in altmode drivers via
> typec_altmode_get_data_role to know if Enter Mode should be sent.

The functions are okay by me, but is the above statement correct?
Are you mixing power role and data role?

> Signed-off-by: RD Babiera <rdbabiera@google.com>
> ---
>  drivers/usb/typec/class.c         | 13 +++++++++++++
>  include/linux/usb/typec.h         |  1 +
>  include/linux/usb/typec_altmode.h | 12 ++++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 67a533e35150..9b2647cb199b 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -2120,6 +2120,19 @@ void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
>  }
>  EXPORT_SYMBOL_GPL(typec_set_data_role);
>  
> +/**
> + * typec_get_data_role - Get port data role
> + * @port: The USB Type-C Port to query
> + *
> + * This routine is used by the altmode drivers to determine if the port is the
> + * DFP before issuing Enter Mode
> + */
> +enum typec_data_role typec_get_data_role(struct typec_port *port)
> +{
> +	return port->data_role;
> +}
> +EXPORT_SYMBOL_GPL(typec_get_data_role);
> +
>  /**
>   * typec_set_pwr_role - Report power role change
>   * @port: The USB Type-C Port where the role was changed
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 252af3f77039..309251572e2e 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -337,6 +337,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
>  void typec_unregister_plug(struct typec_plug *plug);
>  
>  void typec_set_data_role(struct typec_port *port, enum typec_data_role role);
> +enum typec_data_role typec_get_data_role(struct typec_port *port);
>  void typec_set_pwr_role(struct typec_port *port, enum typec_role role);
>  void typec_set_vconn_role(struct typec_port *port, enum typec_role role);
>  void typec_set_pwr_opmode(struct typec_port *port, enum typec_pwr_opmode mode);
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index b3c0866ea70f..559cd6865ba1 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -172,6 +172,18 @@ typec_altmode_get_svdm_version(struct typec_altmode *altmode)
>  	return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
>  }
>  
> +/**
> + * typec_altmode_get_data_role - Get port data role. Alt Mode drivers should only
> + * issue Enter Mode through the port if they are the DFP.

The second sentence should go below. But I'm not sure it's correct.

> + * @altmode Handle to the alternate mode
> + */
> +static inline enum typec_data_role
> +typec_altmode_get_data_role(struct typec_altmode *altmode)
> +{
> +	return typec_get_data_role(typec_altmode2port(altmode));
> +}
> +
>  /**
>   * struct typec_altmode_driver - USB Type-C alternate mode device driver
>   * @id_table: Null terminated array of SVIDs
> -- 
> 2.51.0.261.g7ce5a0a67e-goog

-- 
heikki
Re: [PATCH v1 1/2] usb: typec: class: add typec_get_data_role symbol
Posted by RD Babiera 1 month ago
On Thu, Aug 28, 2025 at 5:44 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
> > Alt Mode drivers are responsible for sending Enter Mode through the TCPM,
> > but only a DFP is allowed to send Enter Mode. typec_get_data_role gets
> > the port's data role, which can then be used in altmode drivers via
> > typec_altmode_get_data_role to know if Enter Mode should be sent.
> The functions are okay by me, but is the above statement correct?
> Are you mixing power role and data role?

The PD Specification lists the DFP as being the USB host when USB
Communication is supported while acting as a DFP; it gives
a charger being a DFP and not supporting USB Communication as an
example.

> > @@ -172,6 +172,18 @@ typec_altmode_get_svdm_version(struct typec_altmode *altmode)
> >       return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
> >  }
> >
> > +/**
> > + * typec_altmode_get_data_role - Get port data role. Alt Mode drivers should only
> > + * issue Enter Mode through the port if they are the DFP.
>
> The second sentence should go below. But I'm not sure it's correct.

The kernel-doc comment guide agrees with that statement. I'll change it,
thanks for the heads up!

---
RD