| Message ID | 20210202161733.932215-2-kyletso@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | common SVDM version and VDO from dt | expand |
On Wed, Feb 03, 2021 at 02:47:24PM +0200, Heikki Krogerus wrote: > Hi Kyle, > > On Wed, Feb 03, 2021 at 12:17:26AM +0800, Kyle Tso wrote: > > PD Spec Revision 3.0 Version 2.0 + ECNs 2020-12-10 > > 6.4.4.2.3 Structured VDM Version > > "The Structured VDM Version field of the Discover Identity Command > > sent and received during VDM discovery Shall be used to determine the > > lowest common Structured VDM Version supported by the Port Partners or > > Cable Plug and Shall continue to operate using this Specification > > Revision until they are Detached." > > > > Add a variable in typec_capability to specify the highest SVDM version > > supported by the port and another variable in typec_port to cache the > > negotiated SVDM version between the port partners. > > > > Also add setter/getter functions for the negotiated SVDM version. > > > > Signed-off-by: Kyle Tso <kyletso@google.com> > > --- > > drivers/usb/typec/class.c | 13 +++++++++++++ > > include/linux/usb/typec.h | 10 ++++++++++ > > 2 files changed, 23 insertions(+) > > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > > index b6ceab3dc16b..42d1be1eece9 100644 > > --- a/drivers/usb/typec/class.c > > +++ b/drivers/usb/typec/class.c > > @@ -51,6 +51,7 @@ struct typec_port { > > enum typec_role vconn_role; > > enum typec_pwr_opmode pwr_opmode; > > enum typec_port_type port_type; > > + enum usb_pd_svdm_ver svdm_version; > > struct mutex port_type_lock; > > I just realized that you are storing that in the port object. I guess > we don't have to change this right now, but it would have been more > clear to store that in the partner object IMO. > > > enum typec_orientation orientation; > > @@ -1841,6 +1842,18 @@ int typec_find_port_data_role(const char *name) > > } > > EXPORT_SYMBOL_GPL(typec_find_port_data_role); > > > > +void typec_set_svdm_version(struct typec_port *port, enum usb_pd_svdm_ver ver) > > +{ > > + port->svdm_version = ver; > > +} > > +EXPORT_SYMBOL_GPL(typec_set_svdm_version); > > + > > +enum usb_pd_svdm_ver typec_get_svdm_version(struct typec_port *port) > > +{ > > + return port->svdm_version; > > +} > > +EXPORT_SYMBOL_GPL(typec_get_svdm_version); > > You need to document those exported functions! You need to do that in > any case, but in this case it's very important, because the purpose of > these functions is not clear from the ctx. Thinking about it, would it make make sense to define the functions as static inline ? Thanks, Guenter > > I'm sorry for noticing that so late. Since you do need to fix that, > please see if you can also store that detail in the partner device > object instead of the port object. > > thanks, > > -- > heikki
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index b6ceab3dc16b..42d1be1eece9 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -51,6 +51,7 @@ struct typec_port { enum typec_role vconn_role; enum typec_pwr_opmode pwr_opmode; enum typec_port_type port_type; + enum usb_pd_svdm_ver svdm_version; struct mutex port_type_lock; enum typec_orientation orientation; @@ -1841,6 +1842,18 @@ int typec_find_port_data_role(const char *name) } EXPORT_SYMBOL_GPL(typec_find_port_data_role); +void typec_set_svdm_version(struct typec_port *port, enum usb_pd_svdm_ver ver) +{ + port->svdm_version = ver; +} +EXPORT_SYMBOL_GPL(typec_set_svdm_version); + +enum usb_pd_svdm_ver typec_get_svdm_version(struct typec_port *port) +{ + return port->svdm_version; +} +EXPORT_SYMBOL_GPL(typec_get_svdm_version); + /* ------------------------------------------ */ /* API for Multiplexer/DeMultiplexer Switches */ diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h index 4946eca742d5..f692d9ee0f13 100644 --- a/include/linux/usb/typec.h +++ b/include/linux/usb/typec.h @@ -217,12 +217,19 @@ struct typec_operations { enum typec_port_type type); }; +enum usb_pd_svdm_ver { + SVDM_VER_1_0 = 0, + SVDM_VER_2_0 = 1, + SVDM_VER_MAX = SVDM_VER_2_0, +}; + /* * struct typec_capability - USB Type-C Port Capabilities * @type: Supported power role of the port * @data: Supported data role of the port * @revision: USB Type-C Specification release. Binary coded decimal * @pd_revision: USB Power Delivery Specification revision if supported + * @svdm_version: USB PD Structured VDM version if supported * @prefer_role: Initial role preference (DRP ports). * @accessory: Supported Accessory Modes * @fwnode: Optional fwnode of the port @@ -236,6 +243,7 @@ struct typec_capability { enum typec_port_data data; u16 revision; /* 0120H = "1.2" */ u16 pd_revision; /* 0300H = "3.0" */ + enum usb_pd_svdm_ver svdm_version; int prefer_role; enum typec_accessory accessory[TYPEC_MAX_ACCESSORY]; unsigned int orientation_aware:1; @@ -286,4 +294,6 @@ int typec_find_orientation(const char *name); int typec_find_port_power_role(const char *name); int typec_find_power_role(const char *name); int typec_find_port_data_role(const char *name); +void typec_set_svdm_version(struct typec_port *port, enum usb_pd_svdm_ver); +enum usb_pd_svdm_ver typec_get_svdm_version(struct typec_port *port); #endif /* __LINUX_USB_TYPEC_H */
PD Spec Revision 3.0 Version 2.0 + ECNs 2020-12-10 6.4.4.2.3 Structured VDM Version "The Structured VDM Version field of the Discover Identity Command sent and received during VDM discovery Shall be used to determine the lowest common Structured VDM Version supported by the Port Partners or Cable Plug and Shall continue to operate using this Specification Revision until they are Detached." Add a variable in typec_capability to specify the highest SVDM version supported by the port and another variable in typec_port to cache the negotiated SVDM version between the port partners. Also add setter/getter functions for the negotiated SVDM version. Signed-off-by: Kyle Tso <kyletso@google.com> --- drivers/usb/typec/class.c | 13 +++++++++++++ include/linux/usb/typec.h | 10 ++++++++++ 2 files changed, 23 insertions(+)