* [PATCH 1/9] usb: typec: mux: Allow the muxes to be named
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 2/9] usb: typec: mux: Add helpers for setting the mux state Heikki Krogerus
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
The mux devices have been named by using the name of the
parent device as base until now, but if for example the
parent device controls multiple muxes, that will not work.
This makes it possible to supply the name for a mux during
registration.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/mux.c | 6 ++++--
include/linux/usb/typec_mux.h | 2 ++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 5baf0f416c73..3a9970d1d1c0 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -137,7 +137,8 @@ typec_switch_register(struct device *parent,
sw->dev.class = &typec_mux_class;
sw->dev.type = &typec_switch_dev_type;
sw->dev.driver_data = desc->drvdata;
- dev_set_name(&sw->dev, "%s-switch", dev_name(parent));
+ dev_set_name(&sw->dev, "%s-switch",
+ desc->name ? desc->name : dev_name(parent));
ret = device_add(&sw->dev);
if (ret) {
@@ -326,7 +327,8 @@ typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
mux->dev.class = &typec_mux_class;
mux->dev.type = &typec_mux_dev_type;
mux->dev.driver_data = desc->drvdata;
- dev_set_name(&mux->dev, "%s-mux", dev_name(parent));
+ dev_set_name(&mux->dev, "%s-mux",
+ desc->name ? desc->name : dev_name(parent));
ret = device_add(&mux->dev);
if (ret) {
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index be7292c0be5e..47ab5a828b07 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -17,6 +17,7 @@ typedef int (*typec_switch_set_fn_t)(struct typec_switch *sw,
struct typec_switch_desc {
struct fwnode_handle *fwnode;
typec_switch_set_fn_t set;
+ const char *name;
void *drvdata;
};
@@ -42,6 +43,7 @@ typedef int (*typec_mux_set_fn_t)(struct typec_mux *mux,
struct typec_mux_desc {
struct fwnode_handle *fwnode;
typec_mux_set_fn_t set;
+ const char *name;
void *drvdata;
};
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/9] usb: typec: mux: Add helpers for setting the mux state
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
2020-02-13 13:24 ` [PATCH 1/9] usb: typec: mux: Allow the muxes to be named Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 3/9] usb: typec: mux: Allow the mux handles to be requested with fwnode Heikki Krogerus
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
Adding helpers typec_switch_set() and typec_mux_set() that
simply call the ->set callback function of the mux. These
functions make it possible to set the mux states also from
outside the class code.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/class.c | 10 ++++------
drivers/usb/typec/mux.c | 19 +++++++++++++++++++
include/linux/usb/typec_mux.h | 5 +++++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 7c44e930602f..57ef8b91864b 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1495,11 +1495,9 @@ int typec_set_orientation(struct typec_port *port,
{
int ret;
- if (port->sw) {
- ret = port->sw->set(port->sw, orientation);
- if (ret)
- return ret;
- }
+ ret = typec_switch_set(port->sw, orientation);
+ if (ret)
+ return ret;
port->orientation = orientation;
@@ -1533,7 +1531,7 @@ int typec_set_mode(struct typec_port *port, int mode)
state.mode = mode;
- return port->mux ? port->mux->set(port->mux, &state) : 0;
+ return typec_mux_set(port->mux, &state);
}
EXPORT_SYMBOL_GPL(typec_set_mode);
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 3a9970d1d1c0..2b10869f0abd 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -151,6 +151,16 @@ typec_switch_register(struct device *parent,
}
EXPORT_SYMBOL_GPL(typec_switch_register);
+int typec_switch_set(struct typec_switch *sw,
+ enum typec_orientation orientation)
+{
+ if (IS_ERR_OR_NULL(sw))
+ return 0;
+
+ return sw->set(sw, orientation);
+}
+EXPORT_SYMBOL_GPL(typec_switch_set);
+
/**
* typec_switch_unregister - Unregister USB Type-C orientation switch
* @sw: USB Type-C orientation switch
@@ -286,6 +296,15 @@ void typec_mux_put(struct typec_mux *mux)
}
EXPORT_SYMBOL_GPL(typec_mux_put);
+int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
+{
+ if (IS_ERR_OR_NULL(mux))
+ return 0;
+
+ return mux->set(mux, state);
+}
+EXPORT_SYMBOL_GPL(typec_mux_set);
+
static void typec_mux_release(struct device *dev)
{
kfree(to_typec_mux(dev));
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 47ab5a828b07..4991c93df5d0 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -23,6 +23,9 @@ struct typec_switch_desc {
struct typec_switch *typec_switch_get(struct device *dev);
void typec_switch_put(struct typec_switch *sw);
+int typec_switch_set(struct typec_switch *sw,
+ enum typec_orientation orientation);
+
struct typec_switch *
typec_switch_register(struct device *parent,
const struct typec_switch_desc *desc);
@@ -50,6 +53,8 @@ struct typec_mux_desc {
struct typec_mux *
typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc);
void typec_mux_put(struct typec_mux *mux);
+int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
+
struct typec_mux *
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc);
void typec_mux_unregister(struct typec_mux *mux);
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 3/9] usb: typec: mux: Allow the mux handles to be requested with fwnode
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
2020-02-13 13:24 ` [PATCH 1/9] usb: typec: mux: Allow the muxes to be named Heikki Krogerus
2020-02-13 13:24 ` [PATCH 2/9] usb: typec: mux: Add helpers for setting the mux state Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers Heikki Krogerus
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
Introducing fwnode_typec_switch_get() and
fwnode_typec_mux_get() functions that work just like
typec_switch_get() and typec_mux_get() but they take struct
fwnode_handle as the first parameter instead of struct
device.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/mux.c | 22 +++++++++++-----------
include/linux/usb/typec_mux.h | 18 +++++++++++++++---
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 2b10869f0abd..e3a63b33d024 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -59,26 +59,26 @@ static void *typec_switch_match(struct device_connection *con, int ep,
}
/**
- * typec_switch_get - Find USB Type-C orientation switch
- * @dev: The caller device
+ * fwnode_typec_switch_get - Find USB Type-C orientation switch
+ * @fwnode: The caller device node
*
* Finds a switch linked with @dev. Returns a reference to the switch on
* success, NULL if no matching connection was found, or
* ERR_PTR(-EPROBE_DEFER) when a connection was found but the switch
* has not been enumerated yet.
*/
-struct typec_switch *typec_switch_get(struct device *dev)
+struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode)
{
struct typec_switch *sw;
- sw = device_connection_find_match(dev, "orientation-switch", NULL,
+ sw = fwnode_connection_find_match(fwnode, "orientation-switch", NULL,
typec_switch_match);
if (!IS_ERR_OR_NULL(sw))
WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
return sw;
}
-EXPORT_SYMBOL_GPL(typec_switch_get);
+EXPORT_SYMBOL_GPL(fwnode_typec_switch_get);
/**
* typec_put_switch - Release USB Type-C orientation switch
@@ -258,8 +258,8 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
}
/**
- * typec_mux_get - Find USB Type-C Multiplexer
- * @dev: The caller device
+ * fwnode_typec_mux_get - Find USB Type-C Multiplexer
+ * @fwnode: The caller device node
* @desc: Alt Mode description
*
* Finds a mux linked to the caller. This function is primarily meant for the
@@ -267,19 +267,19 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
* matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a connection
* was found but the mux has not been enumerated yet.
*/
-struct typec_mux *typec_mux_get(struct device *dev,
- const struct typec_altmode_desc *desc)
+struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
+ const struct typec_altmode_desc *desc)
{
struct typec_mux *mux;
- mux = device_connection_find_match(dev, "mode-switch", (void *)desc,
+ mux = fwnode_connection_find_match(fwnode, "mode-switch", (void *)desc,
typec_mux_match);
if (!IS_ERR_OR_NULL(mux))
WARN_ON(!try_module_get(mux->dev.parent->driver->owner));
return mux;
}
-EXPORT_SYMBOL_GPL(typec_mux_get);
+EXPORT_SYMBOL_GPL(fwnode_typec_mux_get);
/**
* typec_mux_put - Release handle to a Multiplexer
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 4991c93df5d0..a9d9957933dc 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -3,6 +3,7 @@
#ifndef __USB_TYPEC_MUX
#define __USB_TYPEC_MUX
+#include <linux/property.h>
#include <linux/usb/typec.h>
struct device;
@@ -21,11 +22,16 @@ struct typec_switch_desc {
void *drvdata;
};
-struct typec_switch *typec_switch_get(struct device *dev);
+struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode);
void typec_switch_put(struct typec_switch *sw);
int typec_switch_set(struct typec_switch *sw,
enum typec_orientation orientation);
+static inline struct typec_switch *typec_switch_get(struct device *dev)
+{
+ return fwnode_typec_switch_get(dev_fwnode(dev));
+}
+
struct typec_switch *
typec_switch_register(struct device *parent,
const struct typec_switch_desc *desc);
@@ -50,11 +56,17 @@ struct typec_mux_desc {
void *drvdata;
};
-struct typec_mux *
-typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc);
+struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
+ const struct typec_altmode_desc *desc);
void typec_mux_put(struct typec_mux *mux);
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
+static inline struct typec_mux *
+typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
+{
+ return fwnode_typec_mux_get(dev_fwnode(dev), desc);
+}
+
struct typec_mux *
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc);
void typec_mux_unregister(struct typec_mux *mux);
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (2 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 3/9] usb: typec: mux: Allow the mux handles to be requested with fwnode Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-15 9:19 ` Chunfeng Yun
2020-02-13 13:24 ` [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API Heikki Krogerus
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
Adding usb_role_switch_get/set_drvdata() functions that the
switch drivers can use for setting and getting private data
pointer that is associated with the switch.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/roles/class.c | 22 ++++++++++++++++++++++
include/linux/usb/role.h | 16 ++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index 63a00ff26655..f3132d231599 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -329,6 +329,7 @@ usb_role_switch_register(struct device *parent,
sw->dev.fwnode = desc->fwnode;
sw->dev.class = role_class;
sw->dev.type = &usb_role_dev_type;
+ sw->dev.driver_data = desc->driver_data;
dev_set_name(&sw->dev, "%s-role-switch", dev_name(parent));
ret = device_register(&sw->dev);
@@ -356,6 +357,27 @@ void usb_role_switch_unregister(struct usb_role_switch *sw)
}
EXPORT_SYMBOL_GPL(usb_role_switch_unregister);
+/**
+ * usb_role_switch_set_drvdata - Assign private data pointer to a switch
+ * @sw: USB Role Switch
+ * @data: Private data pointer
+ */
+void usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data)
+{
+ dev_set_drvdata(&sw->dev, data);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_set_drvdata);
+
+/**
+ * usb_role_switch_get_drvdata - Get the private data pointer of a switch
+ * @sw: USB Role Switch
+ */
+void *usb_role_switch_get_drvdata(struct usb_role_switch *sw)
+{
+ return dev_get_drvdata(&sw->dev);
+}
+EXPORT_SYMBOL_GPL(usb_role_switch_get_drvdata);
+
static int __init usb_roles_init(void)
{
role_class = class_create(THIS_MODULE, "usb_role");
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index efac3af83d6b..02dae936cebd 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -25,6 +25,7 @@ typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
* @set: Callback for setting the role
* @get: Callback for getting the role (optional)
* @allow_userspace_control: If true userspace may change the role through sysfs
+ * @driver_data: Private data pointer
*
* @usb2_port and @usb3_port will point to the USB host port and @udc to the USB
* device controller behind the USB connector with the role switch. If
@@ -40,6 +41,7 @@ struct usb_role_switch_desc {
usb_role_switch_set_t set;
usb_role_switch_get_t get;
bool allow_userspace_control;
+ void *driver_data;
};
@@ -57,6 +59,9 @@ struct usb_role_switch *
usb_role_switch_register(struct device *parent,
const struct usb_role_switch_desc *desc);
void usb_role_switch_unregister(struct usb_role_switch *sw);
+
+void usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data);
+void *usb_role_switch_get_drvdata(struct usb_role_switch *sw);
#else
static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
enum usb_role role)
@@ -90,6 +95,17 @@ usb_role_switch_register(struct device *parent,
}
static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
+
+static inline void
+usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data)
+{
+}
+
+static inline void *usb_role_switch_get_drvdata(struct usb_role_switch *sw)
+{
+ return NULL;
+}
+
#endif
#endif /* __LINUX_USB_ROLE_H */
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers
2020-02-13 13:24 ` [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers Heikki Krogerus
@ 2020-02-15 9:19 ` Chunfeng Yun
2020-02-17 9:16 ` Heikki Krogerus
0 siblings, 1 reply; 20+ messages in thread
From: Chunfeng Yun @ 2020-02-15 9:19 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg Kroah-Hartman, Benson Leung, Prashant Malani,
Mika Westerberg, linux-kernel, linux-usb
On Thu, 2020-02-13 at 16:24 +0300, Heikki Krogerus wrote:
> Adding usb_role_switch_get/set_drvdata() functions that the
> switch drivers can use for setting and getting private data
> pointer that is associated with the switch.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/roles/class.c | 22 ++++++++++++++++++++++
> include/linux/usb/role.h | 16 ++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index 63a00ff26655..f3132d231599 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -329,6 +329,7 @@ usb_role_switch_register(struct device *parent,
> sw->dev.fwnode = desc->fwnode;
> sw->dev.class = role_class;
> sw->dev.type = &usb_role_dev_type;
> + sw->dev.driver_data = desc->driver_data;
How about use dev_set_drvdata()? will keep align with
usb_role_switch_set/get_drvdata(),
> dev_set_name(&sw->dev, "%s-role-switch", dev_name(parent));
>
> ret = device_register(&sw->dev);
> @@ -356,6 +357,27 @@ void usb_role_switch_unregister(struct usb_role_switch *sw)
> }
> EXPORT_SYMBOL_GPL(usb_role_switch_unregister);
>
> +/**
> + * usb_role_switch_set_drvdata - Assign private data pointer to a switch
> + * @sw: USB Role Switch
> + * @data: Private data pointer
> + */
> +void usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data)
> +{
> + dev_set_drvdata(&sw->dev, data);
> +}
> +EXPORT_SYMBOL_GPL(usb_role_switch_set_drvdata);
> +
> +/**
> + * usb_role_switch_get_drvdata - Get the private data pointer of a switch
> + * @sw: USB Role Switch
> + */
> +void *usb_role_switch_get_drvdata(struct usb_role_switch *sw)
> +{
> + return dev_get_drvdata(&sw->dev);
> +}
> +EXPORT_SYMBOL_GPL(usb_role_switch_get_drvdata);
> +
> static int __init usb_roles_init(void)
> {
> role_class = class_create(THIS_MODULE, "usb_role");
> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
> index efac3af83d6b..02dae936cebd 100644
> --- a/include/linux/usb/role.h
> +++ b/include/linux/usb/role.h
> @@ -25,6 +25,7 @@ typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
> * @set: Callback for setting the role
> * @get: Callback for getting the role (optional)
> * @allow_userspace_control: If true userspace may change the role through sysfs
> + * @driver_data: Private data pointer
> *
> * @usb2_port and @usb3_port will point to the USB host port and @udc to the USB
> * device controller behind the USB connector with the role switch. If
> @@ -40,6 +41,7 @@ struct usb_role_switch_desc {
> usb_role_switch_set_t set;
> usb_role_switch_get_t get;
> bool allow_userspace_control;
> + void *driver_data;
> };
>
>
> @@ -57,6 +59,9 @@ struct usb_role_switch *
> usb_role_switch_register(struct device *parent,
> const struct usb_role_switch_desc *desc);
> void usb_role_switch_unregister(struct usb_role_switch *sw);
> +
> +void usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data);
> +void *usb_role_switch_get_drvdata(struct usb_role_switch *sw);
> #else
> static inline int usb_role_switch_set_role(struct usb_role_switch *sw,
> enum usb_role role)
> @@ -90,6 +95,17 @@ usb_role_switch_register(struct device *parent,
> }
>
> static inline void usb_role_switch_unregister(struct usb_role_switch *sw) { }
> +
> +static inline void
> +usb_role_switch_set_drvdata(struct usb_role_switch *sw, void *data)
> +{
> +}
> +
> +static inline void *usb_role_switch_get_drvdata(struct usb_role_switch *sw)
> +{
> + return NULL;
> +}
> +
> #endif
>
> #endif /* __LINUX_USB_ROLE_H */
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers
2020-02-15 9:19 ` Chunfeng Yun
@ 2020-02-17 9:16 ` Heikki Krogerus
0 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-17 9:16 UTC (permalink / raw)
To: Chunfeng Yun
Cc: Greg Kroah-Hartman, Benson Leung, Prashant Malani,
Mika Westerberg, linux-kernel, linux-usb
On Sat, Feb 15, 2020 at 05:19:58PM +0800, Chunfeng Yun wrote:
> On Thu, 2020-02-13 at 16:24 +0300, Heikki Krogerus wrote:
> > Adding usb_role_switch_get/set_drvdata() functions that the
> > switch drivers can use for setting and getting private data
> > pointer that is associated with the switch.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > drivers/usb/roles/class.c | 22 ++++++++++++++++++++++
> > include/linux/usb/role.h | 16 ++++++++++++++++
> > 2 files changed, 38 insertions(+)
> >
> > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> > index 63a00ff26655..f3132d231599 100644
> > --- a/drivers/usb/roles/class.c
> > +++ b/drivers/usb/roles/class.c
> > @@ -329,6 +329,7 @@ usb_role_switch_register(struct device *parent,
> > sw->dev.fwnode = desc->fwnode;
> > sw->dev.class = role_class;
> > sw->dev.type = &usb_role_dev_type;
> > + sw->dev.driver_data = desc->driver_data;
> How about use dev_set_drvdata()? will keep align with
> usb_role_switch_set/get_drvdata(),
Yes, makes sense. I'll change that.
Thanks,
--
heikki
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (3 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 4/9] usb: roles: Leave the private driver data pointer to the drivers Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:32 ` Heikki Krogerus
2020-02-15 9:33 ` Chunfeng Yun
2020-02-13 13:24 ` [PATCH 6/9] usb: roles: Allow the role switches to be named Heikki Krogerus
` (3 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
The USB role callback functions had a parameter pointing to
the parent device (struct device) of the switch. The
assumption was that the switch parent is always the
controller. Firstly, that may not be true in every case, and
secondly, it prevents us from supporting devices that supply
multiple muxes.
Changing the first parameter of usb_role_switch_set_t and
usb_role_switch_get_t from struct device to struct
usb_role_switch.
Cc: Peter Chen <Peter.Chen@nxp.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
Cc: Bin Liu <b-liu@ti.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/cdns3/core.c | 10 ++++---
drivers/usb/chipidea/core.c | 10 ++++---
drivers/usb/dwc3/dwc3-meson-g12a.c | 10 ++++---
drivers/usb/gadget/udc/renesas_usb3.c | 26 ++++++++++---------
drivers/usb/gadget/udc/tegra-xudc.c | 8 +++---
drivers/usb/mtu3/mtu3_dr.c | 9 ++++---
drivers/usb/musb/mediatek.c | 9 ++++---
drivers/usb/roles/class.c | 4 +--
.../usb/roles/intel-xhci-usb-role-switch.c | 26 +++++++++++--------
include/linux/usb/role.h | 5 ++--
10 files changed, 67 insertions(+), 50 deletions(-)
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index c2123ef8d8a3..a7a7cb9a2a48 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -330,9 +330,9 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
*
* Returns role
*/
-static enum usb_role cdns3_role_get(struct device *dev)
+static enum usb_role cdns3_role_get(struct usb_role_switch *sw)
{
- struct cdns3 *cdns = dev_get_drvdata(dev);
+ struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
return cdns->role;
}
@@ -346,9 +346,9 @@ static enum usb_role cdns3_role_get(struct device *dev)
* - Role switch for dual-role devices
* - USB_ROLE_GADGET <--> USB_ROLE_NONE for peripheral-only devices
*/
-static int cdns3_role_set(struct device *dev, enum usb_role role)
+static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
{
- struct cdns3 *cdns = dev_get_drvdata(dev);
+ struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
int ret = 0;
pm_runtime_get_sync(cdns->dev);
@@ -536,6 +536,8 @@ static int cdns3_probe(struct platform_device *pdev)
goto err4;
}
+ usb_role_switch_set_drvdata(cdns->role_sw, cdns);
+
ret = cdns3_drd_init(cdns);
if (ret)
goto err5;
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 52139c2a9924..ae0bdc036464 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -600,9 +600,9 @@ static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
return NOTIFY_DONE;
}
-static enum usb_role ci_usb_role_switch_get(struct device *dev)
+static enum usb_role ci_usb_role_switch_get(struct usb_role_switch *sw)
{
- struct ci_hdrc *ci = dev_get_drvdata(dev);
+ struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
enum usb_role role;
unsigned long flags;
@@ -613,9 +613,10 @@ static enum usb_role ci_usb_role_switch_get(struct device *dev)
return role;
}
-static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
+static int ci_usb_role_switch_set(struct usb_role_switch *sw,
+ enum usb_role role)
{
- struct ci_hdrc *ci = dev_get_drvdata(dev);
+ struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
struct ci_hdrc_cable *cable = NULL;
enum usb_role current_role = ci_role_to_usb_role(ci);
enum ci_role ci_role = usb_role_to_ci_role(role);
@@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
}
if (ci_role_switch.fwnode) {
+ ci_role_switch.driver_data = ci;
ci->role_switch = usb_role_switch_register(dev,
&ci_role_switch);
if (IS_ERR(ci->role_switch)) {
diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index 8a3ec1a951fe..3309ce90ca14 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -321,9 +321,10 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
return 0;
}
-static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
+static int dwc3_meson_g12a_role_set(struct usb_role_switch *sw,
+ enum usb_role role)
{
- struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
+ struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
enum phy_mode mode;
if (role == USB_ROLE_NONE)
@@ -338,9 +339,9 @@ static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
return dwc3_meson_g12a_otg_mode_set(priv, mode);
}
-static enum usb_role dwc3_meson_g12a_role_get(struct device *dev)
+static enum usb_role dwc3_meson_g12a_role_get(struct usb_role_switch *sw)
{
- struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
+ struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
return priv->otg_phy_mode == PHY_MODE_USB_HOST ?
USB_ROLE_HOST : USB_ROLE_DEVICE;
@@ -499,6 +500,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
priv->switch_desc.allow_userspace_control = true;
priv->switch_desc.set = dwc3_meson_g12a_role_set;
priv->switch_desc.get = dwc3_meson_g12a_role_get;
+ priv->switch_desc.driver_data = priv;
priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc);
if (IS_ERR(priv->role_switch))
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index c5c3c14df67a..4da90160b400 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2355,14 +2355,14 @@ static const struct usb_gadget_ops renesas_usb3_gadget_ops = {
.set_selfpowered = renesas_usb3_set_selfpowered,
};
-static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
+static enum usb_role renesas_usb3_role_switch_get(struct usb_role_switch *sw)
{
- struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+ struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
enum usb_role cur_role;
- pm_runtime_get_sync(dev);
+ pm_runtime_get_sync(usb3_to_dev(usb3));
cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
- pm_runtime_put(dev);
+ pm_runtime_put(usb3_to_dev(usb3));
return cur_role;
}
@@ -2372,7 +2372,7 @@ static void handle_ext_role_switch_states(struct device *dev,
{
struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
struct device *host = usb3->host_dev;
- enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+ enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
switch (role) {
case USB_ROLE_NONE:
@@ -2424,7 +2424,7 @@ static void handle_role_switch_states(struct device *dev,
{
struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
struct device *host = usb3->host_dev;
- enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+ enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
device_release_driver(host);
@@ -2438,19 +2438,19 @@ static void handle_role_switch_states(struct device *dev,
}
}
-static int renesas_usb3_role_switch_set(struct device *dev,
+static int renesas_usb3_role_switch_set(struct usb_role_switch *sw,
enum usb_role role)
{
- struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+ struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
- pm_runtime_get_sync(dev);
+ pm_runtime_get_sync(usb3_to_dev(usb3));
if (usb3->role_sw_by_connector)
- handle_ext_role_switch_states(dev, role);
+ handle_ext_role_switch_states(usb3_to_dev(usb3), role);
else
- handle_role_switch_states(dev, role);
+ handle_role_switch_states(usb3_to_dev(usb3), role);
- pm_runtime_put(dev);
+ pm_runtime_put(usb3_to_dev(usb3));
return 0;
}
@@ -2831,6 +2831,8 @@ static int renesas_usb3_probe(struct platform_device *pdev)
renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
}
+ renesas_usb3_role_switch_desc.driver_data = usb3;
+
INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
usb3->role_sw = usb_role_switch_register(&pdev->dev,
&renesas_usb3_role_switch_desc);
diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index 634c2c19a176..b9df6369d56d 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -676,12 +676,13 @@ static void tegra_xudc_usb_role_sw_work(struct work_struct *work)
}
-static int tegra_xudc_usb_role_sw_set(struct device *dev, enum usb_role role)
+static int tegra_xudc_usb_role_sw_set(struct usb_role_switch *sw,
+ enum usb_role role)
{
- struct tegra_xudc *xudc = dev_get_drvdata(dev);
+ struct tegra_xudc *xudc = usb_role_switch_get_drvdata(sw);
unsigned long flags;
- dev_dbg(dev, "%s role is %d\n", __func__, role);
+ dev_dbg(xudc->dev, "%s role is %d\n", __func__, role);
spin_lock_irqsave(&xudc->lock, flags);
@@ -3590,6 +3591,7 @@ static int tegra_xudc_probe(struct platform_device *pdev)
if (of_property_read_bool(xudc->dev->of_node, "usb-role-switch")) {
role_sx_desc.set = tegra_xudc_usb_role_sw_set;
role_sx_desc.fwnode = dev_fwnode(xudc->dev);
+ role_sx_desc.driver_data = xudc;
xudc->usb_role_sw = usb_role_switch_register(xudc->dev,
&role_sx_desc);
diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
index 08e18448e8b8..04f666e85731 100644
--- a/drivers/usb/mtu3/mtu3_dr.c
+++ b/drivers/usb/mtu3/mtu3_dr.c
@@ -320,9 +320,9 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
}
-static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
+static int ssusb_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
{
- struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+ struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
bool to_host = false;
if (role == USB_ROLE_HOST)
@@ -334,9 +334,9 @@ static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
return 0;
}
-static enum usb_role ssusb_role_sw_get(struct device *dev)
+static enum usb_role ssusb_role_sw_get(struct usb_role_switch *sw)
{
- struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
+ struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
enum usb_role role;
role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
@@ -356,6 +356,7 @@ static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
role_sx_desc.set = ssusb_role_sw_set;
role_sx_desc.get = ssusb_role_sw_get;
role_sx_desc.fwnode = dev_fwnode(ssusb->dev);
+ role_sx_desc.driver_data = ssusb;
otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
return PTR_ERR_OR_ZERO(otg_sx->role_sw);
diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c
index 6b88c2f5d970..703b98d71180 100644
--- a/drivers/usb/musb/mediatek.c
+++ b/drivers/usb/musb/mediatek.c
@@ -115,9 +115,9 @@ static void mtk_musb_clks_disable(struct mtk_glue *glue)
clk_disable_unprepare(glue->main);
}
-static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
+static int musb_usb_role_sx_set(struct usb_role_switch *sw, enum usb_role role)
{
- struct mtk_glue *glue = dev_get_drvdata(dev);
+ struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
struct musb *musb = glue->musb;
u8 devctl = readb(musb->mregs + MUSB_DEVCTL);
enum usb_role new_role;
@@ -168,9 +168,9 @@ static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
return 0;
}
-static enum usb_role musb_usb_role_sx_get(struct device *dev)
+static enum usb_role musb_usb_role_sx_get(struct usb_role_switch *sw)
{
- struct mtk_glue *glue = dev_get_drvdata(dev);
+ struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
return glue->role;
}
@@ -182,6 +182,7 @@ static int mtk_otg_switch_init(struct mtk_glue *glue)
role_sx_desc.set = musb_usb_role_sx_set;
role_sx_desc.get = musb_usb_role_sx_get;
role_sx_desc.fwnode = dev_fwnode(glue->dev);
+ role_sx_desc.driver_data = glue;
glue->role_sw = usb_role_switch_register(glue->dev, &role_sx_desc);
return PTR_ERR_OR_ZERO(glue->role_sw);
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index f3132d231599..d5e57d26c31f 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -48,7 +48,7 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
mutex_lock(&sw->lock);
- ret = sw->set(sw->dev.parent, role);
+ ret = sw->set(sw, role);
if (!ret)
sw->role = role;
@@ -75,7 +75,7 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
mutex_lock(&sw->lock);
if (sw->get)
- role = sw->get(sw->dev.parent);
+ role = sw->get(sw);
else
role = sw->role;
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 80d6559bbcb2..5c96e929acea 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -42,6 +42,7 @@
#define DRV_NAME "intel_xhci_usb_sw"
struct intel_xhci_usb_data {
+ struct device *dev;
struct usb_role_switch *role_sw;
void __iomem *base;
bool enable_sw_switch;
@@ -51,9 +52,10 @@ static const struct software_node intel_xhci_usb_node = {
"intel-xhci-usb-sw",
};
-static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
+static int intel_xhci_usb_set_role(struct usb_role_switch *sw,
+ enum usb_role role)
{
- struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
+ struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
unsigned long timeout;
acpi_status status;
u32 glk, val;
@@ -66,11 +68,11 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
*/
status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, &glk);
if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
- dev_err(dev, "Error could not acquire lock\n");
+ dev_err(data->dev, "Error could not acquire lock\n");
return -EIO;
}
- pm_runtime_get_sync(dev);
+ pm_runtime_get_sync(data->dev);
/*
* Set idpin value as requested.
@@ -112,7 +114,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
do {
val = readl(data->base + DUAL_ROLE_CFG1);
if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
- pm_runtime_put(dev);
+ pm_runtime_put(data->dev);
return 0;
}
@@ -120,21 +122,21 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
usleep_range(5000, 10000);
} while (time_before(jiffies, timeout));
- pm_runtime_put(dev);
+ pm_runtime_put(data->dev);
- dev_warn(dev, "Timeout waiting for role-switch\n");
+ dev_warn(data->dev, "Timeout waiting for role-switch\n");
return -ETIMEDOUT;
}
-static enum usb_role intel_xhci_usb_get_role(struct device *dev)
+static enum usb_role intel_xhci_usb_get_role(struct usb_role_switch *sw)
{
- struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
+ struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
enum usb_role role;
u32 val;
- pm_runtime_get_sync(dev);
+ pm_runtime_get_sync(data->dev);
val = readl(data->base + DUAL_ROLE_CFG0);
- pm_runtime_put(dev);
+ pm_runtime_put(data->dev);
if (!(val & SW_IDPIN))
role = USB_ROLE_HOST;
@@ -175,7 +177,9 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
sw_desc.get = intel_xhci_usb_get_role,
sw_desc.allow_userspace_control = true,
sw_desc.fwnode = software_node_fwnode(&intel_xhci_usb_node);
+ sw_desc.driver_data = data;
+ data->dev = dev;
data->enable_sw_switch = !device_property_read_bool(dev,
"sw_switch_disable");
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index 02dae936cebd..c028ba8029ad 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -13,8 +13,9 @@ enum usb_role {
USB_ROLE_DEVICE,
};
-typedef int (*usb_role_switch_set_t)(struct device *dev, enum usb_role role);
-typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
+typedef int (*usb_role_switch_set_t)(struct usb_role_switch *sw,
+ enum usb_role role);
+typedef enum usb_role (*usb_role_switch_get_t)(struct usb_role_switch *sw);
/**
* struct usb_role_switch_desc - USB Role Switch Descriptor
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-13 13:24 ` [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API Heikki Krogerus
@ 2020-02-13 13:32 ` Heikki Krogerus
2020-02-18 7:23 ` Peter Chen
2020-02-15 9:33 ` Chunfeng Yun
1 sibling, 1 reply; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:32 UTC (permalink / raw)
To: Peter Chen, Felipe Balbi, Chunfeng Yun, Bin Liu
Cc: Benson Leung, Prashant Malani, Mika Westerberg,
Greg Kroah-Hartman, linux-kernel, linux-usb
Hi guys,
On Thu, Feb 13, 2020 at 04:24:24PM +0300, Heikki Krogerus wrote:
> The USB role callback functions had a parameter pointing to
> the parent device (struct device) of the switch. The
> assumption was that the switch parent is always the
> controller. Firstly, that may not be true in every case, and
> secondly, it prevents us from supporting devices that supply
> multiple muxes.
>
> Changing the first parameter of usb_role_switch_set_t and
> usb_role_switch_get_t from struct device to struct
> usb_role_switch.
>
> Cc: Peter Chen <Peter.Chen@nxp.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Bin Liu <b-liu@ti.com>
This was meant for you guys, but I suppressed CC for everybody. Sorry
about that.
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/cdns3/core.c | 10 ++++---
> drivers/usb/chipidea/core.c | 10 ++++---
> drivers/usb/dwc3/dwc3-meson-g12a.c | 10 ++++---
> drivers/usb/gadget/udc/renesas_usb3.c | 26 ++++++++++---------
> drivers/usb/gadget/udc/tegra-xudc.c | 8 +++---
> drivers/usb/mtu3/mtu3_dr.c | 9 ++++---
> drivers/usb/musb/mediatek.c | 9 ++++---
> drivers/usb/roles/class.c | 4 +--
> .../usb/roles/intel-xhci-usb-role-switch.c | 26 +++++++++++--------
> include/linux/usb/role.h | 5 ++--
> 10 files changed, 67 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index c2123ef8d8a3..a7a7cb9a2a48 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -330,9 +330,9 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
> *
> * Returns role
> */
> -static enum usb_role cdns3_role_get(struct device *dev)
> +static enum usb_role cdns3_role_get(struct usb_role_switch *sw)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
>
> return cdns->role;
> }
> @@ -346,9 +346,9 @@ static enum usb_role cdns3_role_get(struct device *dev)
> * - Role switch for dual-role devices
> * - USB_ROLE_GADGET <--> USB_ROLE_NONE for peripheral-only devices
> */
> -static int cdns3_role_set(struct device *dev, enum usb_role role)
> +static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
> int ret = 0;
>
> pm_runtime_get_sync(cdns->dev);
> @@ -536,6 +536,8 @@ static int cdns3_probe(struct platform_device *pdev)
> goto err4;
> }
>
> + usb_role_switch_set_drvdata(cdns->role_sw, cdns);
> +
> ret = cdns3_drd_init(cdns);
> if (ret)
> goto err5;
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 52139c2a9924..ae0bdc036464 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -600,9 +600,9 @@ static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> return NOTIFY_DONE;
> }
>
> -static enum usb_role ci_usb_role_switch_get(struct device *dev)
> +static enum usb_role ci_usb_role_switch_get(struct usb_role_switch *sw)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
> unsigned long flags;
>
> @@ -613,9 +613,10 @@ static enum usb_role ci_usb_role_switch_get(struct device *dev)
> return role;
> }
>
> -static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> +static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> struct ci_hdrc_cable *cable = NULL;
> enum usb_role current_role = ci_role_to_usb_role(ci);
> enum ci_role ci_role = usb_role_to_ci_role(role);
> @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> }
>
> if (ci_role_switch.fwnode) {
> + ci_role_switch.driver_data = ci;
> ci->role_switch = usb_role_switch_register(dev,
> &ci_role_switch);
> if (IS_ERR(ci->role_switch)) {
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> index 8a3ec1a951fe..3309ce90ca14 100644
> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -321,9 +321,10 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
> return 0;
> }
>
> -static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> +static int dwc3_meson_g12a_role_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
> enum phy_mode mode;
>
> if (role == USB_ROLE_NONE)
> @@ -338,9 +339,9 @@ static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> return dwc3_meson_g12a_otg_mode_set(priv, mode);
> }
>
> -static enum usb_role dwc3_meson_g12a_role_get(struct device *dev)
> +static enum usb_role dwc3_meson_g12a_role_get(struct usb_role_switch *sw)
> {
> - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
>
> return priv->otg_phy_mode == PHY_MODE_USB_HOST ?
> USB_ROLE_HOST : USB_ROLE_DEVICE;
> @@ -499,6 +500,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> priv->switch_desc.allow_userspace_control = true;
> priv->switch_desc.set = dwc3_meson_g12a_role_set;
> priv->switch_desc.get = dwc3_meson_g12a_role_get;
> + priv->switch_desc.driver_data = priv;
>
> priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc);
> if (IS_ERR(priv->role_switch))
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index c5c3c14df67a..4da90160b400 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -2355,14 +2355,14 @@ static const struct usb_gadget_ops renesas_usb3_gadget_ops = {
> .set_selfpowered = renesas_usb3_set_selfpowered,
> };
>
> -static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> +static enum usb_role renesas_usb3_role_switch_get(struct usb_role_switch *sw)
> {
> - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
> enum usb_role cur_role;
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(usb3_to_dev(usb3));
> cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> - pm_runtime_put(dev);
> + pm_runtime_put(usb3_to_dev(usb3));
>
> return cur_role;
> }
> @@ -2372,7 +2372,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
>
> switch (role) {
> case USB_ROLE_NONE:
> @@ -2424,7 +2424,7 @@ static void handle_role_switch_states(struct device *dev,
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
>
> if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> device_release_driver(host);
> @@ -2438,19 +2438,19 @@ static void handle_role_switch_states(struct device *dev,
> }
> }
>
> -static int renesas_usb3_role_switch_set(struct device *dev,
> +static int renesas_usb3_role_switch_set(struct usb_role_switch *sw,
> enum usb_role role)
> {
> - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(usb3_to_dev(usb3));
>
> if (usb3->role_sw_by_connector)
> - handle_ext_role_switch_states(dev, role);
> + handle_ext_role_switch_states(usb3_to_dev(usb3), role);
> else
> - handle_role_switch_states(dev, role);
> + handle_role_switch_states(usb3_to_dev(usb3), role);
>
> - pm_runtime_put(dev);
> + pm_runtime_put(usb3_to_dev(usb3));
>
> return 0;
> }
> @@ -2831,6 +2831,8 @@ static int renesas_usb3_probe(struct platform_device *pdev)
> renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
> }
>
> + renesas_usb3_role_switch_desc.driver_data = usb3;
> +
> INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> usb3->role_sw = usb_role_switch_register(&pdev->dev,
> &renesas_usb3_role_switch_desc);
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index 634c2c19a176..b9df6369d56d 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -676,12 +676,13 @@ static void tegra_xudc_usb_role_sw_work(struct work_struct *work)
>
> }
>
> -static int tegra_xudc_usb_role_sw_set(struct device *dev, enum usb_role role)
> +static int tegra_xudc_usb_role_sw_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct tegra_xudc *xudc = dev_get_drvdata(dev);
> + struct tegra_xudc *xudc = usb_role_switch_get_drvdata(sw);
> unsigned long flags;
>
> - dev_dbg(dev, "%s role is %d\n", __func__, role);
> + dev_dbg(xudc->dev, "%s role is %d\n", __func__, role);
>
> spin_lock_irqsave(&xudc->lock, flags);
>
> @@ -3590,6 +3591,7 @@ static int tegra_xudc_probe(struct platform_device *pdev)
> if (of_property_read_bool(xudc->dev->of_node, "usb-role-switch")) {
> role_sx_desc.set = tegra_xudc_usb_role_sw_set;
> role_sx_desc.fwnode = dev_fwnode(xudc->dev);
> + role_sx_desc.driver_data = xudc;
>
> xudc->usb_role_sw = usb_role_switch_register(xudc->dev,
> &role_sx_desc);
> diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
> index 08e18448e8b8..04f666e85731 100644
> --- a/drivers/usb/mtu3/mtu3_dr.c
> +++ b/drivers/usb/mtu3/mtu3_dr.c
> @@ -320,9 +320,9 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
> mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
> }
>
> -static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> +static int ssusb_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> bool to_host = false;
>
> if (role == USB_ROLE_HOST)
> @@ -334,9 +334,9 @@ static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> return 0;
> }
>
> -static enum usb_role ssusb_role_sw_get(struct device *dev)
> +static enum usb_role ssusb_role_sw_get(struct usb_role_switch *sw)
> {
> - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
>
> role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> @@ -356,6 +356,7 @@ static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
> role_sx_desc.set = ssusb_role_sw_set;
> role_sx_desc.get = ssusb_role_sw_get;
> role_sx_desc.fwnode = dev_fwnode(ssusb->dev);
> + role_sx_desc.driver_data = ssusb;
> otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
>
> return PTR_ERR_OR_ZERO(otg_sx->role_sw);
> diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c
> index 6b88c2f5d970..703b98d71180 100644
> --- a/drivers/usb/musb/mediatek.c
> +++ b/drivers/usb/musb/mediatek.c
> @@ -115,9 +115,9 @@ static void mtk_musb_clks_disable(struct mtk_glue *glue)
> clk_disable_unprepare(glue->main);
> }
>
> -static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> +static int musb_usb_role_sx_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct mtk_glue *glue = dev_get_drvdata(dev);
> + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
> struct musb *musb = glue->musb;
> u8 devctl = readb(musb->mregs + MUSB_DEVCTL);
> enum usb_role new_role;
> @@ -168,9 +168,9 @@ static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> return 0;
> }
>
> -static enum usb_role musb_usb_role_sx_get(struct device *dev)
> +static enum usb_role musb_usb_role_sx_get(struct usb_role_switch *sw)
> {
> - struct mtk_glue *glue = dev_get_drvdata(dev);
> + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
>
> return glue->role;
> }
> @@ -182,6 +182,7 @@ static int mtk_otg_switch_init(struct mtk_glue *glue)
> role_sx_desc.set = musb_usb_role_sx_set;
> role_sx_desc.get = musb_usb_role_sx_get;
> role_sx_desc.fwnode = dev_fwnode(glue->dev);
> + role_sx_desc.driver_data = glue;
> glue->role_sw = usb_role_switch_register(glue->dev, &role_sx_desc);
>
> return PTR_ERR_OR_ZERO(glue->role_sw);
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index f3132d231599..d5e57d26c31f 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -48,7 +48,7 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
>
> mutex_lock(&sw->lock);
>
> - ret = sw->set(sw->dev.parent, role);
> + ret = sw->set(sw, role);
> if (!ret)
> sw->role = role;
>
> @@ -75,7 +75,7 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> mutex_lock(&sw->lock);
>
> if (sw->get)
> - role = sw->get(sw->dev.parent);
> + role = sw->get(sw);
> else
> role = sw->role;
>
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 80d6559bbcb2..5c96e929acea 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -42,6 +42,7 @@
> #define DRV_NAME "intel_xhci_usb_sw"
>
> struct intel_xhci_usb_data {
> + struct device *dev;
> struct usb_role_switch *role_sw;
> void __iomem *base;
> bool enable_sw_switch;
> @@ -51,9 +52,10 @@ static const struct software_node intel_xhci_usb_node = {
> "intel-xhci-usb-sw",
> };
>
> -static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> +static int intel_xhci_usb_set_role(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> unsigned long timeout;
> acpi_status status;
> u32 glk, val;
> @@ -66,11 +68,11 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> */
> status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, &glk);
> if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> - dev_err(dev, "Error could not acquire lock\n");
> + dev_err(data->dev, "Error could not acquire lock\n");
> return -EIO;
> }
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(data->dev);
>
> /*
> * Set idpin value as requested.
> @@ -112,7 +114,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> do {
> val = readl(data->base + DUAL_ROLE_CFG1);
> if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
> return 0;
> }
>
> @@ -120,21 +122,21 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> usleep_range(5000, 10000);
> } while (time_before(jiffies, timeout));
>
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
>
> - dev_warn(dev, "Timeout waiting for role-switch\n");
> + dev_warn(data->dev, "Timeout waiting for role-switch\n");
> return -ETIMEDOUT;
> }
>
> -static enum usb_role intel_xhci_usb_get_role(struct device *dev)
> +static enum usb_role intel_xhci_usb_get_role(struct usb_role_switch *sw)
> {
> - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
> u32 val;
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(data->dev);
> val = readl(data->base + DUAL_ROLE_CFG0);
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
>
> if (!(val & SW_IDPIN))
> role = USB_ROLE_HOST;
> @@ -175,7 +177,9 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
> sw_desc.get = intel_xhci_usb_get_role,
> sw_desc.allow_userspace_control = true,
> sw_desc.fwnode = software_node_fwnode(&intel_xhci_usb_node);
> + sw_desc.driver_data = data;
>
> + data->dev = dev;
> data->enable_sw_switch = !device_property_read_bool(dev,
> "sw_switch_disable");
>
> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
> index 02dae936cebd..c028ba8029ad 100644
> --- a/include/linux/usb/role.h
> +++ b/include/linux/usb/role.h
> @@ -13,8 +13,9 @@ enum usb_role {
> USB_ROLE_DEVICE,
> };
>
> -typedef int (*usb_role_switch_set_t)(struct device *dev, enum usb_role role);
> -typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
> +typedef int (*usb_role_switch_set_t)(struct usb_role_switch *sw,
> + enum usb_role role);
> +typedef enum usb_role (*usb_role_switch_get_t)(struct usb_role_switch *sw);
>
> /**
> * struct usb_role_switch_desc - USB Role Switch Descriptor
> --
> 2.25.0
thanks,
--
heikki
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-13 13:32 ` Heikki Krogerus
@ 2020-02-18 7:23 ` Peter Chen
2020-02-18 12:25 ` Heikki Krogerus
0 siblings, 1 reply; 20+ messages in thread
From: Peter Chen @ 2020-02-18 7:23 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
On 20-02-13 15:32:39, Heikki Krogerus wrote:
> Hi guys,
>
> On Thu, Feb 13, 2020 at 04:24:24PM +0300, Heikki Krogerus wrote:
> > The USB role callback functions had a parameter pointing to
> > the parent device (struct device) of the switch. The
> > assumption was that the switch parent is always the
> > controller. Firstly, that may not be true in every case, and
> > secondly, it prevents us from supporting devices that supply
> > multiple muxes.
> >
> > Changing the first parameter of usb_role_switch_set_t and
> > usb_role_switch_get_t from struct device to struct
> > usb_role_switch.
> >
> > Cc: Peter Chen <Peter.Chen@nxp.com>
> > Cc: Felipe Balbi <balbi@kernel.org>
> > Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > Cc: Bin Liu <b-liu@ti.com>
>
> This was meant for you guys, but I suppressed CC for everybody. Sorry
> about that.
>
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > drivers/usb/cdns3/core.c | 10 ++++---
> > drivers/usb/chipidea/core.c | 10 ++++---
> > drivers/usb/dwc3/dwc3-meson-g12a.c | 10 ++++---
> > drivers/usb/gadget/udc/renesas_usb3.c | 26 ++++++++++---------
> > drivers/usb/gadget/udc/tegra-xudc.c | 8 +++---
> > drivers/usb/mtu3/mtu3_dr.c | 9 ++++---
> > drivers/usb/musb/mediatek.c | 9 ++++---
> > drivers/usb/roles/class.c | 4 +--
> > .../usb/roles/intel-xhci-usb-role-switch.c | 26 +++++++++++--------
> > include/linux/usb/role.h | 5 ++--
> > 10 files changed, 67 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> > index c2123ef8d8a3..a7a7cb9a2a48 100644
> > --- a/drivers/usb/cdns3/core.c
> > +++ b/drivers/usb/cdns3/core.c
> > @@ -330,9 +330,9 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
> > *
> > * Returns role
> > */
> > -static enum usb_role cdns3_role_get(struct device *dev)
> > +static enum usb_role cdns3_role_get(struct usb_role_switch *sw)
> > {
> > - struct cdns3 *cdns = dev_get_drvdata(dev);
> > + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
> >
> > return cdns->role;
> > }
> > @@ -346,9 +346,9 @@ static enum usb_role cdns3_role_get(struct device *dev)
> > * - Role switch for dual-role devices
> > * - USB_ROLE_GADGET <--> USB_ROLE_NONE for peripheral-only devices
> > */
> > -static int cdns3_role_set(struct device *dev, enum usb_role role)
> > +static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
> > {
> > - struct cdns3 *cdns = dev_get_drvdata(dev);
> > + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
> > int ret = 0;
> >
> > pm_runtime_get_sync(cdns->dev);
> > @@ -536,6 +536,8 @@ static int cdns3_probe(struct platform_device *pdev)
> > goto err4;
> > }
> >
> > + usb_role_switch_set_drvdata(cdns->role_sw, cdns);
> > +
> > ret = cdns3_drd_init(cdns);
> > if (ret)
> > goto err5;
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 52139c2a9924..ae0bdc036464 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -600,9 +600,9 @@ static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> > return NOTIFY_DONE;
> > }
> >
> > -static enum usb_role ci_usb_role_switch_get(struct device *dev)
> > +static enum usb_role ci_usb_role_switch_get(struct usb_role_switch *sw)
> > {
> > - struct ci_hdrc *ci = dev_get_drvdata(dev);
> > + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> > enum usb_role role;
> > unsigned long flags;
> >
> > @@ -613,9 +613,10 @@ static enum usb_role ci_usb_role_switch_get(struct device *dev)
> > return role;
> > }
> >
> > -static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> > +static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> > + enum usb_role role)
> > {
> > - struct ci_hdrc *ci = dev_get_drvdata(dev);
> > + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> > struct ci_hdrc_cable *cable = NULL;
> > enum usb_role current_role = ci_role_to_usb_role(ci);
> > enum ci_role ci_role = usb_role_to_ci_role(role);
> > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > }
> >
> > if (ci_role_switch.fwnode) {
> > + ci_role_switch.driver_data = ci;
> > ci->role_switch = usb_role_switch_register(dev,
> > &ci_role_switch);
Why the struct usb_role_switch_desc needs drvdata, the struct
usb_role_switch has already one?
Peter
> > if (IS_ERR(ci->role_switch)) {
> > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> > index 8a3ec1a951fe..3309ce90ca14 100644
> > --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> > @@ -321,9 +321,10 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
> > return 0;
> > }
> >
> > -static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> > +static int dwc3_meson_g12a_role_set(struct usb_role_switch *sw,
> > + enum usb_role role)
> > {
> > - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> > + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
> > enum phy_mode mode;
> >
> > if (role == USB_ROLE_NONE)
> > @@ -338,9 +339,9 @@ static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> > return dwc3_meson_g12a_otg_mode_set(priv, mode);
> > }
> >
> > -static enum usb_role dwc3_meson_g12a_role_get(struct device *dev)
> > +static enum usb_role dwc3_meson_g12a_role_get(struct usb_role_switch *sw)
> > {
> > - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> > + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
> >
> > return priv->otg_phy_mode == PHY_MODE_USB_HOST ?
> > USB_ROLE_HOST : USB_ROLE_DEVICE;
> > @@ -499,6 +500,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> > priv->switch_desc.allow_userspace_control = true;
> > priv->switch_desc.set = dwc3_meson_g12a_role_set;
> > priv->switch_desc.get = dwc3_meson_g12a_role_get;
> > + priv->switch_desc.driver_data = priv;
> >
> > priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc);
> > if (IS_ERR(priv->role_switch))
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> > index c5c3c14df67a..4da90160b400 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -2355,14 +2355,14 @@ static const struct usb_gadget_ops renesas_usb3_gadget_ops = {
> > .set_selfpowered = renesas_usb3_set_selfpowered,
> > };
> >
> > -static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> > +static enum usb_role renesas_usb3_role_switch_get(struct usb_role_switch *sw)
> > {
> > - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
> > enum usb_role cur_role;
> >
> > - pm_runtime_get_sync(dev);
> > + pm_runtime_get_sync(usb3_to_dev(usb3));
> > cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> > - pm_runtime_put(dev);
> > + pm_runtime_put(usb3_to_dev(usb3));
> >
> > return cur_role;
> > }
> > @@ -2372,7 +2372,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> > {
> > struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > struct device *host = usb3->host_dev;
> > - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> > + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
> >
> > switch (role) {
> > case USB_ROLE_NONE:
> > @@ -2424,7 +2424,7 @@ static void handle_role_switch_states(struct device *dev,
> > {
> > struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > struct device *host = usb3->host_dev;
> > - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> > + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
> >
> > if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> > device_release_driver(host);
> > @@ -2438,19 +2438,19 @@ static void handle_role_switch_states(struct device *dev,
> > }
> > }
> >
> > -static int renesas_usb3_role_switch_set(struct device *dev,
> > +static int renesas_usb3_role_switch_set(struct usb_role_switch *sw,
> > enum usb_role role)
> > {
> > - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
> >
> > - pm_runtime_get_sync(dev);
> > + pm_runtime_get_sync(usb3_to_dev(usb3));
> >
> > if (usb3->role_sw_by_connector)
> > - handle_ext_role_switch_states(dev, role);
> > + handle_ext_role_switch_states(usb3_to_dev(usb3), role);
> > else
> > - handle_role_switch_states(dev, role);
> > + handle_role_switch_states(usb3_to_dev(usb3), role);
> >
> > - pm_runtime_put(dev);
> > + pm_runtime_put(usb3_to_dev(usb3));
> >
> > return 0;
> > }
> > @@ -2831,6 +2831,8 @@ static int renesas_usb3_probe(struct platform_device *pdev)
> > renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
> > }
> >
> > + renesas_usb3_role_switch_desc.driver_data = usb3;
> > +
> > INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> > usb3->role_sw = usb_role_switch_register(&pdev->dev,
> > &renesas_usb3_role_switch_desc);
> > diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> > index 634c2c19a176..b9df6369d56d 100644
> > --- a/drivers/usb/gadget/udc/tegra-xudc.c
> > +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> > @@ -676,12 +676,13 @@ static void tegra_xudc_usb_role_sw_work(struct work_struct *work)
> >
> > }
> >
> > -static int tegra_xudc_usb_role_sw_set(struct device *dev, enum usb_role role)
> > +static int tegra_xudc_usb_role_sw_set(struct usb_role_switch *sw,
> > + enum usb_role role)
> > {
> > - struct tegra_xudc *xudc = dev_get_drvdata(dev);
> > + struct tegra_xudc *xudc = usb_role_switch_get_drvdata(sw);
> > unsigned long flags;
> >
> > - dev_dbg(dev, "%s role is %d\n", __func__, role);
> > + dev_dbg(xudc->dev, "%s role is %d\n", __func__, role);
> >
> > spin_lock_irqsave(&xudc->lock, flags);
> >
> > @@ -3590,6 +3591,7 @@ static int tegra_xudc_probe(struct platform_device *pdev)
> > if (of_property_read_bool(xudc->dev->of_node, "usb-role-switch")) {
> > role_sx_desc.set = tegra_xudc_usb_role_sw_set;
> > role_sx_desc.fwnode = dev_fwnode(xudc->dev);
> > + role_sx_desc.driver_data = xudc;
> >
> > xudc->usb_role_sw = usb_role_switch_register(xudc->dev,
> > &role_sx_desc);
> > diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
> > index 08e18448e8b8..04f666e85731 100644
> > --- a/drivers/usb/mtu3/mtu3_dr.c
> > +++ b/drivers/usb/mtu3/mtu3_dr.c
> > @@ -320,9 +320,9 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
> > mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
> > }
> >
> > -static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> > +static int ssusb_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> > {
> > - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> > + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> > bool to_host = false;
> >
> > if (role == USB_ROLE_HOST)
> > @@ -334,9 +334,9 @@ static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> > return 0;
> > }
> >
> > -static enum usb_role ssusb_role_sw_get(struct device *dev)
> > +static enum usb_role ssusb_role_sw_get(struct usb_role_switch *sw)
> > {
> > - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> > + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> > enum usb_role role;
> >
> > role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> > @@ -356,6 +356,7 @@ static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
> > role_sx_desc.set = ssusb_role_sw_set;
> > role_sx_desc.get = ssusb_role_sw_get;
> > role_sx_desc.fwnode = dev_fwnode(ssusb->dev);
> > + role_sx_desc.driver_data = ssusb;
> > otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
> >
> > return PTR_ERR_OR_ZERO(otg_sx->role_sw);
> > diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c
> > index 6b88c2f5d970..703b98d71180 100644
> > --- a/drivers/usb/musb/mediatek.c
> > +++ b/drivers/usb/musb/mediatek.c
> > @@ -115,9 +115,9 @@ static void mtk_musb_clks_disable(struct mtk_glue *glue)
> > clk_disable_unprepare(glue->main);
> > }
> >
> > -static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> > +static int musb_usb_role_sx_set(struct usb_role_switch *sw, enum usb_role role)
> > {
> > - struct mtk_glue *glue = dev_get_drvdata(dev);
> > + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
> > struct musb *musb = glue->musb;
> > u8 devctl = readb(musb->mregs + MUSB_DEVCTL);
> > enum usb_role new_role;
> > @@ -168,9 +168,9 @@ static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> > return 0;
> > }
> >
> > -static enum usb_role musb_usb_role_sx_get(struct device *dev)
> > +static enum usb_role musb_usb_role_sx_get(struct usb_role_switch *sw)
> > {
> > - struct mtk_glue *glue = dev_get_drvdata(dev);
> > + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
> >
> > return glue->role;
> > }
> > @@ -182,6 +182,7 @@ static int mtk_otg_switch_init(struct mtk_glue *glue)
> > role_sx_desc.set = musb_usb_role_sx_set;
> > role_sx_desc.get = musb_usb_role_sx_get;
> > role_sx_desc.fwnode = dev_fwnode(glue->dev);
> > + role_sx_desc.driver_data = glue;
> > glue->role_sw = usb_role_switch_register(glue->dev, &role_sx_desc);
> >
> > return PTR_ERR_OR_ZERO(glue->role_sw);
> > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> > index f3132d231599..d5e57d26c31f 100644
> > --- a/drivers/usb/roles/class.c
> > +++ b/drivers/usb/roles/class.c
> > @@ -48,7 +48,7 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
> >
> > mutex_lock(&sw->lock);
> >
> > - ret = sw->set(sw->dev.parent, role);
> > + ret = sw->set(sw, role);
> > if (!ret)
> > sw->role = role;
> >
> > @@ -75,7 +75,7 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> > mutex_lock(&sw->lock);
> >
> > if (sw->get)
> > - role = sw->get(sw->dev.parent);
> > + role = sw->get(sw);
> > else
> > role = sw->role;
> >
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index 80d6559bbcb2..5c96e929acea 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -42,6 +42,7 @@
> > #define DRV_NAME "intel_xhci_usb_sw"
> >
> > struct intel_xhci_usb_data {
> > + struct device *dev;
> > struct usb_role_switch *role_sw;
> > void __iomem *base;
> > bool enable_sw_switch;
> > @@ -51,9 +52,10 @@ static const struct software_node intel_xhci_usb_node = {
> > "intel-xhci-usb-sw",
> > };
> >
> > -static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > +static int intel_xhci_usb_set_role(struct usb_role_switch *sw,
> > + enum usb_role role)
> > {
> > - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> > + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> > unsigned long timeout;
> > acpi_status status;
> > u32 glk, val;
> > @@ -66,11 +68,11 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > */
> > status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, &glk);
> > if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> > - dev_err(dev, "Error could not acquire lock\n");
> > + dev_err(data->dev, "Error could not acquire lock\n");
> > return -EIO;
> > }
> >
> > - pm_runtime_get_sync(dev);
> > + pm_runtime_get_sync(data->dev);
> >
> > /*
> > * Set idpin value as requested.
> > @@ -112,7 +114,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > do {
> > val = readl(data->base + DUAL_ROLE_CFG1);
> > if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
> > - pm_runtime_put(dev);
> > + pm_runtime_put(data->dev);
> > return 0;
> > }
> >
> > @@ -120,21 +122,21 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> > usleep_range(5000, 10000);
> > } while (time_before(jiffies, timeout));
> >
> > - pm_runtime_put(dev);
> > + pm_runtime_put(data->dev);
> >
> > - dev_warn(dev, "Timeout waiting for role-switch\n");
> > + dev_warn(data->dev, "Timeout waiting for role-switch\n");
> > return -ETIMEDOUT;
> > }
> >
> > -static enum usb_role intel_xhci_usb_get_role(struct device *dev)
> > +static enum usb_role intel_xhci_usb_get_role(struct usb_role_switch *sw)
> > {
> > - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> > + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> > enum usb_role role;
> > u32 val;
> >
> > - pm_runtime_get_sync(dev);
> > + pm_runtime_get_sync(data->dev);
> > val = readl(data->base + DUAL_ROLE_CFG0);
> > - pm_runtime_put(dev);
> > + pm_runtime_put(data->dev);
> >
> > if (!(val & SW_IDPIN))
> > role = USB_ROLE_HOST;
> > @@ -175,7 +177,9 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
> > sw_desc.get = intel_xhci_usb_get_role,
> > sw_desc.allow_userspace_control = true,
> > sw_desc.fwnode = software_node_fwnode(&intel_xhci_usb_node);
> > + sw_desc.driver_data = data;
> >
> > + data->dev = dev;
> > data->enable_sw_switch = !device_property_read_bool(dev,
> > "sw_switch_disable");
> >
> > diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
> > index 02dae936cebd..c028ba8029ad 100644
> > --- a/include/linux/usb/role.h
> > +++ b/include/linux/usb/role.h
> > @@ -13,8 +13,9 @@ enum usb_role {
> > USB_ROLE_DEVICE,
> > };
> >
> > -typedef int (*usb_role_switch_set_t)(struct device *dev, enum usb_role role);
> > -typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
> > +typedef int (*usb_role_switch_set_t)(struct usb_role_switch *sw,
> > + enum usb_role role);
> > +typedef enum usb_role (*usb_role_switch_get_t)(struct usb_role_switch *sw);
> >
> > /**
> > * struct usb_role_switch_desc - USB Role Switch Descriptor
> > --
> > 2.25.0
>
> thanks,
>
> --
> heikki
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-18 7:23 ` Peter Chen
@ 2020-02-18 12:25 ` Heikki Krogerus
2020-02-19 1:58 ` Peter Chen
0 siblings, 1 reply; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-18 12:25 UTC (permalink / raw)
To: Peter Chen
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
Hi,
On Tue, Feb 18, 2020 at 07:23:41AM +0000, Peter Chen wrote:
> > > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > }
> > >
> > > if (ci_role_switch.fwnode) {
> > > + ci_role_switch.driver_data = ci;
> > > ci->role_switch = usb_role_switch_register(dev,
> > > &ci_role_switch);
>
> Why the struct usb_role_switch_desc needs drvdata, the struct
> usb_role_switch has already one?
I'm assuming that you are asking why not just register the switch,
and then call usb_role_switch_set_drvdata(), right?
That may create a race condition where the switch is accessed before
the driver data is available. That can happen for example if the
switch is exposed to the user space.
To play it safe, supplying the driver data as part of the descriptor.
That way we can be sure that the driver data is always available
the moment the switch is registered.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-18 12:25 ` Heikki Krogerus
@ 2020-02-19 1:58 ` Peter Chen
2020-02-19 13:38 ` Heikki Krogerus
0 siblings, 1 reply; 20+ messages in thread
From: Peter Chen @ 2020-02-19 1:58 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
On 20-02-18 14:25:45, Heikki Krogerus wrote:
> Hi,
>
> On Tue, Feb 18, 2020 at 07:23:41AM +0000, Peter Chen wrote:
> > > > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > > }
> > > >
> > > > if (ci_role_switch.fwnode) {
> > > > + ci_role_switch.driver_data = ci;
> > > > ci->role_switch = usb_role_switch_register(dev,
> > > > &ci_role_switch);
> >
> > Why the struct usb_role_switch_desc needs drvdata, the struct
> > usb_role_switch has already one?
>
> I'm assuming that you are asking why not just register the switch,
> and then call usb_role_switch_set_drvdata(), right?
Yes.
>
> That may create a race condition where the switch is accessed before
> the driver data is available. That can happen for example if the
> switch is exposed to the user space.
>
> To play it safe, supplying the driver data as part of the descriptor.
> That way we can be sure that the driver data is always available
> the moment the switch is registered.
>
Then, you may use the uniform way for the driver. Some may have
race condition like you said.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-19 1:58 ` Peter Chen
@ 2020-02-19 13:38 ` Heikki Krogerus
2020-02-19 14:09 ` Peter Chen
0 siblings, 1 reply; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-19 13:38 UTC (permalink / raw)
To: Peter Chen
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
On Wed, Feb 19, 2020 at 01:58:38AM +0000, Peter Chen wrote:
> On 20-02-18 14:25:45, Heikki Krogerus wrote:
> > Hi,
> >
> > On Tue, Feb 18, 2020 at 07:23:41AM +0000, Peter Chen wrote:
> > > > > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > > > }
> > > > >
> > > > > if (ci_role_switch.fwnode) {
> > > > > + ci_role_switch.driver_data = ci;
> > > > > ci->role_switch = usb_role_switch_register(dev,
> > > > > &ci_role_switch);
> > >
> > > Why the struct usb_role_switch_desc needs drvdata, the struct
> > > usb_role_switch has already one?
> >
> > I'm assuming that you are asking why not just register the switch,
> > and then call usb_role_switch_set_drvdata(), right?
>
> Yes.
>
> >
> > That may create a race condition where the switch is accessed before
> > the driver data is available. That can happen for example if the
> > switch is exposed to the user space.
> >
> > To play it safe, supplying the driver data as part of the descriptor.
> > That way we can be sure that the driver data is always available
> > the moment the switch is registered.
> >
>
> Then, you may use the uniform way for the driver. Some may have
> race condition like you said.
Uniform way for the driver?
--
heikki
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-19 13:38 ` Heikki Krogerus
@ 2020-02-19 14:09 ` Peter Chen
2020-02-19 14:14 ` Heikki Krogerus
0 siblings, 1 reply; 20+ messages in thread
From: Peter Chen @ 2020-02-19 14:09 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
On 20-02-19 15:38:15, Heikki Krogerus wrote:
> On Wed, Feb 19, 2020 at 01:58:38AM +0000, Peter Chen wrote:
> > On 20-02-18 14:25:45, Heikki Krogerus wrote:
> > > Hi,
> > >
> > > On Tue, Feb 18, 2020 at 07:23:41AM +0000, Peter Chen wrote:
> > > > > > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > > > > }
> > > > > >
> > > > > > if (ci_role_switch.fwnode) {
> > > > > > + ci_role_switch.driver_data = ci;
> > > > > > ci->role_switch = usb_role_switch_register(dev,
> > > > > > &ci_role_switch);
> > > >
> > > > Why the struct usb_role_switch_desc needs drvdata, the struct
> > > > usb_role_switch has already one?
> > >
> > > I'm assuming that you are asking why not just register the switch,
> > > and then call usb_role_switch_set_drvdata(), right?
> >
> > Yes.
> >
> > >
> > > That may create a race condition where the switch is accessed before
> > > the driver data is available. That can happen for example if the
> > > switch is exposed to the user space.
> > >
> > > To play it safe, supplying the driver data as part of the descriptor.
> > > That way we can be sure that the driver data is always available
> > > the moment the switch is registered.
> > >
> >
> > Then, you may use the uniform way for the driver. Some may have
> > race condition like you said.
>
> Uniform way for the driver?
>
Sorry, unify way. Take chipidea and cdns3 as an example, at chipidea you
use struct usb_role_switch_desc
+ ci_role_switch.driver_data = ci;
ci->role_switch = usb_role_switch_register(dev,
&ci_role_switch);
But at cdns3 side, you set usb_role_switch drvdata directly.
+ usb_role_switch_set_drvdata(cdns->role_sw, cdns);
But according to your comments, all the driver needs to use chipidea's
way to avoid race condition.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-19 14:09 ` Peter Chen
@ 2020-02-19 14:14 ` Heikki Krogerus
0 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-19 14:14 UTC (permalink / raw)
To: Peter Chen
Cc: Felipe Balbi, Chunfeng Yun, Bin Liu, Benson Leung,
Prashant Malani, Mika Westerberg, Greg Kroah-Hartman,
linux-kernel, linux-usb
On Wed, Feb 19, 2020 at 02:09:54PM +0000, Peter Chen wrote:
> On 20-02-19 15:38:15, Heikki Krogerus wrote:
> > On Wed, Feb 19, 2020 at 01:58:38AM +0000, Peter Chen wrote:
> > > On 20-02-18 14:25:45, Heikki Krogerus wrote:
> > > > Hi,
> > > >
> > > > On Tue, Feb 18, 2020 at 07:23:41AM +0000, Peter Chen wrote:
> > > > > > > @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> > > > > > > }
> > > > > > >
> > > > > > > if (ci_role_switch.fwnode) {
> > > > > > > + ci_role_switch.driver_data = ci;
> > > > > > > ci->role_switch = usb_role_switch_register(dev,
> > > > > > > &ci_role_switch);
> > > > >
> > > > > Why the struct usb_role_switch_desc needs drvdata, the struct
> > > > > usb_role_switch has already one?
> > > >
> > > > I'm assuming that you are asking why not just register the switch,
> > > > and then call usb_role_switch_set_drvdata(), right?
> > >
> > > Yes.
> > >
> > > >
> > > > That may create a race condition where the switch is accessed before
> > > > the driver data is available. That can happen for example if the
> > > > switch is exposed to the user space.
> > > >
> > > > To play it safe, supplying the driver data as part of the descriptor.
> > > > That way we can be sure that the driver data is always available
> > > > the moment the switch is registered.
> > > >
> > >
> > > Then, you may use the uniform way for the driver. Some may have
> > > race condition like you said.
> >
> > Uniform way for the driver?
> >
>
> Sorry, unify way. Take chipidea and cdns3 as an example, at chipidea you
> use struct usb_role_switch_desc
>
> + ci_role_switch.driver_data = ci;
> ci->role_switch = usb_role_switch_register(dev,
> &ci_role_switch);
>
> But at cdns3 side, you set usb_role_switch drvdata directly.
> + usb_role_switch_set_drvdata(cdns->role_sw, cdns);
>
> But according to your comments, all the driver needs to use chipidea's
> way to avoid race condition.
OK, I'll fix that.
thanks,
--
heikki
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API
2020-02-13 13:24 ` [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API Heikki Krogerus
2020-02-13 13:32 ` Heikki Krogerus
@ 2020-02-15 9:33 ` Chunfeng Yun
1 sibling, 0 replies; 20+ messages in thread
From: Chunfeng Yun @ 2020-02-15 9:33 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg Kroah-Hartman, Benson Leung, Prashant Malani,
Mika Westerberg, linux-kernel, linux-usb
On Thu, 2020-02-13 at 16:24 +0300, Heikki Krogerus wrote:
> The USB role callback functions had a parameter pointing to
> the parent device (struct device) of the switch. The
> assumption was that the switch parent is always the
> controller. Firstly, that may not be true in every case, and
> secondly, it prevents us from supporting devices that supply
> multiple muxes.
>
> Changing the first parameter of usb_role_switch_set_t and
> usb_role_switch_get_t from struct device to struct
> usb_role_switch.
>
> Cc: Peter Chen <Peter.Chen@nxp.com>
> Cc: Felipe Balbi <balbi@kernel.org>
> Cc: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Cc: Bin Liu <b-liu@ti.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/cdns3/core.c | 10 ++++---
> drivers/usb/chipidea/core.c | 10 ++++---
> drivers/usb/dwc3/dwc3-meson-g12a.c | 10 ++++---
> drivers/usb/gadget/udc/renesas_usb3.c | 26 ++++++++++---------
> drivers/usb/gadget/udc/tegra-xudc.c | 8 +++---
> drivers/usb/mtu3/mtu3_dr.c | 9 ++++---
> drivers/usb/musb/mediatek.c | 9 ++++---
> drivers/usb/roles/class.c | 4 +--
> .../usb/roles/intel-xhci-usb-role-switch.c | 26 +++++++++++--------
> include/linux/usb/role.h | 5 ++--
> 10 files changed, 67 insertions(+), 50 deletions(-)
Looks good to me for mtu3/mtu3_dr.c and musb/mediatek.c
Only build pass, but have no platforms to test it due to n-COV and work
at home now, so
Reviewed-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index c2123ef8d8a3..a7a7cb9a2a48 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -330,9 +330,9 @@ int cdns3_hw_role_switch(struct cdns3 *cdns)
> *
> * Returns role
> */
> -static enum usb_role cdns3_role_get(struct device *dev)
> +static enum usb_role cdns3_role_get(struct usb_role_switch *sw)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
>
> return cdns->role;
> }
> @@ -346,9 +346,9 @@ static enum usb_role cdns3_role_get(struct device *dev)
> * - Role switch for dual-role devices
> * - USB_ROLE_GADGET <--> USB_ROLE_NONE for peripheral-only devices
> */
> -static int cdns3_role_set(struct device *dev, enum usb_role role)
> +static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct cdns3 *cdns = dev_get_drvdata(dev);
> + struct cdns3 *cdns = usb_role_switch_get_drvdata(sw);
> int ret = 0;
>
> pm_runtime_get_sync(cdns->dev);
> @@ -536,6 +536,8 @@ static int cdns3_probe(struct platform_device *pdev)
> goto err4;
> }
>
> + usb_role_switch_set_drvdata(cdns->role_sw, cdns);
> +
> ret = cdns3_drd_init(cdns);
> if (ret)
> goto err5;
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 52139c2a9924..ae0bdc036464 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -600,9 +600,9 @@ static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> return NOTIFY_DONE;
> }
>
> -static enum usb_role ci_usb_role_switch_get(struct device *dev)
> +static enum usb_role ci_usb_role_switch_get(struct usb_role_switch *sw)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
> unsigned long flags;
>
> @@ -613,9 +613,10 @@ static enum usb_role ci_usb_role_switch_get(struct device *dev)
> return role;
> }
>
> -static int ci_usb_role_switch_set(struct device *dev, enum usb_role role)
> +static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct ci_hdrc *ci = dev_get_drvdata(dev);
> + struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> struct ci_hdrc_cable *cable = NULL;
> enum usb_role current_role = ci_role_to_usb_role(ci);
> enum ci_role ci_role = usb_role_to_ci_role(role);
> @@ -1118,6 +1119,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> }
>
> if (ci_role_switch.fwnode) {
> + ci_role_switch.driver_data = ci;
> ci->role_switch = usb_role_switch_register(dev,
> &ci_role_switch);
> if (IS_ERR(ci->role_switch)) {
> diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> index 8a3ec1a951fe..3309ce90ca14 100644
> --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> @@ -321,9 +321,10 @@ static int dwc3_meson_g12a_otg_mode_set(struct dwc3_meson_g12a *priv,
> return 0;
> }
>
> -static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> +static int dwc3_meson_g12a_role_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
> enum phy_mode mode;
>
> if (role == USB_ROLE_NONE)
> @@ -338,9 +339,9 @@ static int dwc3_meson_g12a_role_set(struct device *dev, enum usb_role role)
> return dwc3_meson_g12a_otg_mode_set(priv, mode);
> }
>
> -static enum usb_role dwc3_meson_g12a_role_get(struct device *dev)
> +static enum usb_role dwc3_meson_g12a_role_get(struct usb_role_switch *sw)
> {
> - struct dwc3_meson_g12a *priv = dev_get_drvdata(dev);
> + struct dwc3_meson_g12a *priv = usb_role_switch_get_drvdata(sw);
>
> return priv->otg_phy_mode == PHY_MODE_USB_HOST ?
> USB_ROLE_HOST : USB_ROLE_DEVICE;
> @@ -499,6 +500,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
> priv->switch_desc.allow_userspace_control = true;
> priv->switch_desc.set = dwc3_meson_g12a_role_set;
> priv->switch_desc.get = dwc3_meson_g12a_role_get;
> + priv->switch_desc.driver_data = priv;
>
> priv->role_switch = usb_role_switch_register(dev, &priv->switch_desc);
> if (IS_ERR(priv->role_switch))
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index c5c3c14df67a..4da90160b400 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -2355,14 +2355,14 @@ static const struct usb_gadget_ops renesas_usb3_gadget_ops = {
> .set_selfpowered = renesas_usb3_set_selfpowered,
> };
>
> -static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> +static enum usb_role renesas_usb3_role_switch_get(struct usb_role_switch *sw)
> {
> - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
> enum usb_role cur_role;
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(usb3_to_dev(usb3));
> cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> - pm_runtime_put(dev);
> + pm_runtime_put(usb3_to_dev(usb3));
>
> return cur_role;
> }
> @@ -2372,7 +2372,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
>
> switch (role) {
> case USB_ROLE_NONE:
> @@ -2424,7 +2424,7 @@ static void handle_role_switch_states(struct device *dev,
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> - enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> + enum usb_role cur_role = renesas_usb3_role_switch_get(usb3->role_sw);
>
> if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> device_release_driver(host);
> @@ -2438,19 +2438,19 @@ static void handle_role_switch_states(struct device *dev,
> }
> }
>
> -static int renesas_usb3_role_switch_set(struct device *dev,
> +static int renesas_usb3_role_switch_set(struct usb_role_switch *sw,
> enum usb_role role)
> {
> - struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct renesas_usb3 *usb3 = usb_role_switch_get_drvdata(sw);
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(usb3_to_dev(usb3));
>
> if (usb3->role_sw_by_connector)
> - handle_ext_role_switch_states(dev, role);
> + handle_ext_role_switch_states(usb3_to_dev(usb3), role);
> else
> - handle_role_switch_states(dev, role);
> + handle_role_switch_states(usb3_to_dev(usb3), role);
>
> - pm_runtime_put(dev);
> + pm_runtime_put(usb3_to_dev(usb3));
>
> return 0;
> }
> @@ -2831,6 +2831,8 @@ static int renesas_usb3_probe(struct platform_device *pdev)
> renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
> }
>
> + renesas_usb3_role_switch_desc.driver_data = usb3;
> +
> INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> usb3->role_sw = usb_role_switch_register(&pdev->dev,
> &renesas_usb3_role_switch_desc);
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index 634c2c19a176..b9df6369d56d 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -676,12 +676,13 @@ static void tegra_xudc_usb_role_sw_work(struct work_struct *work)
>
> }
>
> -static int tegra_xudc_usb_role_sw_set(struct device *dev, enum usb_role role)
> +static int tegra_xudc_usb_role_sw_set(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct tegra_xudc *xudc = dev_get_drvdata(dev);
> + struct tegra_xudc *xudc = usb_role_switch_get_drvdata(sw);
> unsigned long flags;
>
> - dev_dbg(dev, "%s role is %d\n", __func__, role);
> + dev_dbg(xudc->dev, "%s role is %d\n", __func__, role);
>
> spin_lock_irqsave(&xudc->lock, flags);
>
> @@ -3590,6 +3591,7 @@ static int tegra_xudc_probe(struct platform_device *pdev)
> if (of_property_read_bool(xudc->dev->of_node, "usb-role-switch")) {
> role_sx_desc.set = tegra_xudc_usb_role_sw_set;
> role_sx_desc.fwnode = dev_fwnode(xudc->dev);
> + role_sx_desc.driver_data = xudc;
>
> xudc->usb_role_sw = usb_role_switch_register(xudc->dev,
> &role_sx_desc);
> diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
> index 08e18448e8b8..04f666e85731 100644
> --- a/drivers/usb/mtu3/mtu3_dr.c
> +++ b/drivers/usb/mtu3/mtu3_dr.c
> @@ -320,9 +320,9 @@ void ssusb_set_force_mode(struct ssusb_mtk *ssusb,
> mtu3_writel(ssusb->ippc_base, SSUSB_U2_CTRL(0), value);
> }
>
> -static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> +static int ssusb_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> bool to_host = false;
>
> if (role == USB_ROLE_HOST)
> @@ -334,9 +334,9 @@ static int ssusb_role_sw_set(struct device *dev, enum usb_role role)
> return 0;
> }
>
> -static enum usb_role ssusb_role_sw_get(struct device *dev)
> +static enum usb_role ssusb_role_sw_get(struct usb_role_switch *sw)
> {
> - struct ssusb_mtk *ssusb = dev_get_drvdata(dev);
> + struct ssusb_mtk *ssusb = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
>
> role = ssusb->is_host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
> @@ -356,6 +356,7 @@ static int ssusb_role_sw_register(struct otg_switch_mtk *otg_sx)
> role_sx_desc.set = ssusb_role_sw_set;
> role_sx_desc.get = ssusb_role_sw_get;
> role_sx_desc.fwnode = dev_fwnode(ssusb->dev);
> + role_sx_desc.driver_data = ssusb;
> otg_sx->role_sw = usb_role_switch_register(ssusb->dev, &role_sx_desc);
>
> return PTR_ERR_OR_ZERO(otg_sx->role_sw);
> diff --git a/drivers/usb/musb/mediatek.c b/drivers/usb/musb/mediatek.c
> index 6b88c2f5d970..703b98d71180 100644
> --- a/drivers/usb/musb/mediatek.c
> +++ b/drivers/usb/musb/mediatek.c
> @@ -115,9 +115,9 @@ static void mtk_musb_clks_disable(struct mtk_glue *glue)
> clk_disable_unprepare(glue->main);
> }
>
> -static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> +static int musb_usb_role_sx_set(struct usb_role_switch *sw, enum usb_role role)
> {
> - struct mtk_glue *glue = dev_get_drvdata(dev);
> + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
> struct musb *musb = glue->musb;
> u8 devctl = readb(musb->mregs + MUSB_DEVCTL);
> enum usb_role new_role;
> @@ -168,9 +168,9 @@ static int musb_usb_role_sx_set(struct device *dev, enum usb_role role)
> return 0;
> }
>
> -static enum usb_role musb_usb_role_sx_get(struct device *dev)
> +static enum usb_role musb_usb_role_sx_get(struct usb_role_switch *sw)
> {
> - struct mtk_glue *glue = dev_get_drvdata(dev);
> + struct mtk_glue *glue = usb_role_switch_get_drvdata(sw);
>
> return glue->role;
> }
> @@ -182,6 +182,7 @@ static int mtk_otg_switch_init(struct mtk_glue *glue)
> role_sx_desc.set = musb_usb_role_sx_set;
> role_sx_desc.get = musb_usb_role_sx_get;
> role_sx_desc.fwnode = dev_fwnode(glue->dev);
> + role_sx_desc.driver_data = glue;
> glue->role_sw = usb_role_switch_register(glue->dev, &role_sx_desc);
>
> return PTR_ERR_OR_ZERO(glue->role_sw);
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index f3132d231599..d5e57d26c31f 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -48,7 +48,7 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
>
> mutex_lock(&sw->lock);
>
> - ret = sw->set(sw->dev.parent, role);
> + ret = sw->set(sw, role);
> if (!ret)
> sw->role = role;
>
> @@ -75,7 +75,7 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
> mutex_lock(&sw->lock);
>
> if (sw->get)
> - role = sw->get(sw->dev.parent);
> + role = sw->get(sw);
> else
> role = sw->role;
>
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 80d6559bbcb2..5c96e929acea 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -42,6 +42,7 @@
> #define DRV_NAME "intel_xhci_usb_sw"
>
> struct intel_xhci_usb_data {
> + struct device *dev;
> struct usb_role_switch *role_sw;
> void __iomem *base;
> bool enable_sw_switch;
> @@ -51,9 +52,10 @@ static const struct software_node intel_xhci_usb_node = {
> "intel-xhci-usb-sw",
> };
>
> -static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> +static int intel_xhci_usb_set_role(struct usb_role_switch *sw,
> + enum usb_role role)
> {
> - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> unsigned long timeout;
> acpi_status status;
> u32 glk, val;
> @@ -66,11 +68,11 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> */
> status = acpi_acquire_global_lock(ACPI_WAIT_FOREVER, &glk);
> if (ACPI_FAILURE(status) && status != AE_NOT_CONFIGURED) {
> - dev_err(dev, "Error could not acquire lock\n");
> + dev_err(data->dev, "Error could not acquire lock\n");
> return -EIO;
> }
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(data->dev);
>
> /*
> * Set idpin value as requested.
> @@ -112,7 +114,7 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> do {
> val = readl(data->base + DUAL_ROLE_CFG1);
> if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
> return 0;
> }
>
> @@ -120,21 +122,21 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
> usleep_range(5000, 10000);
> } while (time_before(jiffies, timeout));
>
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
>
> - dev_warn(dev, "Timeout waiting for role-switch\n");
> + dev_warn(data->dev, "Timeout waiting for role-switch\n");
> return -ETIMEDOUT;
> }
>
> -static enum usb_role intel_xhci_usb_get_role(struct device *dev)
> +static enum usb_role intel_xhci_usb_get_role(struct usb_role_switch *sw)
> {
> - struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
> + struct intel_xhci_usb_data *data = usb_role_switch_get_drvdata(sw);
> enum usb_role role;
> u32 val;
>
> - pm_runtime_get_sync(dev);
> + pm_runtime_get_sync(data->dev);
> val = readl(data->base + DUAL_ROLE_CFG0);
> - pm_runtime_put(dev);
> + pm_runtime_put(data->dev);
>
> if (!(val & SW_IDPIN))
> role = USB_ROLE_HOST;
> @@ -175,7 +177,9 @@ static int intel_xhci_usb_probe(struct platform_device *pdev)
> sw_desc.get = intel_xhci_usb_get_role,
> sw_desc.allow_userspace_control = true,
> sw_desc.fwnode = software_node_fwnode(&intel_xhci_usb_node);
> + sw_desc.driver_data = data;
>
> + data->dev = dev;
> data->enable_sw_switch = !device_property_read_bool(dev,
> "sw_switch_disable");
>
> diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
> index 02dae936cebd..c028ba8029ad 100644
> --- a/include/linux/usb/role.h
> +++ b/include/linux/usb/role.h
> @@ -13,8 +13,9 @@ enum usb_role {
> USB_ROLE_DEVICE,
> };
>
> -typedef int (*usb_role_switch_set_t)(struct device *dev, enum usb_role role);
> -typedef enum usb_role (*usb_role_switch_get_t)(struct device *dev);
> +typedef int (*usb_role_switch_set_t)(struct usb_role_switch *sw,
> + enum usb_role role);
> +typedef enum usb_role (*usb_role_switch_get_t)(struct usb_role_switch *sw);
>
> /**
> * struct usb_role_switch_desc - USB Role Switch Descriptor
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/9] usb: roles: Allow the role switches to be named
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (4 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 5/9] usb: roles: Provide the switch drivers handle to the switch in the API Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 7/9] device property: Export fwnode_get_name() Heikki Krogerus
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
The switch devices have been named by using the name of the
parent device as base for now, but if for example the
parent device controls multiple muxes, that will not work.
Adding an optional member "name" to the switch descriptor
that can be used for naming the switch during registration.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/roles/class.c | 3 ++-
include/linux/usb/role.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index d5e57d26c31f..e17df54f2852 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -330,7 +330,8 @@ usb_role_switch_register(struct device *parent,
sw->dev.class = role_class;
sw->dev.type = &usb_role_dev_type;
sw->dev.driver_data = desc->driver_data;
- dev_set_name(&sw->dev, "%s-role-switch", dev_name(parent));
+ dev_set_name(&sw->dev, "%s-role-switch",
+ desc->name ? desc->name : dev_name(parent));
ret = device_register(&sw->dev);
if (ret) {
diff --git a/include/linux/usb/role.h b/include/linux/usb/role.h
index c028ba8029ad..0164fed31b06 100644
--- a/include/linux/usb/role.h
+++ b/include/linux/usb/role.h
@@ -27,6 +27,7 @@ typedef enum usb_role (*usb_role_switch_get_t)(struct usb_role_switch *sw);
* @get: Callback for getting the role (optional)
* @allow_userspace_control: If true userspace may change the role through sysfs
* @driver_data: Private data pointer
+ * @name: Name for the switch (optional)
*
* @usb2_port and @usb3_port will point to the USB host port and @udc to the USB
* device controller behind the USB connector with the role switch. If
@@ -43,6 +44,7 @@ struct usb_role_switch_desc {
usb_role_switch_get_t get;
bool allow_userspace_control;
void *driver_data;
+ const char *name;
};
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 7/9] device property: Export fwnode_get_name()
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (5 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 6/9] usb: roles: Allow the role switches to be named Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 8/9] usb: typec: Add definitions for Thunderbolt 3 Alternate Mode Heikki Krogerus
2020-02-13 13:24 ` [PATCH 9/9] usb: typec: driver for Intel PMC mux control Heikki Krogerus
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
This makes it possible to take advantage of the function in
the device drivers.
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/base/property.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 511f6d7acdfe..5f35c0ccf5e0 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -566,6 +566,7 @@ const char *fwnode_get_name(const struct fwnode_handle *fwnode)
{
return fwnode_call_ptr_op(fwnode, get_name);
}
+EXPORT_SYMBOL_GPL(fwnode_get_name);
/**
* fwnode_get_name_prefix - Return the prefix of node for printing purposes
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 8/9] usb: typec: Add definitions for Thunderbolt 3 Alternate Mode
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (6 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 7/9] device property: Export fwnode_get_name() Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
2020-02-13 13:24 ` [PATCH 9/9] usb: typec: driver for Intel PMC mux control Heikki Krogerus
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
This adds separate header file for the Thunderbolt 3
Alternate Mode (aka. TBT). The header supplies definitions for
all the Thunderbolt specific VDOs (Vendor Defined Objects)
that are described in the USB Type-C Connector specification
v2.0, as well as definition for the Thunderbolt 3 Standard
ID (SID).
There is also a new connector state value for the
Thunderbolt 3 Alternate Mode that can be used with the mux
drivers.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
include/linux/usb/typec_tbt.h | 53 +++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 include/linux/usb/typec_tbt.h
diff --git a/include/linux/usb/typec_tbt.h b/include/linux/usb/typec_tbt.h
new file mode 100644
index 000000000000..47c2d501ddce
--- /dev/null
+++ b/include/linux/usb/typec_tbt.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __USB_TYPEC_TBT_H
+#define __USB_TYPEC_TBT_H
+
+#include <linux/usb/typec_altmode.h>
+
+#define USB_TYPEC_VENDOR_INTEL 0x8087
+/* Alias for convenience */
+#define USB_TYPEC_TBT_SID USB_TYPEC_VENDOR_INTEL
+
+/* Connector state for Thunderbolt3 */
+#define TYPEC_TBT_MODE TYPEC_STATE_MODAL
+
+/**
+ * struct typec_thunderbolt_data - Thundebolt3 Alt Mode specific data
+ * @device_mode: Device Discover Mode VDO
+ * @cable_mode: Cable Discover Mode VDO
+ * @enter_vdo: Enter Mode VDO
+ */
+struct typec_thunderbolt_data {
+ u32 device_mode;
+ u32 cable_mode;
+ u32 enter_vdo;
+};
+
+/* TBT3 Device Discover Mode VDO bits */
+#define TBT_MODE BIT(0)
+#define TBT_ADAPTER(_vdo_) (((_vdo_) & BIT(16)) >> 16)
+#define TBT_ADAPTER_LEGACY 0
+#define TBT_ADAPTER_TBT3 1
+#define TBT_INTEL_SPECIFIC_B0 BIT(26)
+#define TBT_VENDOR_SPECIFIC_B0 BIT(30)
+#define TBT_VENDOR_SPECIFIC_B1 BIT(31)
+
+#define TBT_SET_ADAPTER(a) (((a) & 1) << 16)
+
+/* TBT3 Cable Discover Mode VDO bits */
+#define TBT_CABLE_SPEED(_vdo_) (((_vdo_) & GENMASK(18, 16)) >> 16)
+#define TBT_CABLE_USB3_GEN1 1
+#define TBT_CABLE_USB3_PASSIVE 2
+#define TBT_CABLE_10_AND_20GBPS 3
+#define TBT_CABLE_ROUNDED BIT(19)
+#define TBT_CABLE_OPTICAL BIT(21)
+#define TBT_CABLE_RETIMER BIT(22)
+#define TBT_CABLE_LINK_TRAINING BIT(23)
+
+#define TBT_SET_CABLE_SPEED(_s_) (((_s_) & GENMASK(2, 0)) << 16)
+
+/* TBT3 Device Enter Mode VDO bits */
+#define TBT_ENTER_MODE_CABLE_SPEED(s) TBT_SET_CABLE_SPEED(s)
+#define TBT_ENTER_MODE_ACTIVE_CABLE BIT(24)
+
+#endif /* __USB_TYPEC_TBT_H */
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 9/9] usb: typec: driver for Intel PMC mux control
2020-02-13 13:24 [PATCH 0/9] usb: typec: Driver for Intel PMC Mux-Agent Heikki Krogerus
` (7 preceding siblings ...)
2020-02-13 13:24 ` [PATCH 8/9] usb: typec: Add definitions for Thunderbolt 3 Alternate Mode Heikki Krogerus
@ 2020-02-13 13:24 ` Heikki Krogerus
8 siblings, 0 replies; 20+ messages in thread
From: Heikki Krogerus @ 2020-02-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Benson Leung, Prashant Malani, Mika Westerberg, linux-kernel, linux-usb
The Intel PMC microcontroller on the latest Intel platforms
has a new function that allows configuration of the USB
Multiplexer/DeMultiplexer switches that are under the
control of the PMC.
The Intel PMC mux control (aka. mux-agent) can be used for
swapping the USB data role and for entering alternate modes,
DisplayPort or Thunderbolt3.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/usb/typec/mux/Kconfig | 9 +
drivers/usb/typec/mux/Makefile | 1 +
drivers/usb/typec/mux/intel_pmc_mux.c | 434 ++++++++++++++++++++++++++
3 files changed, 444 insertions(+)
create mode 100644 drivers/usb/typec/mux/intel_pmc_mux.c
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index 01ed0d5e10e8..77eb97b2aa86 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -9,4 +9,13 @@ config TYPEC_MUX_PI3USB30532
Say Y or M if your system has a Pericom PI3USB30532 Type-C cross
switch / mux chip found on some devices with a Type-C port.
+config TYPEC_MUX_INTEL_PMC
+ tristate "Intel PMC mux control"
+ depends on INTEL_PMC_IPC
+ select USB_ROLE_SWITCH
+ help
+ Driver for USB muxes controlled by Intel PMC FW. Intel PMC FW can
+ control the USB role switch and also the multiplexer/demultiplexer
+ switches used with USB Type-C Alternate Modes.
+
endmenu
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
index 1332e469b8a0..280a6f553115 100644
--- a/drivers/usb/typec/mux/Makefile
+++ b/drivers/usb/typec/mux/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_TYPEC_MUX_PI3USB30532) += pi3usb30532.o
+obj-$(CONFIG_TYPEC_MUX_INTEL_PMC) += intel_pmc_mux.o
diff --git a/drivers/usb/typec/mux/intel_pmc_mux.c b/drivers/usb/typec/mux/intel_pmc_mux.c
new file mode 100644
index 000000000000..f5c5e0aef66f
--- /dev/null
+++ b/drivers/usb/typec/mux/intel_pmc_mux.c
@@ -0,0 +1,434 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Intel PMC USB mux control
+ *
+ * Copyright (C) 2020 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/usb/role.h>
+#include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_dp.h>
+#include <linux/usb/typec_tbt.h>
+
+#include <asm/intel_pmc_ipc.h>
+
+#define PMC_USBC_CMD 0xa7
+
+/* "Usage" OOB Message field values */
+enum {
+ PMC_USB_CONNECT,
+ PMC_USB_DISCONNECT,
+ PMC_USB_SAFE_MODE,
+ PMC_USB_ALT_MODE,
+ PMC_USB_DP_HPD,
+};
+
+#define PMC_USB_MSG_USB2_PORT_SHIFT 0
+#define PMC_USB_MSG_USB3_PORT_SHIFT 4
+#define PMC_USB_MSG_UFP_SHIFT 4
+#define PMC_USB_MSG_ORI_HSL_SHIFT 5
+#define PMC_USB_MSG_ORI_AUX_SHIFT 6
+
+/* Alt Mode Request */
+struct altmode_req {
+ u8 usage;
+ u8 mode_type;
+ u8 mode_id;
+ u8 reserved;
+ u32 mode_data;
+} __packed;
+
+#define PMC_USB_MODE_TYPE_SHIFT 4
+
+enum {
+ PMC_USB_MODE_TYPE_USB,
+ PMC_USB_MODE_TYPE_DP,
+ PMC_USB_MODE_TYPE_TBT,
+};
+
+/* Common Mode Data bits */
+#define PMC_USB_ALTMODE_ACTIVE_CABLE BIT(2)
+
+#define PMC_USB_ALTMODE_ORI_SHIFT 1
+#define PMC_USB_ALTMODE_UFP_SHIFT 3
+#define PMC_USB_ALTMODE_ORI_AUX_SHIFT 4
+#define PMC_USB_ALTMODE_ORI_HSL_SHIFT 5
+
+/* DP specific Mode Data bits */
+#define PMC_USB_ALTMODE_DP_MODE_SHIFT 8
+
+/* TBT specific Mode Data bits */
+#define PMC_USB_ALTMODE_TBT_TYPE BIT(17)
+#define PMC_USB_ALTMODE_CABLE_TYPE BIT(18)
+#define PMC_USB_ALTMODE_ACTIVE_LINK BIT(20)
+#define PMC_USB_ALTMODE_FORCE_LSR BIT(23)
+#define PMC_USB_ALTMODE_CABLE_SPD(_s_) (((_s_) & GENMASK(2, 0)) << 25)
+#define PMC_USB_ALTMODE_CABLE_USB31 1
+#define PMC_USB_ALTMODE_CABLE_10GPS 2
+#define PMC_USB_ALTMODE_CABLE_20GPS 3
+#define PMC_USB_ALTMODE_TBT_GEN(_g_) (((_g_) & GENMASK(1, 0)) << 28)
+
+/* Display HPD Request bits */
+#define PMC_USB_DP_HPD_IRQ BIT(5)
+#define PMC_USB_DP_HPD_LVL BIT(6)
+
+struct pmc_usb;
+
+struct pmc_usb_port {
+ int num;
+ struct pmc_usb *pmc;
+ struct typec_mux *typec_mux;
+ struct typec_switch *typec_sw;
+ struct usb_role_switch *usb_sw;
+
+ enum typec_orientation orientation;
+ enum usb_role role;
+
+ u8 usb2_port;
+ u8 usb3_port;
+};
+
+struct pmc_usb {
+ u8 num_ports;
+ struct device *dev;
+ struct pmc_usb_port *port;
+};
+
+static int pmc_usb_command(struct pmc_usb_port *port, u8 *msg, u32 len)
+{
+ u8 response[4];
+
+ /*
+ * Error bit will always be 0 with the USBC command.
+ * Status can be checked from the response message.
+ */
+ intel_pmc_ipc_command(PMC_USBC_CMD, 0, msg, len,
+ (void *)response, 1);
+
+ if (response[2]) {
+ if (response[2] & BIT(1))
+ return -EIO;
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int
+pmc_usb_mux_dp_hpd(struct pmc_usb_port *port, struct typec_mux_state *state)
+{
+ struct typec_displayport_data *data = state->data;
+ u8 msg[2] = { };
+
+ msg[0] = PMC_USB_DP_HPD;
+ msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+
+ msg[1] = PMC_USB_DP_HPD_IRQ;
+
+ if (data->status & DP_STATUS_HPD_STATE)
+ msg[1] |= PMC_USB_DP_HPD_LVL;
+
+ return pmc_usb_command(port, msg, sizeof(msg));
+}
+
+static int
+pmc_usb_mux_dp(struct pmc_usb_port *port, struct typec_mux_state *state)
+{
+ struct typec_displayport_data *data = state->data;
+ struct altmode_req req = { };
+
+ if (data->status & DP_STATUS_IRQ_HPD)
+ return pmc_usb_mux_dp_hpd(port, state);
+
+ req.usage = PMC_USB_ALT_MODE;
+ req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+ req.mode_type = PMC_USB_MODE_TYPE_DP << PMC_USB_MODE_TYPE_SHIFT;
+
+ req.mode_data = (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
+ req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
+ req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
+ req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
+
+ req.mode_data |= (state->mode - TYPEC_STATE_MODAL) <<
+ PMC_USB_ALTMODE_DP_MODE_SHIFT;
+
+ return pmc_usb_command(port, (void *)&req, sizeof(req));
+}
+
+static int
+pmc_usb_mux_tbt(struct pmc_usb_port *port, struct typec_mux_state *state)
+{
+ struct typec_thunderbolt_data *data = state->data;
+ u8 cable_speed = TBT_CABLE_SPEED(data->cable_mode);
+ struct altmode_req req = { };
+
+ req.usage = PMC_USB_ALT_MODE;
+ req.usage |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+ req.mode_type = PMC_USB_MODE_TYPE_TBT << PMC_USB_MODE_TYPE_SHIFT;
+
+ req.mode_data = (port->orientation - 1) << PMC_USB_ALTMODE_ORI_SHIFT;
+ req.mode_data |= (port->role - 1) << PMC_USB_ALTMODE_UFP_SHIFT;
+ req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_AUX_SHIFT;
+ req.mode_data |= (port->orientation - 1) << PMC_USB_ALTMODE_ORI_HSL_SHIFT;
+
+ if (TBT_ADAPTER(data->device_mode) == TBT_ADAPTER_TBT3)
+ req.mode_data |= PMC_USB_ALTMODE_TBT_TYPE;
+
+ if (data->cable_mode & TBT_CABLE_OPTICAL)
+ req.mode_data |= PMC_USB_ALTMODE_CABLE_TYPE;
+
+ if (data->cable_mode & TBT_CABLE_LINK_TRAINING)
+ req.mode_data |= PMC_USB_ALTMODE_ACTIVE_LINK;
+
+ if (data->enter_vdo & TBT_ENTER_MODE_ACTIVE_CABLE)
+ req.mode_data |= PMC_USB_ALTMODE_ACTIVE_CABLE;
+
+ req.mode_data |= PMC_USB_ALTMODE_CABLE_SPD(cable_speed);
+
+ return pmc_usb_command(port, (void *)&req, sizeof(req));
+}
+
+static int pmc_usb_mux_safe_state(struct pmc_usb_port *port)
+{
+ u8 msg;
+
+ msg = PMC_USB_SAFE_MODE;
+ msg |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+
+ return pmc_usb_command(port, &msg, sizeof(msg));
+}
+
+static int pmc_usb_connect(struct pmc_usb_port *port)
+{
+ u8 msg[2];
+
+ msg[0] = PMC_USB_CONNECT;
+ msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+
+ msg[1] = port->usb2_port << PMC_USB_MSG_USB2_PORT_SHIFT;
+ msg[1] |= (port->orientation - 1) << PMC_USB_MSG_ORI_HSL_SHIFT;
+ msg[1] |= (port->orientation - 1) << PMC_USB_MSG_ORI_AUX_SHIFT;
+
+ return pmc_usb_command(port, msg, sizeof(msg));
+}
+
+static int pmc_usb_disconnect(struct pmc_usb_port *port)
+{
+ u8 msg[2];
+
+ msg[0] = PMC_USB_DISCONNECT;
+ msg[0] |= port->usb3_port << PMC_USB_MSG_USB3_PORT_SHIFT;
+
+ msg[1] = port->usb2_port << PMC_USB_MSG_USB2_PORT_SHIFT;
+
+ return pmc_usb_command(port, msg, sizeof(msg));
+}
+
+static int
+pmc_usb_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
+{
+ struct pmc_usb_port *port = typec_mux_get_drvdata(mux);
+
+ if (!state->alt)
+ return 0;
+
+ if (state->mode == TYPEC_STATE_SAFE)
+ return pmc_usb_mux_safe_state(port);
+
+ switch (state->alt->svid) {
+ case USB_TYPEC_TBT_SID:
+ return pmc_usb_mux_tbt(port, state);
+ case USB_TYPEC_DP_SID:
+ return pmc_usb_mux_dp(port, state);
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int pmc_usb_set_orientation(struct typec_switch *sw,
+ enum typec_orientation orientation)
+{
+ struct pmc_usb_port *port = typec_switch_get_drvdata(sw);
+
+ if (port->orientation == orientation)
+ return 0;
+
+ port->orientation = orientation;
+
+ if (port->role) {
+ if (orientation == TYPEC_ORIENTATION_NONE)
+ return pmc_usb_disconnect(port);
+ else
+ return pmc_usb_connect(port);
+ }
+
+ return 0;
+}
+
+static int pmc_usb_set_role(struct usb_role_switch *sw, enum usb_role role)
+{
+ struct pmc_usb_port *port = usb_role_switch_get_drvdata(sw);
+
+ if (port->role == role)
+ return 0;
+
+ port->role = role;
+
+ if (port->orientation) {
+ if (role == USB_ROLE_NONE)
+ return pmc_usb_disconnect(port);
+ else
+ return pmc_usb_connect(port);
+ }
+
+ return 0;
+}
+
+static int pmc_usb_register_port(struct pmc_usb *pmc, int index,
+ struct fwnode_handle *fwnode)
+{
+ struct pmc_usb_port *port = &pmc->port[index];
+ struct usb_role_switch_desc desc = { };
+ struct typec_switch_desc sw_desc = { };
+ struct typec_mux_desc mux_desc = { };
+ int ret;
+
+ ret = fwnode_property_read_u8(fwnode, "usb2-port", &port->usb2_port);
+ if (ret)
+ return ret;
+
+ ret = fwnode_property_read_u8(fwnode, "usb3-port", &port->usb3_port);
+ if (ret)
+ return ret;
+
+ port->num = index;
+ port->pmc = pmc;
+
+ sw_desc.fwnode = fwnode;
+ sw_desc.drvdata = port;
+ sw_desc.name = fwnode_get_name(fwnode);
+ sw_desc.set = pmc_usb_set_orientation;
+
+ port->typec_sw = typec_switch_register(pmc->dev, &sw_desc);
+ if (IS_ERR(port->typec_sw))
+ return PTR_ERR(port->typec_sw);
+
+ mux_desc.fwnode = fwnode;
+ mux_desc.drvdata = port;
+ mux_desc.name = fwnode_get_name(fwnode);
+ mux_desc.set = pmc_usb_mux_set;
+
+ port->typec_mux = typec_mux_register(pmc->dev, &mux_desc);
+ if (IS_ERR(port->typec_mux)) {
+ ret = PTR_ERR(port->typec_mux);
+ goto err_unregister_switch;
+ }
+
+ desc.fwnode = fwnode;
+ desc.driver_data = port;
+ desc.name = fwnode_get_name(fwnode);
+ desc.set = pmc_usb_set_role;
+
+ port->usb_sw = usb_role_switch_register(pmc->dev, &desc);
+ if (IS_ERR(port->usb_sw)) {
+ ret = PTR_ERR(port->usb_sw);
+ goto err_unregister_mux;
+ }
+
+ return 0;
+
+err_unregister_mux:
+ typec_mux_unregister(port->typec_mux);
+
+err_unregister_switch:
+ typec_switch_unregister(port->typec_sw);
+
+ return ret;
+}
+
+static int pmc_usb_probe(struct platform_device *pdev)
+{
+ struct fwnode_handle *fwnode = NULL;
+ struct pmc_usb *pmc;
+ int i = 0;
+ int ret;
+
+ pmc = devm_kzalloc(&pdev->dev, sizeof(*pmc), GFP_KERNEL);
+ if (!pmc)
+ return -ENOMEM;
+
+ device_for_each_child_node(&pdev->dev, fwnode)
+ pmc->num_ports++;
+
+ pmc->port = devm_kcalloc(&pdev->dev, pmc->num_ports,
+ sizeof(struct pmc_usb_port), GFP_KERNEL);
+ if (!pmc->port)
+ return -ENOMEM;
+
+ pmc->dev = &pdev->dev;
+
+ /*
+ * For every physical USB connector (USB2 and USB3 combo) there is a
+ * child ACPI device node under the PMC mux ACPI device object.
+ */
+ for (i = 0; i < pmc->num_ports; i++) {
+ fwnode = device_get_next_child_node(pmc->dev, fwnode);
+ if (!fwnode)
+ break;
+
+ ret = pmc_usb_register_port(pmc, i, fwnode);
+ if (ret)
+ goto err_remove_ports;
+ }
+
+ platform_set_drvdata(pdev, pmc);
+
+ return 0;
+
+err_remove_ports:
+ for (i = 0; i < pmc->num_ports; i++) {
+ typec_switch_unregister(pmc->port[i].typec_sw);
+ typec_mux_unregister(pmc->port[i].typec_mux);
+ }
+
+ return ret;
+}
+
+static int pmc_usb_remove(struct platform_device *pdev)
+{
+ struct pmc_usb *pmc = platform_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i < pmc->num_ports; i++) {
+ typec_switch_unregister(pmc->port[i].typec_sw);
+ typec_mux_unregister(pmc->port[i].typec_mux);
+ }
+
+ return 0;
+}
+
+static const struct acpi_device_id pmc_usb_acpi_ids[] = {
+ { "INTC105C", },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, pmc_usb_acpi_ids);
+
+static struct platform_driver pmc_usb_driver = {
+ .driver = {
+ .name = "intel_pmc_usb",
+ .acpi_match_table = ACPI_PTR(pmc_usb_acpi_ids),
+ },
+ .probe = pmc_usb_probe,
+ .remove = pmc_usb_remove,
+};
+
+module_platform_driver(pmc_usb_driver);
+
+MODULE_AUTHOR("Heikki Krogerus <heikki.krogerus@linux.intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel PMC USB mux control");
--
2.25.0
^ permalink raw reply related [flat|nested] 20+ messages in thread