| Message ID | 20210329084426.78138-7-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: Linking ports to their Type-C connectors | expand |
On Mon, Mar 29, 2021 at 11:44:26AM +0300, Heikki Krogerus wrote: > +#ifdef CONFIG_USB This feels odd in a file under drivers/usb/ is it still relevant? Will this code get built for non-USB systems (i.e. gadget only?) > +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; > +} > +#endif > + > +int typec_link_ports(struct typec_port *con) > +{ > + int ret = 0; > + > + con->pld = get_pld(&con->dev); > + if (!con->pld) > + return 0; > + > +#ifdef CONFIG_USB > + ret = usb_for_each_port(&con->dev, each_port); > + if (ret) > + typec_unlink_ports(con); If you have proper #ifdef for CONFIG_USB in the .h file, then there's no need for the #ifdef in the .c file. thanks, greg k-h
On Mon, Mar 29, 2021 at 10:48:19AM +0200, Greg Kroah-Hartman wrote: > On Mon, Mar 29, 2021 at 11:44:26AM +0300, Heikki Krogerus wrote: > > +#ifdef CONFIG_USB > > This feels odd in a file under drivers/usb/ is it still relevant? Will > this code get built for non-USB systems (i.e. gadget only?) Yes, later. The typec connector class can not depend on CONFIG_USB for sure. > > +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; > > +} > > +#endif > > + > > +int typec_link_ports(struct typec_port *con) > > +{ > > + int ret = 0; > > + > > + con->pld = get_pld(&con->dev); > > + if (!con->pld) > > + return 0; > > + > > +#ifdef CONFIG_USB > > + ret = usb_for_each_port(&con->dev, each_port); > > + if (ret) > > + typec_unlink_ports(con); > > If you have proper #ifdef for CONFIG_USB in the .h file, then there's no > need for the #ifdef in the .c file. We could do that now, but we will have to move the ifdef back to the C file the moment we add support for Thunderbolt ports and/or DisplayPorts. I could make a stub for the usb_for_each_port() function in case CONFIG_USB is not enable. Would that work? thanks,
> I could make a stub for the usb_for_each_port() function in case > CONFIG_USB is not enable. Would that work? Ah, I think that's what you meant :-) I'll fix it. thaks,
On Mon, Mar 29, 2021 at 12:19:36PM +0300, Heikki Krogerus wrote: > > I could make a stub for the usb_for_each_port() function in case > > CONFIG_USB is not enable. Would that work? > > Ah, I think that's what you meant :-) Yes, that's what I meant :) thanks, greg k-h
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 52294f7020a8b..aef03eb7e1523 100644 --- a/drivers/usb/typec/class.h +++ b/drivers/usb/typec/class.h @@ -79,7 +79,7 @@ extern const struct device_type typec_port_dev_type; extern struct class typec_mux_class; extern struct class typec_class; -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); #endif /* __USB_TYPEC_CLASS__ */ diff --git a/drivers/usb/typec/port-mapper.c b/drivers/usb/typec/port-mapper.c index 5bee7a97242fe..fafd98de89df5 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) { #ifdef CONFIG_ACPI struct acpi_pld_info *pld; @@ -53,7 +53,7 @@ void *get_pld(struct device *dev) #endif } -void free_pld(void *pld) +static void free_pld(void *pld) { #ifdef CONFIG_ACPI ACPI_FREE(pld); @@ -217,3 +217,64 @@ void typec_unlink_port(struct device *port) class_for_each_device(&typec_class, NULL, port, port_match_and_unlink); } EXPORT_SYMBOL_GPL(typec_unlink_port); + +#ifdef CONFIG_USB +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; +} +#endif + +int typec_link_ports(struct typec_port *con) +{ + int ret = 0; + + con->pld = get_pld(&con->dev); + if (!con->pld) + return 0; + +#ifdef CONFIG_USB + ret = usb_for_each_port(&con->dev, each_port); + if (ret) + typec_unlink_ports(con); +#endif + 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); +}
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 <heikki.krogerus@linux.intel.com> --- drivers/usb/typec/class.c | 9 +++-- drivers/usb/typec/class.h | 4 +- drivers/usb/typec/port-mapper.c | 65 ++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 7 deletions(-)