Hi,
Adding a simple function typec_link_port() that can be used to create
a symlink "connector" that points to the USB Type-C connector of a
port. It is used with USB ports initially, but hopefully later also
with other things like DisplayPorts.
Being able to see which connector is connected to a port is important
in general, but it is really important when for example the data or
power role of a device needs to swapped. The user probable wants to
know which USB device is disconnected if role swap on a USB Type-C
connector is executed.
Hope these are OK.
thanks,
Heikki Krogerus (6):
usb: Iterator for ports
usb: typec: Organize the private headers properly
usb: typec: Declare the typec_class static
usb: typec: Port mapping utility
usb: Link the ports to the connectors they are attached to
usb: typec: Link all ports during connector registration
Documentation/ABI/testing/sysfs-bus-usb | 9 +
drivers/usb/core/port.c | 3 +
drivers/usb/core/usb.c | 43 ++++
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/bus.c | 2 +
drivers/usb/typec/bus.h | 19 +-
drivers/usb/typec/class.c | 101 +++------
drivers/usb/typec/class.h | 94 ++++++++
drivers/usb/typec/mux.c | 4 +-
drivers/usb/typec/mux.h | 21 ++
drivers/usb/typec/port-mapper.c | 283 ++++++++++++++++++++++++
include/linux/usb.h | 1 +
include/linux/usb/typec.h | 13 ++
13 files changed, 499 insertions(+), 95 deletions(-)
create mode 100644 drivers/usb/typec/class.h
create mode 100644 drivers/usb/typec/mux.h
create mode 100644 drivers/usb/typec/port-mapper.c
--
2.30.2
Adding a header file for each subsystem - the connector
class, alt mode bus and the class for the muxes.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/bus.c | 2 ++
drivers/usb/typec/bus.h | 19 +---------
drivers/usb/typec/class.c | 69 +++--------------------------------
drivers/usb/typec/class.h | 76 +++++++++++++++++++++++++++++++++++++++
drivers/usb/typec/mux.c | 4 +--
drivers/usb/typec/mux.h | 21 +++++++++++
6 files changed, 107 insertions(+), 84 deletions(-)
create mode 100644 drivers/usb/typec/class.h
create mode 100644 drivers/usb/typec/mux.h
diff --git a/drivers/usb/typec/bus.c b/drivers/usb/typec/bus.c
index e8ddb81cb6df4..7f3c9a8e2bf08 100644
--- a/drivers/usb/typec/bus.c
+++ b/drivers/usb/typec/bus.c
@@ -9,6 +9,8 @@
#include <linux/usb/pd_vdo.h>
#include "bus.h"
+#include "class.h"
+#include "mux.h"
static inline int
typec_altmode_set_mux(struct altmode *alt, unsigned long conf, void *data)
diff --git a/drivers/usb/typec/bus.h b/drivers/usb/typec/bus.h
index 8ba8112d2740d..56dec268d4dd9 100644
--- a/drivers/usb/typec/bus.h
+++ b/drivers/usb/typec/bus.h
@@ -4,9 +4,9 @@
#define __USB_TYPEC_ALTMODE_H__
#include <linux/usb/typec_altmode.h>
-#include <linux/usb/typec_mux.h>
struct bus_type;
+struct typec_mux;
struct altmode {
unsigned int id;
@@ -28,24 +28,7 @@ struct altmode {
extern struct bus_type typec_bus;
extern const struct device_type typec_altmode_dev_type;
-extern const struct device_type typec_port_dev_type;
#define is_typec_altmode(_dev_) (_dev_->type == &typec_altmode_dev_type)
-#define is_typec_port(_dev_) (_dev_->type == &typec_port_dev_type)
-
-extern struct class typec_mux_class;
-
-struct typec_switch {
- struct device dev;
- typec_switch_set_fn_t set;
-};
-
-struct typec_mux {
- struct device dev;
- typec_mux_set_fn_t set;
-};
-
-#define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev)
-#define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev)
#endif /* __USB_TYPEC_ALTMODE_H__ */
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 45f0bf65e9aba..5fa279a96b6ef 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -6,74 +6,15 @@
* Author: Heikki Krogerus <[email protected]>
*/
-#include <linux/device.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/slab.h>
#include <linux/usb/pd_vdo.h>
+#include <linux/usb/typec_mux.h>
#include "bus.h"
-
-struct typec_plug {
- struct device dev;
- enum typec_plug_index index;
- struct ida mode_ids;
- int num_altmodes;
-};
-
-struct typec_cable {
- struct device dev;
- enum typec_plug_type type;
- struct usb_pd_identity *identity;
- unsigned int active:1;
- u16 pd_revision; /* 0300H = "3.0" */
-};
-
-struct typec_partner {
- struct device dev;
- unsigned int usb_pd:1;
- struct usb_pd_identity *identity;
- enum typec_accessory accessory;
- struct ida mode_ids;
- int num_altmodes;
- u16 pd_revision; /* 0300H = "3.0" */
- enum usb_pd_svdm_ver svdm_version;
-};
-
-struct typec_port {
- unsigned int id;
- struct device dev;
- struct ida mode_ids;
-
- int prefer_role;
- enum typec_data_role data_role;
- enum typec_role pwr_role;
- enum typec_role vconn_role;
- enum typec_pwr_opmode pwr_opmode;
- enum typec_port_type port_type;
- struct mutex port_type_lock;
-
- enum typec_orientation orientation;
- struct typec_switch *sw;
- struct typec_mux *mux;
-
- const struct typec_capability *cap;
- const struct typec_operations *ops;
-};
-
-#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
-#define to_typec_plug(_dev_) container_of(_dev_, struct typec_plug, dev)
-#define to_typec_cable(_dev_) container_of(_dev_, struct typec_cable, dev)
-#define to_typec_partner(_dev_) container_of(_dev_, struct typec_partner, dev)
-
-static const struct device_type typec_partner_dev_type;
-static const struct device_type typec_cable_dev_type;
-static const struct device_type typec_plug_dev_type;
-
-#define is_typec_partner(_dev_) (_dev_->type == &typec_partner_dev_type)
-#define is_typec_cable(_dev_) (_dev_->type == &typec_cable_dev_type)
-#define is_typec_plug(_dev_) (_dev_->type == &typec_plug_dev_type)
+#include "class.h"
static DEFINE_IDA(typec_index_ida);
static struct class *typec_class;
@@ -726,7 +667,7 @@ static void typec_partner_release(struct device *dev)
kfree(partner);
}
-static const struct device_type typec_partner_dev_type = {
+const struct device_type typec_partner_dev_type = {
.name = "typec_partner",
.groups = typec_partner_groups,
.release = typec_partner_release,
@@ -941,7 +882,7 @@ static const struct attribute_group *typec_plug_groups[] = {
NULL
};
-static const struct device_type typec_plug_dev_type = {
+const struct device_type typec_plug_dev_type = {
.name = "typec_plug",
.groups = typec_plug_groups,
.release = typec_plug_release,
@@ -1089,7 +1030,7 @@ static void typec_cable_release(struct device *dev)
kfree(cable);
}
-static const struct device_type typec_cable_dev_type = {
+const struct device_type typec_cable_dev_type = {
.name = "typec_cable",
.groups = typec_cable_groups,
.release = typec_cable_release,
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
new file mode 100644
index 0000000000000..d414be58d122e
--- /dev/null
+++ b/drivers/usb/typec/class.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_CLASS__
+#define __USB_TYPEC_CLASS__
+
+#include <linux/device.h>
+#include <linux/usb/typec.h>
+
+struct typec_mux;
+struct typec_switch;
+
+struct typec_plug {
+ struct device dev;
+ enum typec_plug_index index;
+ struct ida mode_ids;
+ int num_altmodes;
+};
+
+struct typec_cable {
+ struct device dev;
+ enum typec_plug_type type;
+ struct usb_pd_identity *identity;
+ unsigned int active:1;
+ u16 pd_revision; /* 0300H = "3.0" */
+};
+
+struct typec_partner {
+ struct device dev;
+ unsigned int usb_pd:1;
+ struct usb_pd_identity *identity;
+ enum typec_accessory accessory;
+ struct ida mode_ids;
+ int num_altmodes;
+ u16 pd_revision; /* 0300H = "3.0" */
+ enum usb_pd_svdm_ver svdm_version;
+};
+
+struct typec_port {
+ unsigned int id;
+ struct device dev;
+ struct ida mode_ids;
+
+ int prefer_role;
+ enum typec_data_role data_role;
+ enum typec_role pwr_role;
+ enum typec_role vconn_role;
+ enum typec_pwr_opmode pwr_opmode;
+ enum typec_port_type port_type;
+ struct mutex port_type_lock;
+
+ enum typec_orientation orientation;
+ struct typec_switch *sw;
+ struct typec_mux *mux;
+
+ const struct typec_capability *cap;
+ const struct typec_operations *ops;
+};
+
+#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
+#define to_typec_plug(_dev_) container_of(_dev_, struct typec_plug, dev)
+#define to_typec_cable(_dev_) container_of(_dev_, struct typec_cable, dev)
+#define to_typec_partner(_dev_) container_of(_dev_, struct typec_partner, dev)
+
+extern const struct device_type typec_partner_dev_type;
+extern const struct device_type typec_cable_dev_type;
+extern const struct device_type typec_plug_dev_type;
+extern const struct device_type typec_port_dev_type;
+
+#define is_typec_partner(dev) ((dev)->type == &typec_partner_dev_type)
+#define is_typec_cable(dev) ((dev)->type == &typec_cable_dev_type)
+#define is_typec_plug(dev) ((dev)->type == &typec_plug_dev_type)
+#define is_typec_port(dev) ((dev)->type == &typec_port_dev_type)
+
+extern struct class typec_mux_class;
+
+#endif /* __USB_TYPEC_CLASS__ */
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index cf720e944aaaa..9da22ae3006c9 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -13,9 +13,9 @@
#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/slab.h>
-#include <linux/usb/typec_mux.h>
-#include "bus.h"
+#include "class.h"
+#include "mux.h"
static bool dev_name_ends_with(struct device *dev, const char *suffix)
{
diff --git a/drivers/usb/typec/mux.h b/drivers/usb/typec/mux.h
new file mode 100644
index 0000000000000..4fd9426ee44f6
--- /dev/null
+++ b/drivers/usb/typec/mux.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_MUX__
+#define __USB_TYPEC_MUX__
+
+#include <linux/usb/typec_mux.h>
+
+struct typec_switch {
+ struct device dev;
+ typec_switch_set_fn_t set;
+};
+
+struct typec_mux {
+ struct device dev;
+ typec_mux_set_fn_t set;
+};
+
+#define to_typec_switch(_dev_) container_of(_dev_, struct typec_switch, dev)
+#define to_typec_mux(_dev_) container_of(_dev_, struct typec_mux, dev)
+
+#endif /* __USB_TYPEC_MUX__ */
--
2.30.2
Introducing usb_for_each_port(). It works the same way as
usb_for_each_dev(), but instead of going through every USB
device in the system, it walks through the USB ports in the
system.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
include/linux/usb.h | 1 +
2 files changed, 44 insertions(+)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2ce3667ec6fae..6d49db9a1b208 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
}
EXPORT_SYMBOL_GPL(usb_for_each_dev);
+struct each_hub_arg {
+ void *data;
+ int (*fn)(struct device *, void *);
+};
+
+static int __each_hub(struct device *dev, void *data)
+{
+ struct each_hub_arg *arg = (struct each_hub_arg *)data;
+ struct usb_device *hdev = to_usb_device(dev);
+ struct usb_hub *hub;
+ int ret;
+ int i;
+
+ hub = usb_hub_to_struct_hub(hdev);
+ if (!hub)
+ return 0;
+
+ for (i = 0; i < hdev->maxchild; i++) {
+ ret = arg->fn(&hub->ports[i]->dev, arg->data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * usb_for_each_port - interate over all USB ports in the system
+ * @data: data pointer that will be handed to the callback function
+ * @fn: callback function to be called for each USB port
+ *
+ * Iterate over all USB ports and call @fn for each, passing it @data. If it
+ * returns anything other than 0, we break the iteration prematurely and return
+ * that value.
+ */
+int usb_for_each_port(void *data, int (*fn)(struct device *, void *))
+{
+ struct each_hub_arg arg = {data, fn};
+
+ return bus_for_each_dev(&usb_bus_type, NULL, &arg, __each_hub);
+}
+EXPORT_SYMBOL_GPL(usb_for_each_port);
+
/**
* usb_release_dev - free a usb device structure when all users of it are finished.
* @dev: device that's been disconnected
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 57c1e0ce5eba4..06ed5779fb4d8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -869,6 +869,7 @@ extern int usb_match_one_id(struct usb_interface *interface,
const struct usb_device_id *id);
extern int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *));
+int usb_for_each_port(void *data, int (*fn)(struct device *, void *));
extern struct usb_interface *usb_find_interface(struct usb_driver *drv,
int minor);
extern struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
--
2.30.2
This is only to make the handling of the class consistent
with the two other susbsystems - the alt mode bus and the
mux class.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/class.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 5fa279a96b6ef..d3e1002386357 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -17,7 +17,11 @@
#include "class.h"
static DEFINE_IDA(typec_index_ida);
-static struct class *typec_class;
+
+static struct class typec_class = {
+ .name = "typec",
+ .owner = THIS_MODULE,
+};
/* ------------------------------------------------------------------------- */
/* Common attributes */
@@ -551,7 +555,7 @@ typec_register_altmode(struct device *parent,
/* Plug alt modes need a class to generate udev events. */
if (is_typec_plug(parent))
- alt->adev.dev.class = typec_class;
+ alt->adev.dev.class = &typec_class;
ret = device_register(&alt->adev.dev);
if (ret) {
@@ -815,7 +819,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
partner->identity = desc->identity;
}
- partner->dev.class = typec_class;
+ partner->dev.class = &typec_class;
partner->dev.parent = &port->dev;
partner->dev.type = &typec_partner_dev_type;
dev_set_name(&partner->dev, "%s-partner", dev_name(&port->dev));
@@ -967,7 +971,7 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
ida_init(&plug->mode_ids);
plug->num_altmodes = -1;
plug->index = desc->index;
- plug->dev.class = typec_class;
+ plug->dev.class = &typec_class;
plug->dev.parent = &cable->dev;
plug->dev.type = &typec_plug_dev_type;
dev_set_name(&plug->dev, "%s-%s", dev_name(cable->dev.parent), name);
@@ -1132,7 +1136,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
cable->identity = desc->identity;
}
- cable->dev.class = typec_class;
+ cable->dev.class = &typec_class;
cable->dev.parent = &port->dev;
cable->dev.type = &typec_cable_dev_type;
dev_set_name(&cable->dev, "%s-cable", dev_name(&port->dev));
@@ -1986,7 +1990,7 @@ struct typec_port *typec_register_port(struct device *parent,
port->prefer_role = cap->prefer_role;
device_initialize(&port->dev);
- port->dev.class = typec_class;
+ port->dev.class = &typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = &typec_port_dev_type;
@@ -2049,11 +2053,9 @@ static int __init typec_init(void)
if (ret)
goto err_unregister_bus;
- typec_class = class_create(THIS_MODULE, "typec");
- if (IS_ERR(typec_class)) {
- ret = PTR_ERR(typec_class);
+ ret = class_register(&typec_class);
+ if (ret)
goto err_unregister_mux_class;
- }
return 0;
@@ -2069,7 +2071,7 @@ subsys_initcall(typec_init);
static void __exit typec_exit(void)
{
- class_destroy(typec_class);
+ class_unregister(&typec_class);
ida_destroy(&typec_index_ida);
bus_unregister(&typec_bus);
class_unregister(&typec_mux_class);
--
2.30.2
Adding functions that can be used to link/unlink ports -
USB ports, TBT3/USB4 ports, DisplayPorts and so on - to
the USB Type-C connectors they are attached to inside a
system. The symlink that is created for the port device is
named "connector".
Initially only ACPI is supported. ACPI port object shares
the _PLD (Physical Location of Device) with the USB Type-C
connector that it's attached to.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/class.c | 7 +-
drivers/usb/typec/class.h | 18 +++
drivers/usb/typec/port-mapper.c | 225 ++++++++++++++++++++++++++++++++
include/linux/usb/typec.h | 13 ++
5 files changed, 263 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/typec/port-mapper.c
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index a820e6e8c1ffc..ef790cb72d8c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,6 +4,7 @@ CFLAGS_tps6598x.o := -I$(src)
obj-$(CONFIG_TYPEC) += typec.o
typec-y := class.o mux.o bus.o
+typec-$(CONFIG_ACPI) += port-mapper.o
obj-$(CONFIG_TYPEC) += altmodes/
obj-$(CONFIG_TYPEC_TCPM) += tcpm/
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index d3e1002386357..ff199e2d26c7b 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -18,7 +18,7 @@
static DEFINE_IDA(typec_index_ida);
-static struct class typec_class = {
+struct class typec_class = {
.name = "typec",
.owner = THIS_MODULE,
};
@@ -1601,6 +1601,7 @@ static void typec_release(struct device *dev)
ida_destroy(&port->mode_ids);
typec_switch_put(port->sw);
typec_mux_put(port->mux);
+ free_pld(port->pld);
kfree(port->cap);
kfree(port);
}
@@ -1983,6 +1984,8 @@ struct typec_port *typec_register_port(struct device *parent,
ida_init(&port->mode_ids);
mutex_init(&port->port_type_lock);
+ mutex_init(&port->port_list_lock);
+ INIT_LIST_HEAD(&port->port_list);
port->id = id;
port->ops = cap->ops;
@@ -2024,6 +2027,8 @@ struct typec_port *typec_register_port(struct device *parent,
return ERR_PTR(ret);
}
+ port->pld = get_pld(&port->dev);
+
return port;
}
EXPORT_SYMBOL_GPL(typec_register_port);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index d414be58d122e..f3bc4d175d79c 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -54,6 +54,11 @@ struct typec_port {
const struct typec_capability *cap;
const struct typec_operations *ops;
+
+ struct list_head port_list;
+ struct mutex port_list_lock; /* Port list lock */
+
+ void *pld;
};
#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
@@ -72,5 +77,18 @@ extern const struct device_type typec_port_dev_type;
#define is_typec_port(dev) ((dev)->type == &typec_port_dev_type)
extern struct class typec_mux_class;
+extern struct class typec_class;
+
+#ifdef CONFIG_ACPI
+void *get_pld(struct device *dev);
+void free_pld(void *pld);
+#else
+static inline void *get_pld(struct device *dev)
+{
+ return NULL;
+}
+
+static inline void free_pld(void *pld) { }
+#endif
#endif /* __USB_TYPEC_CLASS__ */
diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c
new file mode 100644
index 0000000000000..97264a4f11967
--- /dev/null
+++ b/drivers/usb/typec/port-mapper.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Type-C Connector Class Port Mapping Utility
+ *
+ * Copyright (C) 2021, Intel Corporation
+ * Author: Heikki Krogerus <[email protected]>
+ */
+
+#include <linux/acpi.h>
+#include <linux/usb.h>
+#include <linux/usb/typec.h>
+
+#include "class.h"
+
+struct port_node {
+ struct list_head list;
+ struct device *dev;
+ void *pld;
+};
+
+static int acpi_pld_match(const struct acpi_pld_info *pld1,
+ const struct acpi_pld_info *pld2)
+{
+ if (!pld1 || !pld2)
+ return 0;
+
+ /*
+ * To speed things up, first checking only the group_position. It seems
+ * to often have the first unique value in the _PLD.
+ */
+ if (pld1->group_position == pld2->group_position)
+ return !memcmp(pld1, pld2, sizeof(struct acpi_pld_info));
+
+ return 0;
+}
+
+void *get_pld(struct device *dev)
+{
+ struct acpi_pld_info *pld;
+ acpi_status status;
+
+ if (!has_acpi_companion(dev))
+ return NULL;
+
+ status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld);
+ if (ACPI_FAILURE(status))
+ return NULL;
+
+ return pld;
+}
+
+void free_pld(void *pld)
+{
+ ACPI_FREE(pld);
+}
+
+static int __link_port(struct typec_port *con, struct port_node *node)
+{
+ int ret;
+
+ ret = sysfs_create_link(&node->dev->kobj, &con->dev.kobj, "connector");
+ if (ret)
+ return ret;
+
+ ret = sysfs_create_link(&con->dev.kobj, &node->dev->kobj,
+ dev_name(node->dev));
+ if (ret) {
+ sysfs_remove_link(&node->dev->kobj, "connector");
+ return ret;
+ }
+
+ list_add_tail(&node->list, &con->port_list);
+
+ return 0;
+}
+
+static int link_port(struct typec_port *con, struct port_node *node)
+{
+ int ret;
+
+ mutex_lock(&con->port_list_lock);
+ ret = __link_port(con, node);
+ mutex_unlock(&con->port_list_lock);
+
+ return ret;
+}
+
+static void __unlink_port(struct typec_port *con, struct port_node *node)
+{
+ sysfs_remove_link(&con->dev.kobj, dev_name(node->dev));
+ sysfs_remove_link(&node->dev->kobj, "connector");
+ list_del(&node->list);
+}
+
+static void unlink_port(struct typec_port *con, struct port_node *node)
+{
+ mutex_lock(&con->port_list_lock);
+ __unlink_port(con, node);
+ mutex_unlock(&con->port_list_lock);
+}
+
+static struct port_node *create_port_node(struct device *port)
+{
+ struct port_node *node;
+
+ node = kzalloc(sizeof(*node), GFP_KERNEL);
+ if (!node)
+ return ERR_PTR(-ENOMEM);
+
+ node->dev = get_device(port);
+ node->pld = get_pld(port);
+
+ return node;
+}
+
+static void remove_port_node(struct port_node *node)
+{
+ put_device(node->dev);
+ free_pld(node->pld);
+ kfree(node);
+}
+
+static int connector_pld_match(struct device *dev, const void *pld)
+{
+ if (!is_typec_port(dev))
+ return 0;
+
+ return acpi_pld_match(to_typec_port(dev)->pld, pld);
+}
+
+static struct device *find_connector(struct port_node *node)
+{
+ if (!node->pld)
+ return NULL;
+
+ return class_find_device(&typec_class, NULL, node->pld, connector_pld_match);
+}
+
+/**
+ * typec_link_port - Link a port to its connector
+ * @port: The port device
+ *
+ * Find the connector of @port and create symlink named "connector" for it.
+ * Returns 0 on success, or errno in case of a failure.
+ *
+ * NOTE. The function increments the reference count of @port on success.
+ */
+int typec_link_port(struct device *port)
+{
+ struct device *connector;
+ struct port_node *node;
+ int ret = 0;
+
+ node = create_port_node(port);
+ if (IS_ERR(node))
+ return PTR_ERR(node);
+
+ connector = find_connector(node);
+ if (!connector)
+ goto remove_node;
+
+ ret = link_port(to_typec_port(connector), node);
+ if (ret)
+ goto put_connector;
+
+ return 0;
+
+put_connector:
+ put_device(connector);
+remove_node:
+ remove_port_node(node);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(typec_link_port);
+
+struct port_match_data {
+ struct device *port;
+ struct device *connector;
+ struct port_node *node;
+};
+
+static int port_match(struct device *connector, void *_data)
+{
+ struct port_match_data *data = _data;
+ struct port_node *node;
+ int ret;
+
+ if (!is_typec_port(connector))
+ return 0;
+
+ mutex_lock(&to_typec_port(connector)->port_list_lock);
+ list_for_each_entry(node, &to_typec_port(connector)->port_list, list) {
+ ret = node->dev == data->port;
+ if (ret) {
+ data->connector = connector;
+ data->node = node;
+ break;
+ }
+ }
+ mutex_unlock(&to_typec_port(connector)->port_list_lock);
+
+ return ret;
+}
+
+/**
+ * typec_unlink_port - Unlink port from its connector
+ * @port: The port device
+ *
+ * Removes the symlink "connector" and decrements the reference count of @port.
+ */
+void typec_unlink_port(struct device *port)
+{
+ struct port_match_data data = { .port = port, };
+ int ret;
+
+ ret = class_for_each_device(&typec_class, NULL, &data, port_match);
+ if (!ret)
+ return;
+
+ unlink_port(to_typec_port(data.connector), data.node);
+ remove_port_node(data.node);
+ put_device(data.connector);
+}
+EXPORT_SYMBOL_GPL(typec_unlink_port);
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 91b4303ca305c..604cd2da15e83 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -298,4 +298,17 @@ int typec_find_port_data_role(const char *name);
void typec_partner_set_svdm_version(struct typec_partner *partner,
enum usb_pd_svdm_ver svdm_version);
int typec_get_negotiated_svdm_version(struct typec_port *port);
+
+#if IS_ENABLED(CONFIG_ACPI) && IS_REACHABLE(CONFIG_TYPEC)
+int typec_link_port(struct device *port);
+void typec_unlink_port(struct device *port);
+#else
+static inline int typec_link_port(struct device *port)
+{
+ return 0;
+}
+
+static inline void typec_unlink_port(struct device *port) { }
+#endif
+
#endif /* __LINUX_USB_TYPEC_H */
--
2.30.2
The connectors may be registered after the ports, so the
"connector" links need to be created for the ports also when
ever a new connector gets registered.
Signed-off-by: Heikki Krogerus <[email protected]>
---
drivers/usb/typec/class.c | 9 +++--
drivers/usb/typec/class.h | 10 +++---
drivers/usb/typec/port-mapper.c | 62 +++++++++++++++++++++++++++++++--
3 files changed, 71 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index ff199e2d26c7b..f1c2d823c6509 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1601,7 +1601,6 @@ static void typec_release(struct device *dev)
ida_destroy(&port->mode_ids);
typec_switch_put(port->sw);
typec_mux_put(port->mux);
- free_pld(port->pld);
kfree(port->cap);
kfree(port);
}
@@ -2027,7 +2026,9 @@ struct typec_port *typec_register_port(struct device *parent,
return ERR_PTR(ret);
}
- port->pld = get_pld(&port->dev);
+ ret = typec_link_ports(port);
+ if (ret)
+ dev_warn(&port->dev, "failed to create symlinks (%d)\n", ret);
return port;
}
@@ -2041,8 +2042,10 @@ EXPORT_SYMBOL_GPL(typec_register_port);
*/
void typec_unregister_port(struct typec_port *port)
{
- if (!IS_ERR_OR_NULL(port))
+ if (!IS_ERR_OR_NULL(port)) {
+ typec_unlink_ports(port);
device_unregister(&port->dev);
+ }
}
EXPORT_SYMBOL_GPL(typec_unregister_port);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index f3bc4d175d79c..a6a034f49c228 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -80,15 +80,15 @@ extern struct class typec_mux_class;
extern struct class typec_class;
#ifdef CONFIG_ACPI
-void *get_pld(struct device *dev);
-void free_pld(void *pld);
+int typec_link_ports(struct typec_port *connector);
+void typec_unlink_ports(struct typec_port *connector);
#else
-static inline void *get_pld(struct device *dev)
+static inline int typec_link_ports(struct typec_port *connector)
{
- return NULL;
+ return 0;
}
-static inline void free_pld(void *pld) { }
+static inline void typec_unlink_ports(struct typec_port *connector) { }
#endif
#endif /* __USB_TYPEC_CLASS__ */
diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c
index 97264a4f11967..98eda37d99117 100644
--- a/drivers/usb/typec/port-mapper.c
+++ b/drivers/usb/typec/port-mapper.c
@@ -34,7 +34,7 @@ static int acpi_pld_match(const struct acpi_pld_info *pld1,
return 0;
}
-void *get_pld(struct device *dev)
+static void *get_pld(struct device *dev)
{
struct acpi_pld_info *pld;
acpi_status status;
@@ -49,7 +49,7 @@ void *get_pld(struct device *dev)
return pld;
}
-void free_pld(void *pld)
+static void free_pld(void *pld)
{
ACPI_FREE(pld);
}
@@ -223,3 +223,61 @@ void typec_unlink_port(struct device *port)
put_device(data.connector);
}
EXPORT_SYMBOL_GPL(typec_unlink_port);
+
+static int connector_match(struct device *port, void *connector)
+{
+ struct port_node *node;
+ int ret;
+
+ node = create_port_node(port);
+ if (IS_ERR(node))
+ return PTR_ERR(node);
+
+ if (!node->pld || !connector_pld_match(connector, node->pld)) {
+ remove_port_node(node);
+ return 0;
+ }
+
+ ret = link_port(to_typec_port(connector), node);
+ if (ret) {
+ remove_port_node(node->pld);
+ return ret;
+ }
+
+ get_device(connector);
+
+ return 0;
+}
+
+int typec_link_ports(struct typec_port *con)
+{
+ int ret;
+
+ con->pld = get_pld(&con->dev);
+ if (!con->pld)
+ return 0;
+
+ ret = usb_for_each_port(&con->dev, connector_match);
+ if (ret)
+ typec_unlink_ports(con);
+
+ return ret;
+}
+
+void typec_unlink_ports(struct typec_port *con)
+{
+ struct port_node *node;
+ struct port_node *tmp;
+
+ mutex_lock(&con->port_list_lock);
+
+ list_for_each_entry_safe(node, tmp, &con->port_list, list) {
+ __unlink_port(con, node);
+ remove_port_node(node);
+ put_device(&con->dev);
+ }
+
+ mutex_unlock(&con->port_list_lock);
+
+ free_pld(con->pld);
+}
--
2.30.2
Creating link to the USB Type-C connector for every new port
that is added when possible.
Signed-off-by: Heikki Krogerus <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-usb | 9 +++++++++
drivers/usb/core/port.c | 3 +++
2 files changed, 12 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index bf2c1968525f0..8b4303a0ff51d 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -255,6 +255,15 @@ Description:
is permitted, "u2" if only u2 is permitted, "u1_u2" if both u1 and
u2 are permitted.
+What: /sys/bus/usb/devices/.../(hub interface)/portX/connector
+Date: April 2021
+Contact: Heikki Krogerus <[email protected]>
+Description:
+ Link to the USB Type-C connector when available. This link is
+ only created when USB Type-C Connector Class is enabled, and
+ only if the system firmware is capable of describing the
+ connection between a port and its connector.
+
What: /sys/bus/usb/devices/.../power/usb2_lpm_l1_timeout
Date: May 2013
Contact: Mathias Nyman <[email protected]>
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index dfcca9c876c73..3c382a4b648ec 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -9,6 +9,7 @@
#include <linux/slab.h>
#include <linux/pm_qos.h>
+#include <linux/usb/typec.h>
#include "hub.h"
@@ -576,6 +577,7 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
}
find_and_link_peer(hub, port1);
+ typec_link_port(&port_dev->dev);
/*
* Enable runtime pm and hold a refernce that hub_configure()
@@ -619,5 +621,6 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int port1)
peer = port_dev->peer;
if (peer)
unlink_peers(port_dev, peer);
+ typec_unlink_port(&port_dev->dev);
device_unregister(&port_dev->dev);
}
--
2.30.2
On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> Introducing usb_for_each_port(). It works the same way as
> usb_for_each_dev(), but instead of going through every USB
> device in the system, it walks through the USB ports in the
> system.
>
> Signed-off-by: Heikki Krogerus <[email protected]>
This has a couple of nasty errors.
> ---
> drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/usb.h | 1 +
> 2 files changed, 44 insertions(+)
>
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 2ce3667ec6fae..6d49db9a1b208 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> }
> EXPORT_SYMBOL_GPL(usb_for_each_dev);
>
> +struct each_hub_arg {
> + void *data;
> + int (*fn)(struct device *, void *);
> +};
> +
> +static int __each_hub(struct device *dev, void *data)
> +{
> + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> + struct usb_device *hdev = to_usb_device(dev);
to_usb_device() won't work properly if the struct device isn't embedded
in an actual usb_device structure. And that will happen, since the USB
bus type holds usb_interface structures as well as usb_devices.
In fact, you should use usb_for_each_dev here; it already does what you
want.
> + struct usb_hub *hub;
> + int ret;
> + int i;
> +
> + hub = usb_hub_to_struct_hub(hdev);
> + if (!hub)
> + return 0;
> +
> + for (i = 0; i < hdev->maxchild; i++) {
> + ret = arg->fn(&hub->ports[i]->dev, arg->data);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
Don't you need some sort of locking or refcounting here? What would
happen if this hub got removed while the routine was running?
Alan Stern
On Thu, Mar 25, 2021 at 10:41:09AM -0400, Alan Stern wrote:
> On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> > Introducing usb_for_each_port(). It works the same way as
> > usb_for_each_dev(), but instead of going through every USB
> > device in the system, it walks through the USB ports in the
> > system.
> >
> > Signed-off-by: Heikki Krogerus <[email protected]>
>
> This has a couple of nasty errors.
>
> > ---
> > drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > include/linux/usb.h | 1 +
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > index 2ce3667ec6fae..6d49db9a1b208 100644
> > --- a/drivers/usb/core/usb.c
> > +++ b/drivers/usb/core/usb.c
> > @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> > }
> > EXPORT_SYMBOL_GPL(usb_for_each_dev);
> >
> > +struct each_hub_arg {
> > + void *data;
> > + int (*fn)(struct device *, void *);
> > +};
> > +
> > +static int __each_hub(struct device *dev, void *data)
> > +{
> > + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> > + struct usb_device *hdev = to_usb_device(dev);
>
> to_usb_device() won't work properly if the struct device isn't embedded
> in an actual usb_device structure. And that will happen, since the USB
> bus type holds usb_interface structures as well as usb_devices.
OK, so I need to filter them out.
> In fact, you should use usb_for_each_dev here; it already does what you
> want.
Unfortunately I can't use usb_for_each_dev here, because it deals with
struct usb_device while I need to deal with struct device in the
callback.
> > + struct usb_hub *hub;
> > + int ret;
> > + int i;
> > +
> > + hub = usb_hub_to_struct_hub(hdev);
> > + if (!hub)
> > + return 0;
> > +
> > + for (i = 0; i < hdev->maxchild; i++) {
> > + ret = arg->fn(&hub->ports[i]->dev, arg->data);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
>
> Don't you need some sort of locking or refcounting here? What would
> happen if this hub got removed while the routine was running?
I'll use a lock then.
thanks,
--
heikki
On Thu, Mar 25, 2021 at 05:14:42PM +0200, Heikki Krogerus wrote:
> On Thu, Mar 25, 2021 at 10:41:09AM -0400, Alan Stern wrote:
> > On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> > > Introducing usb_for_each_port(). It works the same way as
> > > usb_for_each_dev(), but instead of going through every USB
> > > device in the system, it walks through the USB ports in the
> > > system.
> > >
> > > Signed-off-by: Heikki Krogerus <[email protected]>
> >
> > This has a couple of nasty errors.
> >
> > > ---
> > > drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/usb.h | 1 +
> > > 2 files changed, 44 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > index 2ce3667ec6fae..6d49db9a1b208 100644
> > > --- a/drivers/usb/core/usb.c
> > > +++ b/drivers/usb/core/usb.c
> > > @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> > > }
> > > EXPORT_SYMBOL_GPL(usb_for_each_dev);
> > >
> > > +struct each_hub_arg {
> > > + void *data;
> > > + int (*fn)(struct device *, void *);
> > > +};
> > > +
> > > +static int __each_hub(struct device *dev, void *data)
> > > +{
> > > + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> > > + struct usb_device *hdev = to_usb_device(dev);
> >
> > to_usb_device() won't work properly if the struct device isn't embedded
> > in an actual usb_device structure. And that will happen, since the USB
> > bus type holds usb_interface structures as well as usb_devices.
>
> OK, so I need to filter them out.
>
> > In fact, you should use usb_for_each_dev here; it already does what you
> > want.
>
> Unfortunately I can't use usb_for_each_dev here, because it deals with
> struct usb_device while I need to deal with struct device in the
> callback.
Why do you need 'struct device' in the callback? All you really want is
the hub devices, right?
> > > + struct usb_hub *hub;
> > > + int ret;
> > > + int i;
> > > +
> > > + hub = usb_hub_to_struct_hub(hdev);
> > > + if (!hub)
> > > + return 0;
> > > +
> > > + for (i = 0; i < hdev->maxchild; i++) {
> > > + ret = arg->fn(&hub->ports[i]->dev, arg->data);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> >
> > Don't you need some sort of locking or refcounting here? What would
> > happen if this hub got removed while the routine was running?
>
> I'll use a lock then.
That's not going to work to be held over a callback. Just properly
increment the reference count.
thanks,
greg k-h
On Thu, Mar 25, 2021 at 05:14:45PM +0200, Heikki Krogerus wrote:
> On Thu, Mar 25, 2021 at 10:41:09AM -0400, Alan Stern wrote:
> > On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> > > Introducing usb_for_each_port(). It works the same way as
> > > usb_for_each_dev(), but instead of going through every USB
> > > device in the system, it walks through the USB ports in the
> > > system.
> > >
> > > Signed-off-by: Heikki Krogerus <[email protected]>
> >
> > This has a couple of nasty errors.
> >
> > > ---
> > > drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/usb.h | 1 +
> > > 2 files changed, 44 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > index 2ce3667ec6fae..6d49db9a1b208 100644
> > > --- a/drivers/usb/core/usb.c
> > > +++ b/drivers/usb/core/usb.c
> > > @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> > > }
> > > EXPORT_SYMBOL_GPL(usb_for_each_dev);
> > >
> > > +struct each_hub_arg {
> > > + void *data;
> > > + int (*fn)(struct device *, void *);
> > > +};
> > > +
> > > +static int __each_hub(struct device *dev, void *data)
> > > +{
> > > + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> > > + struct usb_device *hdev = to_usb_device(dev);
> >
> > to_usb_device() won't work properly if the struct device isn't embedded
> > in an actual usb_device structure. And that will happen, since the USB
> > bus type holds usb_interface structures as well as usb_devices.
>
> OK, so I need to filter them out.
>
> > In fact, you should use usb_for_each_dev here; it already does what you
> > want.
>
> Unfortunately I can't use usb_for_each_dev here, because it deals with
> struct usb_device while I need to deal with struct device in the
> callback.
Ah, I can use it instead of bus_for_each_dev() in usb_for_each_port().
I'll fix these in v2.
For the lock I guess I can just use the peer lock (usb_port_peer_mutex)?
thanks,
--
heikki
On Thu, Mar 25, 2021 at 05:14:42PM +0200, Heikki Krogerus wrote:
> On Thu, Mar 25, 2021 at 10:41:09AM -0400, Alan Stern wrote:
> > On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> > > Introducing usb_for_each_port(). It works the same way as
> > > usb_for_each_dev(), but instead of going through every USB
> > > device in the system, it walks through the USB ports in the
> > > system.
> > >
> > > Signed-off-by: Heikki Krogerus <[email protected]>
> >
> > This has a couple of nasty errors.
> >
> > > ---
> > > drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/usb.h | 1 +
> > > 2 files changed, 44 insertions(+)
> > >
> > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > index 2ce3667ec6fae..6d49db9a1b208 100644
> > > --- a/drivers/usb/core/usb.c
> > > +++ b/drivers/usb/core/usb.c
> > > @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> > > }
> > > EXPORT_SYMBOL_GPL(usb_for_each_dev);
> > >
> > > +struct each_hub_arg {
> > > + void *data;
> > > + int (*fn)(struct device *, void *);
> > > +};
> > > +
> > > +static int __each_hub(struct device *dev, void *data)
> > > +{
> > > + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> > > + struct usb_device *hdev = to_usb_device(dev);
> >
> > to_usb_device() won't work properly if the struct device isn't embedded
> > in an actual usb_device structure. And that will happen, since the USB
> > bus type holds usb_interface structures as well as usb_devices.
>
> OK, so I need to filter them out.
>
> > In fact, you should use usb_for_each_dev here; it already does what you
> > want.
>
> Unfortunately I can't use usb_for_each_dev here, because it deals with
> struct usb_device while I need to deal with struct device in the
> callback.
I see; the prototypes of arg->fn are different. Oh well, it's a shame
the code can't be reused. In any case, you should copy what
usb.c:__each_dev() does.
Alan Stern
> > > + struct usb_hub *hub;
> > > + int ret;
> > > + int i;
> > > +
> > > + hub = usb_hub_to_struct_hub(hdev);
> > > + if (!hub)
> > > + return 0;
> > > +
> > > + for (i = 0; i < hdev->maxchild; i++) {
> > > + ret = arg->fn(&hub->ports[i]->dev, arg->data);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> > > +}
> >
> > Don't you need some sort of locking or refcounting here? What would
> > happen if this hub got removed while the routine was running?
>
> I'll use a lock then.
>
> thanks,
>
> --
> heikki
On Thu, Mar 25, 2021 at 04:20:15PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Mar 25, 2021 at 05:14:42PM +0200, Heikki Krogerus wrote:
> > On Thu, Mar 25, 2021 at 10:41:09AM -0400, Alan Stern wrote:
> > > On Thu, Mar 25, 2021 at 03:29:21PM +0300, Heikki Krogerus wrote:
> > > > Introducing usb_for_each_port(). It works the same way as
> > > > usb_for_each_dev(), but instead of going through every USB
> > > > device in the system, it walks through the USB ports in the
> > > > system.
> > > >
> > > > Signed-off-by: Heikki Krogerus <[email protected]>
> > >
> > > This has a couple of nasty errors.
> > >
> > > > ---
> > > > drivers/usb/core/usb.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > > > include/linux/usb.h | 1 +
> > > > 2 files changed, 44 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> > > > index 2ce3667ec6fae..6d49db9a1b208 100644
> > > > --- a/drivers/usb/core/usb.c
> > > > +++ b/drivers/usb/core/usb.c
> > > > @@ -398,6 +398,49 @@ int usb_for_each_dev(void *data, int (*fn)(struct usb_device *, void *))
> > > > }
> > > > EXPORT_SYMBOL_GPL(usb_for_each_dev);
> > > >
> > > > +struct each_hub_arg {
> > > > + void *data;
> > > > + int (*fn)(struct device *, void *);
> > > > +};
> > > > +
> > > > +static int __each_hub(struct device *dev, void *data)
> > > > +{
> > > > + struct each_hub_arg *arg = (struct each_hub_arg *)data;
> > > > + struct usb_device *hdev = to_usb_device(dev);
> > >
> > > to_usb_device() won't work properly if the struct device isn't embedded
> > > in an actual usb_device structure. And that will happen, since the USB
> > > bus type holds usb_interface structures as well as usb_devices.
> >
> > OK, so I need to filter them out.
> >
> > > In fact, you should use usb_for_each_dev here; it already does what you
> > > want.
> >
> > Unfortunately I can't use usb_for_each_dev here, because it deals with
> > struct usb_device while I need to deal with struct device in the
> > callback.
>
> Why do you need 'struct device' in the callback? All you really want is
> the hub devices, right?
I need the ports, not the hubs.
> > > > + struct usb_hub *hub;
> > > > + int ret;
> > > > + int i;
> > > > +
> > > > + hub = usb_hub_to_struct_hub(hdev);
> > > > + if (!hub)
> > > > + return 0;
> > > > +
> > > > + for (i = 0; i < hdev->maxchild; i++) {
> > > > + ret = arg->fn(&hub->ports[i]->dev, arg->data);
> > > > + if (ret)
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > Don't you need some sort of locking or refcounting here? What would
> > > happen if this hub got removed while the routine was running?
> >
> > I'll use a lock then.
>
> That's not going to work to be held over a callback. Just properly
> increment the reference count.
I though we have done that already. Does bus_for_each_dev() do that
for the device that it passes to the callback until that callback
returns?
thanks,
--
heikki
Hi Heikki,
I love your patch! Yet something to improve:
[auto build test ERROR on peter.chen-usb/for-usb-next]
[also build test ERROR on linus/master v5.12-rc4 next-20210325]
[cannot apply to usb/usb-testing chrome-platform-linux/for-next balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Heikki-Krogerus/usb-Linking-ports-to-their-Type-C-connectors/20210325-203239
base: https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git for-usb-next
config: i386-randconfig-a015-20210325 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/3f5d8681c4754fba373563812803b8d5eb11639a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Heikki-Krogerus/usb-Linking-ports-to-their-Type-C-connectors/20210325-203239
git checkout 3f5d8681c4754fba373563812803b8d5eb11639a
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
ld: drivers/usb/typec/port-mapper.o: in function `typec_link_ports':
>> drivers/usb/typec/port-mapper.c:260: undefined reference to `usb_for_each_port'
vim +260 drivers/usb/typec/port-mapper.c
251
252 int typec_link_ports(struct typec_port *con)
253 {
254 int ret;
255
256 con->pld = get_pld(&con->dev);
257 if (!con->pld)
258 return 0;
259
> 260 ret = usb_for_each_port(&con->dev, connector_match);
261 if (ret)
262 typec_unlink_ports(con);
263
264 return ret;
265 }
266
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]