| Message ID | 20211124231028.696982-3-pmalani@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: Use notifier for linking Type C ports. | expand |
Hi Prashant,
I love your patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on linux/master linus/master peter-chen-usb/for-usb-next v5.16-rc2 next-20211124]
[cannot apply to 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/Prashant-Malani/usb-Use-notifier-for-linking-Type-C-ports/20211125-071439
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-defconfig (https://download.01.org/0day-ci/archive/20211125/202111251010.fxed9VtQ-lkp@intel.com/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/d56a1c2271ef9c1877e9400fb1adc5adbb278e51
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Prashant-Malani/usb-Use-notifier-for-linking-Type-C-ports/20211125-071439
git checkout d56a1c2271ef9c1877e9400fb1adc5adbb278e51
# save the config file 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 <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/usb/core/port.c:12:
>> include/linux/usb/typec.h:322:5: warning: no previous prototype for 'typec_port_registration_register_notify' [-Wmissing-prototypes]
322 | int typec_port_registration_register_notify(struct notifier_block *nb)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/usb/typec.h:327:5: warning: no previous prototype for 'typec_port_registration_unregister_notify' [-Wmissing-prototypes]
327 | int typec_port_registration_unregister_notify(struct notifier_block *nb)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/typec_port_registration_register_notify +322 include/linux/usb/typec.h
ed296d8d0a92758 Prashant Malani 2021-11-24 321
ed296d8d0a92758 Prashant Malani 2021-11-24 @322 int typec_port_registration_register_notify(struct notifier_block *nb)
ed296d8d0a92758 Prashant Malani 2021-11-24 323 {
ed296d8d0a92758 Prashant Malani 2021-11-24 324 return 0;
ed296d8d0a92758 Prashant Malani 2021-11-24 325 }
ed296d8d0a92758 Prashant Malani 2021-11-24 326
ed296d8d0a92758 Prashant Malani 2021-11-24 @327 int typec_port_registration_unregister_notify(struct notifier_block *nb)
ed296d8d0a92758 Prashant Malani 2021-11-24 328 {
ed296d8d0a92758 Prashant Malani 2021-11-24 329 return 0;
ed296d8d0a92758 Prashant Malani 2021-11-24 330 }
ae196ddb0d3186b Heikki Krogerus 2021-04-07 331 #endif
ae196ddb0d3186b Heikki Krogerus 2021-04-07 332
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 22ea1f4f2d66..d09a8c9c1b4e 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -11,6 +11,7 @@ * move struct usb_hub to this file. */ +#include <linux/notifier.h> #include <linux/usb.h> #include <linux/usb/ch11.h> #include <linux/usb/hcd.h> @@ -89,6 +90,7 @@ struct usb_hub { * @is_superspeed cache super-speed status * @usb3_lpm_u1_permit: whether USB3 U1 LPM is permitted. * @usb3_lpm_u2_permit: whether USB3 U2 LPM is permitted. + * @typec_nb: notifier called when a Type C port is registered. */ struct usb_port { struct usb_device *child; @@ -105,6 +107,7 @@ struct usb_port { unsigned int is_superspeed:1; unsigned int usb3_lpm_u1_permit:1; unsigned int usb3_lpm_u2_permit:1; + struct notifier_block typec_nb; }; #define to_usb_port(_dev) \ diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index dfcca9c876c7..53a64ce76183 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" @@ -528,6 +529,14 @@ static void find_and_link_peer(struct usb_hub *hub, int port1) link_peers_report(port_dev, peer); } +static int usb_port_link_typec_port(struct notifier_block *nb, unsigned long event, void *ptr) +{ + struct usb_port *port_dev = container_of(nb, struct usb_port, typec_nb); + + typec_link_port(&port_dev->dev); + return NOTIFY_OK; +} + int usb_hub_create_port_device(struct usb_hub *hub, int port1) { struct usb_port *port_dev; @@ -577,6 +586,14 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) find_and_link_peer(hub, port1); + /* + * In some cases, the Type C port gets registered later, so + * register a Type C notifier so that we can link the ports + * later too. + */ + port_dev->typec_nb.notifier_call = usb_port_link_typec_port; + typec_port_registration_register_notify(&port_dev->typec_nb); + /* * Enable runtime pm and hold a refernce that hub_configure() * will drop once the PM_QOS_NO_POWER_OFF flag state has been set @@ -616,6 +633,7 @@ void usb_hub_remove_port_device(struct usb_hub *hub, int port1) struct usb_port *port_dev = hub->ports[port1 - 1]; struct usb_port *peer; + typec_port_registration_unregister_notify(&port_dev->typec_nb); peer = port_dev->peer; if (peer) unlink_peers(port_dev, peer); diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index 14b82109b0f5..92d03eb65f12 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -2110,10 +2110,7 @@ struct typec_port *typec_register_port(struct device *parent, return ERR_PTR(ret); } - ret = typec_link_ports(port); - if (ret) - dev_warn(&port->dev, "failed to create symlinks (%d)\n", ret); - + /* Used to allow USB ports to register to this Type C port */ blocking_notifier_call_chain(&typec_port_registration_notifier, 0, port); return port; diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h index aef03eb7e152..517236935a55 100644 --- a/drivers/usb/typec/class.h +++ b/drivers/usb/typec/class.h @@ -79,7 +79,6 @@ extern const struct device_type typec_port_dev_type; extern struct class typec_mux_class; extern struct class typec_class; -int typec_link_ports(struct typec_port *connector); void typec_unlink_ports(struct typec_port *connector); #endif /* __USB_TYPEC_CLASS__ */ diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c index 9b0991bdf391..5597291bd769 100644 --- a/drivers/usb/typec/port-mapper.c +++ b/drivers/usb/typec/port-mapper.c @@ -7,7 +7,6 @@ */ #include <linux/acpi.h> -#include <linux/usb.h> #include <linux/usb/typec.h> #include "class.h" @@ -220,46 +219,6 @@ void typec_unlink_port(struct device *port) } EXPORT_SYMBOL_GPL(typec_unlink_port); -static int each_port(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 (!connector_match(connector, node)) { - 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 = 0; - - con->pld = get_pld(&con->dev); - if (!con->pld) - return 0; - - ret = usb_for_each_port(&con->dev, each_port); - if (ret) - typec_unlink_ports(con); - - return ret; -} - void typec_unlink_ports(struct typec_port *con) { struct port_node *node;
Instead of using a helper function to register pre-existing USB ports to a new type C port, have each port register a notifier with the Type C connector class, and use that to perform the linking when a new Type C port is registered. This avoid a cyclic dependency from a later change which is needed to link a new USB port to a pre-existing Type C port. Some context on this is in here [1]; in summary, typec_link_ports() introduces a dependency from typec -> usbcore. However, commit 63cd78617350 ("usb: Link the ports to the connectors they are attached to") then introduces a dependency from usbcore -> typec. In order to allow that commit without creating the cyclic dependency, we use a notifier here instead of typec_link_ports(). Since we don't need the helper typec_link_ports() function anymore, remove it and its related functions from port-mapper code. [1]: https://lore.kernel.org/all/20210412213655.3776e15e@canb.auug.org.au/ Cc: Benson Leung <bleung@chromium.org> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Signed-off-by: Prashant Malani <pmalani@chromium.org> --- drivers/usb/core/hub.h | 3 +++ drivers/usb/core/port.c | 18 +++++++++++++++ drivers/usb/typec/class.c | 5 +--- drivers/usb/typec/class.h | 1 - drivers/usb/typec/port-mapper.c | 41 --------------------------------- 5 files changed, 22 insertions(+), 46 deletions(-)