Re: [PATCH v5 1/8] usb: typec: Manage SVDM version

Heikki Krogerus
2021-02-03T13:37:42Z

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

Hi Kyle,

On Wed, Feb 03, 2021 at 02:47:28PM +0200, Heikki Krogerus wrote:
> 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.
>
> 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.

I'm attaching here my (quite crude) proposal how to do this. It should
give you an idea what I'm after here.

Br,

--
heikki

[-- Attachment #2: svdm_version_proposal.diff --]
[-- Type: text/plain, Size: 2407 bytes --]

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 8f77669f9cf4f..04238b0a5d47f 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -36,6 +36,8 @@ struct typec_partner {
enum typec_accessory accessory;
struct ida mode_ids;
int num_altmodes;
+
+ enum usb_pd_svdm_ver svdm_version;
};

struct typec_port {
@@ -792,6 +794,18 @@ typec_partner_register_altmode(struct typec_partner *partner,
}
EXPORT_SYMBOL_GPL(typec_partner_register_altmode);

+/**
+ * typec_partner_set_svdm_version - Set negotiated SVDM version
+ * @partner: The partner.
+ * ...
+ */
+void typec_partner_set_svdm_version(struct typec_partner *partner,
+ enum usb_pd_svdm_ver svdm_version)
+{
+ partner->svdm_version = svdm_version;
+}
+EXPORT_SYMBOL_GPL(typec_partner_set_svdm_version);
+
/**
* typec_register_partner - Register a USB Type-C Partner
* @port: The USB Type-C Port the partner is connected to
@@ -1847,6 +1861,27 @@ EXPORT_SYMBOL_GPL(typec_set_mode);

/* --------------------------------------- */

+/**
+ * typec_get_negotiated_svdm_version - Get negotiated SVDM version
+ * @port: The port.
+ * ...
+ */
+int typec_get_negotiated_svdm_version(struct typec_port *port)
+{
+ enum usb_pd_svdm_ver svdm_version;
+ struct device *partner_dev;
+
+ partner_dev = device_find_child(&port->dev, NULL, partner_match);
+ if (!partner_dev)
+ return -ENODEV;
+
+ svdm_version = to_typec_partner(partner_dev)->svdm_version;
+ put_device(partner_dev);
+
+ return svdm_version
+}
+EXPORT_SYMBOL_GPL(typec_partner_set_svdm_version);
+
/**
* typec_get_drvdata - Return private driver data pointer
* @port: USB Type-C port
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 5e0a7b7647c3b..91e119e37ba70 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -132,6 +132,16 @@ typec_altmode_get_orientation(struct typec_altmode *altmode)
return typec_get_orientation(typec_altmode2port(altmode));
}

+/**
+ * typec_get_negotiated_svdm_version - Get negotiated SVDM version
+ * ...
+ */
+static inline int
+typec_altmode_get_svdm_version(struct typec_altmode *altmode)
+{
+ return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
+}
+
/**
* struct typec_altmode_driver - USB Type-C alternate mode device driver
* @id_table: Null terminated array of SVIDs