* [PATCH v2 0/7] usb: typec: Small API improvement
@ 2019-10-04 15:08 Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 1/7] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Hi guys,
In this version there should be no more semantic changes.
The original cover letter:
This series moves the callback members from struct typec_capabilities
to a new struct typec_operations. That removes the need for the
drivers to keep a copy of the struct typec_capabilites if they don't
need it, and struct typec_operations can probable always be
constified.
The change is small, however I think the code ends up being a bit more
cleaner looking this way. Let me know if it's OK.
thanks,
Heikki Krogerus (7):
usb: typec: Copy everything from struct typec_capability during
registration
usb: typec: Introduce typec_get_drvdata()
usb: typec: Separate the operations vector
usb: typec: tcpm: Start using struct typec_operations
usb: typec: tps6598x: Start using struct typec_operations
usb: typec: ucsi: Start using struct typec_operations
usb: typec: Remove the callback members from struct typec_capability
drivers/usb/typec/class.c | 37 ++++++++++++++++++--------
drivers/usb/typec/tcpm/tcpm.c | 45 ++++++++++++++------------------
drivers/usb/typec/tps6598x.c | 49 +++++++++++++++++++----------------
drivers/usb/typec/ucsi/ucsi.c | 22 ++++++++--------
include/linux/usb/typec.h | 39 ++++++++++++++++------------
5 files changed, 106 insertions(+), 86 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/7] usb: typec: Copy everything from struct typec_capability during registration
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 2/7] usb: typec: Introduce typec_get_drvdata() Heikki Krogerus
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Copying everything from struct typec_capability to struct
typec_port during port registration. This will make sure
that under no circumstances the driver can change the values
in the struct typec_capability that the port uses.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/class.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 94a3eda62add..0bbf10c8ad58 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -52,6 +52,7 @@ struct typec_port {
struct typec_switch *sw;
struct typec_mux *mux;
+ const struct typec_capability *orig_cap; /* to be removed */
const struct typec_capability *cap;
};
@@ -968,7 +969,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
return -EINVAL;
}
- ret = port->cap->try_role(port->cap, role);
+ ret = port->cap->try_role(port->orig_cap, role);
if (ret)
return ret;
@@ -1014,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->dr_set(port->cap, ret);
+ ret = port->cap->dr_set(port->orig_cap, ret);
if (ret)
goto unlock_and_ret;
@@ -1071,7 +1072,7 @@ static ssize_t power_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->pr_set(port->cap, ret);
+ ret = port->cap->pr_set(port->orig_cap, ret);
if (ret)
goto unlock_and_ret;
@@ -1119,7 +1120,7 @@ port_type_store(struct device *dev, struct device_attribute *attr,
goto unlock_and_ret;
}
- ret = port->cap->port_type_set(port->cap, type);
+ ret = port->cap->port_type_set(port->orig_cap, type);
if (ret)
goto unlock_and_ret;
@@ -1184,7 +1185,7 @@ static ssize_t vconn_source_store(struct device *dev,
if (ret)
return ret;
- ret = port->cap->vconn_set(port->cap, (enum typec_role)source);
+ ret = port->cap->vconn_set(port->orig_cap, (enum typec_role)source);
if (ret)
return ret;
@@ -1278,6 +1279,7 @@ static void typec_release(struct device *dev)
ida_destroy(&port->mode_ids);
typec_switch_put(port->sw);
typec_mux_put(port->mux);
+ kfree(port->cap);
kfree(port);
}
@@ -1579,9 +1581,10 @@ struct typec_port *typec_register_port(struct device *parent,
mutex_init(&port->port_type_lock);
port->id = id;
- port->cap = cap;
+ port->orig_cap = cap;
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
+ port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
device_initialize(&port->dev);
port->dev.class = typec_class;
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/7] usb: typec: Introduce typec_get_drvdata()
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 1/7] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 3/7] usb: typec: Separate the operations vector Heikki Krogerus
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Leaving the private driver_data pointer of the port device
to the port drivers.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/class.c | 11 +++++++++++
include/linux/usb/typec.h | 4 ++++
2 files changed, 15 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 0bbf10c8ad58..89ffe370e426 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1488,6 +1488,16 @@ EXPORT_SYMBOL_GPL(typec_set_mode);
/* --------------------------------------- */
+/**
+ * typec_get_drvdata - Return private driver data pointer
+ * @port: USB Type-C port
+ */
+void *typec_get_drvdata(struct typec_port *port)
+{
+ return dev_get_drvdata(&port->dev);
+}
+EXPORT_SYMBOL_GPL(typec_get_drvdata);
+
/**
* typec_port_register_altmode - Register USB Type-C Port Alternate Mode
* @port: USB Type-C Port that supports the alternate mode
@@ -1592,6 +1602,7 @@ struct typec_port *typec_register_port(struct device *parent,
port->dev.fwnode = cap->fwnode;
port->dev.type = &typec_port_dev_type;
dev_set_name(&port->dev, "port%d", id);
+ dev_set_drvdata(&port->dev, cap->driver_data);
port->sw = typec_switch_get(&port->dev);
if (IS_ERR(port->sw)) {
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 7df4ecabc78a..8b90cd77331c 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -179,6 +179,7 @@ struct typec_partner_desc {
* @sw: Cable plug orientation switch
* @mux: Multiplexer switch for Alternate/Accessory Modes
* @fwnode: Optional fwnode of the port
+ * @driver_data: Private pointer for driver specific info
* @try_role: Set data role preference for DRP port
* @dr_set: Set Data Role
* @pr_set: Set Power Role
@@ -198,6 +199,7 @@ struct typec_capability {
struct typec_switch *sw;
struct typec_mux *mux;
struct fwnode_handle *fwnode;
+ void *driver_data;
int (*try_role)(const struct typec_capability *,
int role);
@@ -241,6 +243,8 @@ int typec_set_orientation(struct typec_port *port,
enum typec_orientation typec_get_orientation(struct typec_port *port);
int typec_set_mode(struct typec_port *port, int mode);
+void *typec_get_drvdata(struct typec_port *port);
+
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);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/7] usb: typec: Separate the operations vector
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 1/7] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 2/7] usb: typec: Introduce typec_get_drvdata() Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-04 15:17 ` Heikki Krogerus
2019-10-05 17:36 ` Guenter Roeck
2019-10-04 15:08 ` [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
` (3 subsequent siblings)
6 siblings, 2 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Introducing struct typec_operations which has the same
callbacks as struct typec_capability. The old callbacks are
kept for now, but after all users have been converted, they
will be removed.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/class.c | 39 +++++++++++++++++++++++++++++----------
include/linux/usb/typec.h | 20 ++++++++++++++++++++
2 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 89ffe370e426..f4972b7ee022 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -54,6 +54,7 @@ struct typec_port {
const struct typec_capability *orig_cap; /* to be removed */
const struct typec_capability *cap;
+ const struct typec_operations *ops;
};
#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
@@ -956,7 +957,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
return -EOPNOTSUPP;
}
- if (!port->cap->try_role) {
+ if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {
dev_dbg(dev, "Setting preferred role not supported\n");
return -EOPNOTSUPP;
}
@@ -969,7 +970,10 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
return -EINVAL;
}
- ret = port->cap->try_role(port->orig_cap, role);
+ if (port->ops && port->ops->try_role)
+ ret = port->ops->try_role(port, role);
+ else
+ ret = port->cap->try_role(port->orig_cap, role);
if (ret)
return ret;
@@ -1000,7 +1004,7 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
- if (!port->cap->dr_set) {
+ if (!port->cap->dr_set || (!port->ops && !port->ops->dr_set)) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1015,7 +1019,10 @@ static ssize_t data_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->dr_set(port->orig_cap, ret);
+ if (port->ops && port->ops->dr_set)
+ ret = port->ops->dr_set(port, ret);
+ else
+ ret = port->cap->dr_set(port->orig_cap, ret);
if (ret)
goto unlock_and_ret;
@@ -1050,7 +1057,7 @@ static ssize_t power_role_store(struct device *dev,
return -EOPNOTSUPP;
}
- if (!port->cap->pr_set) {
+ if (!port->cap->pr_set || (!port->ops && !port->ops->pr_set)) {
dev_dbg(dev, "power role swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1072,7 +1079,10 @@ static ssize_t power_role_store(struct device *dev,
goto unlock_and_ret;
}
- ret = port->cap->pr_set(port->orig_cap, ret);
+ if (port->ops && port->ops->dr_set)
+ ret = port->ops->pr_set(port, ret);
+ else
+ ret = port->cap->pr_set(port->orig_cap, ret);
if (ret)
goto unlock_and_ret;
@@ -1103,7 +1113,8 @@ port_type_store(struct device *dev, struct device_attribute *attr,
int ret;
enum typec_port_type type;
- if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
+ if (port->cap->type != TYPEC_PORT_DRP || !port->cap->port_type_set ||
+ (!port->ops && !port->ops->port_type_set)) {
dev_dbg(dev, "changing port type not supported\n");
return -EOPNOTSUPP;
}
@@ -1120,7 +1131,10 @@ port_type_store(struct device *dev, struct device_attribute *attr,
goto unlock_and_ret;
}
- ret = port->cap->port_type_set(port->orig_cap, type);
+ if (port->ops && port->ops->port_type_set)
+ ret = port->ops->port_type_set(port, type);
+ else
+ ret = port->cap->port_type_set(port->orig_cap, type);
if (ret)
goto unlock_and_ret;
@@ -1176,7 +1190,7 @@ static ssize_t vconn_source_store(struct device *dev,
return -EOPNOTSUPP;
}
- if (!port->cap->vconn_set) {
+ if (!port->cap->vconn_set || (!port->ops && !port->ops->vconn_set)) {
dev_dbg(dev, "VCONN swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1185,7 +1199,11 @@ static ssize_t vconn_source_store(struct device *dev,
if (ret)
return ret;
- ret = port->cap->vconn_set(port->orig_cap, (enum typec_role)source);
+ if (port->ops && port->ops->vconn_set)
+ ret = port->ops->vconn_set(port, (enum typec_role)source);
+ else
+ ret = port->cap->vconn_set(port->orig_cap,
+ (enum typec_role)source);
if (ret)
return ret;
@@ -1591,6 +1609,7 @@ struct typec_port *typec_register_port(struct device *parent,
mutex_init(&port->port_type_lock);
port->id = id;
+ port->ops = cap->ops;
port->orig_cap = cap;
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 8b90cd77331c..c9bef128453b 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -168,6 +168,23 @@ struct typec_partner_desc {
struct usb_pd_identity *identity;
};
+/**
+ * struct typec_operations - USB Type-C Port Operations
+ * @try_role: Set data role preference for DRP port
+ * @dr_set: Set Data Role
+ * @pr_set: Set Power Role
+ * @vconn_set: Source VCONN
+ * @port_type_set: Set port type
+ */
+struct typec_operations {
+ int (*try_role)(struct typec_port *port, int role);
+ int (*dr_set)(struct typec_port *port, enum typec_data_role role);
+ int (*pr_set)(struct typec_port *port, enum typec_role role);
+ int (*vconn_set)(struct typec_port *port, enum typec_role role);
+ int (*port_type_set)(struct typec_port *port,
+ enum typec_port_type type);
+};
+
/*
* struct typec_capability - USB Type-C Port Capabilities
* @type: Supported power role of the port
@@ -180,6 +197,7 @@ struct typec_partner_desc {
* @mux: Multiplexer switch for Alternate/Accessory Modes
* @fwnode: Optional fwnode of the port
* @driver_data: Private pointer for driver specific info
+ * @ops: Port operations vector
* @try_role: Set data role preference for DRP port
* @dr_set: Set Data Role
* @pr_set: Set Power Role
@@ -201,6 +219,8 @@ struct typec_capability {
struct fwnode_handle *fwnode;
void *driver_data;
+ const struct typec_operations *ops;
+
int (*try_role)(const struct typec_capability *,
int role);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
` (2 preceding siblings ...)
2019-10-04 15:08 ` [PATCH v2 3/7] usb: typec: Separate the operations vector Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-05 17:39 ` Guenter Roeck
2019-10-04 15:08 ` [PATCH v2 5/7] usb: typec: tps6598x: " Heikki Krogerus
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/tcpm/tcpm.c | 45 ++++++++++++++++-------------------
1 file changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 96562744101c..32b4ce6ce60b 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -390,12 +390,6 @@ static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
return SRC_UNATTACHED;
}
-static inline
-struct tcpm_port *typec_cap_to_tcpm(const struct typec_capability *cap)
-{
- return container_of(cap, struct tcpm_port, typec_caps);
-}
-
static bool tcpm_port_is_disconnected(struct tcpm_port *port)
{
return (!port->attached && port->cc1 == TYPEC_CC_OPEN &&
@@ -3970,10 +3964,9 @@ void tcpm_pd_hard_reset(struct tcpm_port *port)
}
EXPORT_SYMBOL_GPL(tcpm_pd_hard_reset);
-static int tcpm_dr_set(const struct typec_capability *cap,
- enum typec_data_role data)
+static int tcpm_dr_set(struct typec_port *p, enum typec_data_role data)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4038,10 +4031,9 @@ static int tcpm_dr_set(const struct typec_capability *cap,
return ret;
}
-static int tcpm_pr_set(const struct typec_capability *cap,
- enum typec_role role)
+static int tcpm_pr_set(struct typec_port *p, enum typec_role role)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4082,10 +4074,9 @@ static int tcpm_pr_set(const struct typec_capability *cap,
return ret;
}
-static int tcpm_vconn_set(const struct typec_capability *cap,
- enum typec_role role)
+static int tcpm_vconn_set(struct typec_port *p, enum typec_role role)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
int ret;
mutex_lock(&port->swap_lock);
@@ -4122,9 +4113,9 @@ static int tcpm_vconn_set(const struct typec_capability *cap,
return ret;
}
-static int tcpm_try_role(const struct typec_capability *cap, int role)
+static int tcpm_try_role(struct typec_port *p, int role)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
struct tcpc_dev *tcpc = port->tcpc;
int ret = 0;
@@ -4331,10 +4322,9 @@ static void tcpm_init(struct tcpm_port *port)
tcpm_set_state(port, PORT_RESET, 0);
}
-static int tcpm_port_type_set(const struct typec_capability *cap,
- enum typec_port_type type)
+static int tcpm_port_type_set(struct typec_port *p, enum typec_port_type type)
{
- struct tcpm_port *port = typec_cap_to_tcpm(cap);
+ struct tcpm_port *port = typec_get_drvdata(p);
mutex_lock(&port->lock);
if (type == port->port_type)
@@ -4359,6 +4349,14 @@ static int tcpm_port_type_set(const struct typec_capability *cap,
return 0;
}
+static const struct typec_operations tcpm_ops = {
+ .try_role = tcpm_try_role,
+ .dr_set = tcpm_dr_set,
+ .pr_set = tcpm_pr_set,
+ .vconn_set = tcpm_vconn_set,
+ .port_type_set = tcpm_port_type_set
+};
+
void tcpm_tcpc_reset(struct tcpm_port *port)
{
mutex_lock(&port->lock);
@@ -4770,11 +4768,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
port->typec_caps.fwnode = tcpc->fwnode;
port->typec_caps.revision = 0x0120; /* Type-C spec release 1.2 */
port->typec_caps.pd_revision = 0x0300; /* USB-PD spec release 3.0 */
- port->typec_caps.dr_set = tcpm_dr_set;
- 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.port_type_set = tcpm_port_type_set;
+ port->typec_caps.driver_data = port;
+ port->typec_caps.ops = &tcpm_ops;
port->partner_desc.identity = &port->partner_ident;
port->port_type = port->typec_caps.type;
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/7] usb: typec: tps6598x: Start using struct typec_operations
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
` (3 preceding siblings ...)
2019-10-04 15:08 ` [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 6/7] usb: typec: ucsi: " Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
6 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration. After this there
is not need to keep the capabilities stored anywhere in the
driver.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/usb/typec/tps6598x.c | 49 +++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
index a38d1409f15b..0698addd1185 100644
--- a/drivers/usb/typec/tps6598x.c
+++ b/drivers/usb/typec/tps6598x.c
@@ -94,7 +94,6 @@ struct tps6598x {
struct typec_port *port;
struct typec_partner *partner;
struct usb_pd_identity partner_identity;
- struct typec_capability typec_cap;
};
/*
@@ -307,11 +306,10 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
return 0;
}
-static int
-tps6598x_dr_set(const struct typec_capability *cap, enum typec_data_role role)
+static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
{
- struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
const char *cmd = (role == TYPEC_DEVICE) ? "SWUF" : "SWDF";
+ struct tps6598x *tps = typec_get_drvdata(port);
u32 status;
int ret;
@@ -338,11 +336,10 @@ tps6598x_dr_set(const struct typec_capability *cap, enum typec_data_role role)
return ret;
}
-static int
-tps6598x_pr_set(const struct typec_capability *cap, enum typec_role role)
+static int tps6598x_pr_set(struct typec_port *port, enum typec_role role)
{
- struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
const char *cmd = (role == TYPEC_SINK) ? "SWSk" : "SWSr";
+ struct tps6598x *tps = typec_get_drvdata(port);
u32 status;
int ret;
@@ -369,6 +366,11 @@ tps6598x_pr_set(const struct typec_capability *cap, enum typec_role role)
return ret;
}
+static const struct typec_operations tps6598x_ops = {
+ .dr_set = tps6598x_dr_set,
+ .pr_set = tps6598x_pr_set,
+};
+
static irqreturn_t tps6598x_interrupt(int irq, void *data)
{
struct tps6598x *tps = data;
@@ -448,6 +450,7 @@ static const struct regmap_config tps6598x_regmap_config = {
static int tps6598x_probe(struct i2c_client *client)
{
+ struct typec_capability typec_cap = { };
struct tps6598x *tps;
u32 status;
u32 conf;
@@ -492,40 +495,40 @@ static int tps6598x_probe(struct i2c_client *client)
if (ret < 0)
return ret;
- tps->typec_cap.revision = USB_TYPEC_REV_1_2;
- tps->typec_cap.pd_revision = 0x200;
- tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
- tps->typec_cap.pr_set = tps6598x_pr_set;
- tps->typec_cap.dr_set = tps6598x_dr_set;
+ typec_cap.revision = USB_TYPEC_REV_1_2;
+ typec_cap.pd_revision = 0x200;
+ typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ typec_cap.driver_data = tps;
+ typec_cap.ops = &tps6598x_ops;
switch (TPS_SYSCONF_PORTINFO(conf)) {
case TPS_PORTINFO_SINK_ACCESSORY:
case TPS_PORTINFO_SINK:
- tps->typec_cap.type = TYPEC_PORT_SNK;
- tps->typec_cap.data = TYPEC_PORT_UFP;
+ typec_cap.type = TYPEC_PORT_SNK;
+ typec_cap.data = TYPEC_PORT_UFP;
break;
case TPS_PORTINFO_DRP_UFP_DRD:
case TPS_PORTINFO_DRP_DFP_DRD:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_DRD;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DRD;
break;
case TPS_PORTINFO_DRP_UFP:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_UFP;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_UFP;
break;
case TPS_PORTINFO_DRP_DFP:
- tps->typec_cap.type = TYPEC_PORT_DRP;
- tps->typec_cap.data = TYPEC_PORT_DFP;
+ typec_cap.type = TYPEC_PORT_DRP;
+ typec_cap.data = TYPEC_PORT_DFP;
break;
case TPS_PORTINFO_SOURCE:
- tps->typec_cap.type = TYPEC_PORT_SRC;
- tps->typec_cap.data = TYPEC_PORT_DFP;
+ typec_cap.type = TYPEC_PORT_SRC;
+ typec_cap.data = TYPEC_PORT_DFP;
break;
default:
return -ENODEV;
}
- tps->port = typec_register_port(&client->dev, &tps->typec_cap);
+ tps->port = typec_register_port(&client->dev, &typec_cap);
if (IS_ERR(tps->port))
return PTR_ERR(tps->port);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/7] usb: typec: ucsi: Start using struct typec_operations
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
` (4 preceding siblings ...)
2019-10-04 15:08 ` [PATCH v2 5/7] usb: typec: tps6598x: " Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
6 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
Supplying the operation callbacks as part of a struct
typec_operations instead of as part of struct
typec_capability during port registration.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/usb/typec/ucsi/ucsi.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ba288b964dc8..edd722fb88b8 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -17,9 +17,6 @@
#include "ucsi.h"
#include "trace.h"
-#define to_ucsi_connector(_cap_) container_of(_cap_, struct ucsi_connector, \
- typec_cap)
-
/*
* UCSI_TIMEOUT_MS - PPM communication timeout
*
@@ -713,10 +710,9 @@ static int ucsi_role_cmd(struct ucsi_connector *con, struct ucsi_control *ctrl)
return ret;
}
-static int
-ucsi_dr_swap(const struct typec_capability *cap, enum typec_data_role role)
+static int ucsi_dr_swap(struct typec_port *port, enum typec_data_role role)
{
- struct ucsi_connector *con = to_ucsi_connector(cap);
+ struct ucsi_connector *con = typec_get_drvdata(port);
struct ucsi_control ctrl;
int ret = 0;
@@ -748,10 +744,9 @@ ucsi_dr_swap(const struct typec_capability *cap, enum typec_data_role role)
return ret < 0 ? ret : 0;
}
-static int
-ucsi_pr_swap(const struct typec_capability *cap, enum typec_role role)
+static int ucsi_pr_swap(struct typec_port *port, enum typec_role role)
{
- struct ucsi_connector *con = to_ucsi_connector(cap);
+ struct ucsi_connector *con = typec_get_drvdata(port);
struct ucsi_control ctrl;
int ret = 0;
@@ -788,6 +783,11 @@ ucsi_pr_swap(const struct typec_capability *cap, enum typec_role role)
return ret;
}
+static const struct typec_operations ucsi_ops = {
+ .dr_set = ucsi_dr_swap,
+ .pr_set = ucsi_pr_swap
+};
+
static struct fwnode_handle *ucsi_find_fwnode(struct ucsi_connector *con)
{
struct fwnode_handle *fwnode;
@@ -843,8 +843,8 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
*accessory = TYPEC_ACCESSORY_DEBUG;
cap->fwnode = ucsi_find_fwnode(con);
- cap->dr_set = ucsi_dr_swap;
- cap->pr_set = ucsi_pr_swap;
+ cap->driver_data = con;
+ cap->ops = &ucsi_ops;
/* Register the connector */
con->port = typec_register_port(ucsi->dev, cap);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
` (5 preceding siblings ...)
2019-10-04 15:08 ` [PATCH v2 6/7] usb: typec: ucsi: " Heikki Krogerus
@ 2019-10-04 15:08 ` Heikki Krogerus
2019-10-05 17:37 ` Guenter Roeck
6 siblings, 1 reply; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:08 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
There are no more users for them.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/class.c | 38 ++++++++++----------------------------
include/linux/usb/typec.h | 17 -----------------
2 files changed, 10 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index f4972b7ee022..e7b92f650ba0 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -52,7 +52,6 @@ struct typec_port {
struct typec_switch *sw;
struct typec_mux *mux;
- const struct typec_capability *orig_cap; /* to be removed */
const struct typec_capability *cap;
const struct typec_operations *ops;
};
@@ -957,7 +956,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
return -EOPNOTSUPP;
}
- if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {
+ if (!port->ops && !port->ops->try_role) {
dev_dbg(dev, "Setting preferred role not supported\n");
return -EOPNOTSUPP;
}
@@ -970,10 +969,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
return -EINVAL;
}
- if (port->ops && port->ops->try_role)
- ret = port->ops->try_role(port, role);
- else
- ret = port->cap->try_role(port->orig_cap, role);
+ ret = port->ops->try_role(port, role);
if (ret)
return ret;
@@ -1004,7 +1000,7 @@ static ssize_t data_role_store(struct device *dev,
struct typec_port *port = to_typec_port(dev);
int ret;
- if (!port->cap->dr_set || (!port->ops && !port->ops->dr_set)) {
+ if (!port->ops && !port->ops->dr_set) {
dev_dbg(dev, "data role swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1019,10 +1015,7 @@ static ssize_t data_role_store(struct device *dev,
goto unlock_and_ret;
}
- if (port->ops && port->ops->dr_set)
- ret = port->ops->dr_set(port, ret);
- else
- ret = port->cap->dr_set(port->orig_cap, ret);
+ ret = port->ops->dr_set(port, ret);
if (ret)
goto unlock_and_ret;
@@ -1057,7 +1050,7 @@ static ssize_t power_role_store(struct device *dev,
return -EOPNOTSUPP;
}
- if (!port->cap->pr_set || (!port->ops && !port->ops->pr_set)) {
+ if (!port->ops && !port->ops->pr_set) {
dev_dbg(dev, "power role swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1079,10 +1072,7 @@ static ssize_t power_role_store(struct device *dev,
goto unlock_and_ret;
}
- if (port->ops && port->ops->dr_set)
- ret = port->ops->pr_set(port, ret);
- else
- ret = port->cap->pr_set(port->orig_cap, ret);
+ ret = port->ops->pr_set(port, ret);
if (ret)
goto unlock_and_ret;
@@ -1113,7 +1103,7 @@ port_type_store(struct device *dev, struct device_attribute *attr,
int ret;
enum typec_port_type type;
- if (port->cap->type != TYPEC_PORT_DRP || !port->cap->port_type_set ||
+ if (port->cap->type != TYPEC_PORT_DRP ||
(!port->ops && !port->ops->port_type_set)) {
dev_dbg(dev, "changing port type not supported\n");
return -EOPNOTSUPP;
@@ -1131,10 +1121,7 @@ port_type_store(struct device *dev, struct device_attribute *attr,
goto unlock_and_ret;
}
- if (port->ops && port->ops->port_type_set)
- ret = port->ops->port_type_set(port, type);
- else
- ret = port->cap->port_type_set(port->orig_cap, type);
+ ret = port->ops->port_type_set(port, type);
if (ret)
goto unlock_and_ret;
@@ -1190,7 +1177,7 @@ static ssize_t vconn_source_store(struct device *dev,
return -EOPNOTSUPP;
}
- if (!port->cap->vconn_set || (!port->ops && !port->ops->vconn_set)) {
+ if (!port->ops && !port->ops->vconn_set) {
dev_dbg(dev, "VCONN swapping not supported\n");
return -EOPNOTSUPP;
}
@@ -1199,11 +1186,7 @@ static ssize_t vconn_source_store(struct device *dev,
if (ret)
return ret;
- if (port->ops && port->ops->vconn_set)
- ret = port->ops->vconn_set(port, (enum typec_role)source);
- else
- ret = port->cap->vconn_set(port->orig_cap,
- (enum typec_role)source);
+ ret = port->ops->vconn_set(port, (enum typec_role)source);
if (ret)
return ret;
@@ -1610,7 +1593,6 @@ struct typec_port *typec_register_port(struct device *parent,
port->id = id;
port->ops = cap->ops;
- port->orig_cap = cap;
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index c9bef128453b..894798084319 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -198,11 +198,6 @@ struct typec_operations {
* @fwnode: Optional fwnode of the port
* @driver_data: Private pointer for driver specific info
* @ops: Port operations vector
- * @try_role: Set data role preference for DRP port
- * @dr_set: Set Data Role
- * @pr_set: Set Power Role
- * @vconn_set: Set VCONN Role
- * @port_type_set: Set port type
*
* Static capabilities of a single USB Type-C port.
*/
@@ -220,18 +215,6 @@ struct typec_capability {
void *driver_data;
const struct typec_operations *ops;
-
- int (*try_role)(const struct typec_capability *,
- int role);
-
- int (*dr_set)(const struct typec_capability *,
- enum typec_data_role);
- int (*pr_set)(const struct typec_capability *,
- enum typec_role);
- int (*vconn_set)(const struct typec_capability *,
- enum typec_role);
- int (*port_type_set)(const struct typec_capability *,
- enum typec_port_type);
};
/* Specific to try_role(). Indicates the user want's to clear the preference. */
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/7] usb: typec: Separate the operations vector
2019-10-04 15:08 ` [PATCH v2 3/7] usb: typec: Separate the operations vector Heikki Krogerus
@ 2019-10-04 15:17 ` Heikki Krogerus
2019-10-05 17:36 ` Guenter Roeck
1 sibling, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-04 15:17 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
On Fri, Oct 04, 2019 at 06:08:13PM +0300, Heikki Krogerus wrote:
> @@ -1103,7 +1113,8 @@ port_type_store(struct device *dev, struct device_attribute *attr,
> int ret;
> enum typec_port_type type;
>
> - if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
> + if (port->cap->type != TYPEC_PORT_DRP || !port->cap->port_type_set ||
> + (!port->ops && !port->ops->port_type_set)) {
This is still broken :-/
--
heikki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/7] usb: typec: Separate the operations vector
2019-10-04 15:08 ` [PATCH v2 3/7] usb: typec: Separate the operations vector Heikki Krogerus
2019-10-04 15:17 ` Heikki Krogerus
@ 2019-10-05 17:36 ` Guenter Roeck
2019-10-07 8:44 ` Heikki Krogerus
1 sibling, 1 reply; 13+ messages in thread
From: Guenter Roeck @ 2019-10-05 17:36 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Hans de Goede, linux-usb
On 10/4/19 8:08 AM, Heikki Krogerus wrote:
> Introducing struct typec_operations which has the same
> callbacks as struct typec_capability. The old callbacks are
> kept for now, but after all users have been converted, they
> will be removed.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/class.c | 39 +++++++++++++++++++++++++++++----------
> include/linux/usb/typec.h | 20 ++++++++++++++++++++
> 2 files changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 89ffe370e426..f4972b7ee022 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -54,6 +54,7 @@ struct typec_port {
>
> const struct typec_capability *orig_cap; /* to be removed */
> const struct typec_capability *cap;
> + const struct typec_operations *ops;
> };
>
> #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> @@ -956,7 +957,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
> return -EOPNOTSUPP;
> }
>
> - if (!port->cap->try_role) {
> + if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {
Even though it is only temporary, this should be
if (!port->cap->try_role && (!port->ops || !port->ops->try_role)) {
otherwise both cap->try_role and ops->try_role must exist. Also, there would
be a crash if port->cap->try_role and port->ops are both NULL. Same pretty
much everywhere below.
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability
2019-10-04 15:08 ` [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
@ 2019-10-05 17:37 ` Guenter Roeck
0 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2019-10-05 17:37 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Hans de Goede, linux-usb
On 10/4/19 8:08 AM, Heikki Krogerus wrote:
> There are no more users for them.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/class.c | 38 ++++++++++----------------------------
> include/linux/usb/typec.h | 17 -----------------
> 2 files changed, 10 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index f4972b7ee022..e7b92f650ba0 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -52,7 +52,6 @@ struct typec_port {
> struct typec_switch *sw;
> struct typec_mux *mux;
>
> - const struct typec_capability *orig_cap; /* to be removed */
> const struct typec_capability *cap;
> const struct typec_operations *ops;
> };
> @@ -957,7 +956,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
> return -EOPNOTSUPP;
> }
>
> - if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {
> + if (!port->ops && !port->ops->try_role) {
I think this should be
if (!port->ops || !port->ops->try_role) {
or the code would crash if port->ops is NULL. Same everywhere else below.
Guenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations
2019-10-04 15:08 ` [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
@ 2019-10-05 17:39 ` Guenter Roeck
0 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2019-10-05 17:39 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: Hans de Goede, linux-usb
On 10/4/19 8:08 AM, Heikki Krogerus wrote:
> Supplying the operation callbacks as part of a struct
> typec_operations instead of as part of struct
> typec_capability during port registration.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 45 ++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 96562744101c..32b4ce6ce60b 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -390,12 +390,6 @@ static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
> return SRC_UNATTACHED;
> }
>
> -static inline
> -struct tcpm_port *typec_cap_to_tcpm(const struct typec_capability *cap)
> -{
> - return container_of(cap, struct tcpm_port, typec_caps);
> -}
> -
> static bool tcpm_port_is_disconnected(struct tcpm_port *port)
> {
> return (!port->attached && port->cc1 == TYPEC_CC_OPEN &&
> @@ -3970,10 +3964,9 @@ void tcpm_pd_hard_reset(struct tcpm_port *port)
> }
> EXPORT_SYMBOL_GPL(tcpm_pd_hard_reset);
>
> -static int tcpm_dr_set(const struct typec_capability *cap,
> - enum typec_data_role data)
> +static int tcpm_dr_set(struct typec_port *p, enum typec_data_role data)
> {
> - struct tcpm_port *port = typec_cap_to_tcpm(cap);
> + struct tcpm_port *port = typec_get_drvdata(p);
> int ret;
>
> mutex_lock(&port->swap_lock);
> @@ -4038,10 +4031,9 @@ static int tcpm_dr_set(const struct typec_capability *cap,
> return ret;
> }
>
> -static int tcpm_pr_set(const struct typec_capability *cap,
> - enum typec_role role)
> +static int tcpm_pr_set(struct typec_port *p, enum typec_role role)
> {
> - struct tcpm_port *port = typec_cap_to_tcpm(cap);
> + struct tcpm_port *port = typec_get_drvdata(p);
> int ret;
>
> mutex_lock(&port->swap_lock);
> @@ -4082,10 +4074,9 @@ static int tcpm_pr_set(const struct typec_capability *cap,
> return ret;
> }
>
> -static int tcpm_vconn_set(const struct typec_capability *cap,
> - enum typec_role role)
> +static int tcpm_vconn_set(struct typec_port *p, enum typec_role role)
> {
> - struct tcpm_port *port = typec_cap_to_tcpm(cap);
> + struct tcpm_port *port = typec_get_drvdata(p);
> int ret;
>
> mutex_lock(&port->swap_lock);
> @@ -4122,9 +4113,9 @@ static int tcpm_vconn_set(const struct typec_capability *cap,
> return ret;
> }
>
> -static int tcpm_try_role(const struct typec_capability *cap, int role)
> +static int tcpm_try_role(struct typec_port *p, int role)
> {
> - struct tcpm_port *port = typec_cap_to_tcpm(cap);
> + struct tcpm_port *port = typec_get_drvdata(p);
> struct tcpc_dev *tcpc = port->tcpc;
> int ret = 0;
>
> @@ -4331,10 +4322,9 @@ static void tcpm_init(struct tcpm_port *port)
> tcpm_set_state(port, PORT_RESET, 0);
> }
>
> -static int tcpm_port_type_set(const struct typec_capability *cap,
> - enum typec_port_type type)
> +static int tcpm_port_type_set(struct typec_port *p, enum typec_port_type type)
> {
> - struct tcpm_port *port = typec_cap_to_tcpm(cap);
> + struct tcpm_port *port = typec_get_drvdata(p);
>
> mutex_lock(&port->lock);
> if (type == port->port_type)
> @@ -4359,6 +4349,14 @@ static int tcpm_port_type_set(const struct typec_capability *cap,
> return 0;
> }
>
> +static const struct typec_operations tcpm_ops = {
> + .try_role = tcpm_try_role,
> + .dr_set = tcpm_dr_set,
> + .pr_set = tcpm_pr_set,
> + .vconn_set = tcpm_vconn_set,
> + .port_type_set = tcpm_port_type_set
> +};
> +
> void tcpm_tcpc_reset(struct tcpm_port *port)
> {
> mutex_lock(&port->lock);
> @@ -4770,11 +4768,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
> port->typec_caps.fwnode = tcpc->fwnode;
> port->typec_caps.revision = 0x0120; /* Type-C spec release 1.2 */
> port->typec_caps.pd_revision = 0x0300; /* USB-PD spec release 3.0 */
> - port->typec_caps.dr_set = tcpm_dr_set;
> - 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.port_type_set = tcpm_port_type_set;
> + port->typec_caps.driver_data = port;
> + port->typec_caps.ops = &tcpm_ops;
>
> port->partner_desc.identity = &port->partner_ident;
> port->port_type = port->typec_caps.type;
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/7] usb: typec: Separate the operations vector
2019-10-05 17:36 ` Guenter Roeck
@ 2019-10-07 8:44 ` Heikki Krogerus
0 siblings, 0 replies; 13+ messages in thread
From: Heikki Krogerus @ 2019-10-07 8:44 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hans de Goede, linux-usb
On Sat, Oct 05, 2019 at 10:36:02AM -0700, Guenter Roeck wrote:
> On 10/4/19 8:08 AM, Heikki Krogerus wrote:
> > Introducing struct typec_operations which has the same
> > callbacks as struct typec_capability. The old callbacks are
> > kept for now, but after all users have been converted, they
> > will be removed.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > drivers/usb/typec/class.c | 39 +++++++++++++++++++++++++++++----------
> > include/linux/usb/typec.h | 20 ++++++++++++++++++++
> > 2 files changed, 49 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 89ffe370e426..f4972b7ee022 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -54,6 +54,7 @@ struct typec_port {
> > const struct typec_capability *orig_cap; /* to be removed */
> > const struct typec_capability *cap;
> > + const struct typec_operations *ops;
> > };
> > #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> > @@ -956,7 +957,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
> > return -EOPNOTSUPP;
> > }
> > - if (!port->cap->try_role) {
> > + if (!port->cap->try_role || (!port->ops && !port->ops->try_role)) {
>
> Even though it is only temporary, this should be
> if (!port->cap->try_role && (!port->ops || !port->ops->try_role)) {
>
> otherwise both cap->try_role and ops->try_role must exist. Also, there would
> be a crash if port->cap->try_role and port->ops are both NULL. Same pretty
> much everywhere below.
Yes, this series is broken. I'll prepare v3.
I'm going to add two more patches to this series where I'll drop a few
more unused members from struct typec_capability.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-07 8:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04 15:08 [PATCH v2 0/7] usb: typec: Small API improvement Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 1/7] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 2/7] usb: typec: Introduce typec_get_drvdata() Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 3/7] usb: typec: Separate the operations vector Heikki Krogerus
2019-10-04 15:17 ` Heikki Krogerus
2019-10-05 17:36 ` Guenter Roeck
2019-10-07 8:44 ` Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 4/7] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
2019-10-05 17:39 ` Guenter Roeck
2019-10-04 15:08 ` [PATCH v2 5/7] usb: typec: tps6598x: " Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 6/7] usb: typec: ucsi: " Heikki Krogerus
2019-10-04 15:08 ` [PATCH v2 7/7] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
2019-10-05 17:37 ` Guenter Roeck
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).