Hi, Sorry for being very very slow with getting around to cleaning this up, there just always was some higher prio item getting in the way; and any downtime I had I needed time to recuperate. On 8/26/20 3:17 PM, Heikki Krogerus wrote: > On Tue, Jul 14, 2020 at 01:36:15PM +0200, Hans de Goede wrote: >> This can be used by Type-C controller drivers which use a standard >> usb-connector fwnode, with altmodes sub-node, to describe the available >> altmodes. >> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> drivers/usb/typec/class.c | 56 +++++++++++++++++++++++++++++++++++++++ >> include/linux/usb/typec.h | 7 +++++ >> 2 files changed, 63 insertions(+) >> >> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c >> index c9234748537a..47de2b2e3d54 100644 >> --- a/drivers/usb/typec/class.c >> +++ b/drivers/usb/typec/class.c >> @@ -1607,6 +1607,62 @@ typec_port_register_altmode(struct typec_port *port, >> } >> EXPORT_SYMBOL_GPL(typec_port_register_altmode); >> >> +void typec_port_register_altmodes_from_fwnode(struct typec_port *port, >> + const struct typec_altmode_ops *ops, void *drvdata, >> + struct typec_altmode **altmodes, size_t n, >> + struct fwnode_handle *fwnode) > > That last fwnode parameter should not be needed. The port device > should have the alternate mode nodes under it as child nodes. That is, > unless I'm missing (or forgetting) something? Ack, I'll fix this, test the code and send a v2 soon. Regards, Hans > >> +{ >> + struct fwnode_handle *altmodes_node, *child; >> + struct typec_altmode_desc desc; >> + struct typec_altmode *alt; >> + size_t index = 0; >> + u32 svid, vdo; >> + int ret; >> + >> + altmodes_node = fwnode_get_named_child_node(fwnode, "altmodes"); > > So this would be: > > altmodes_node = fwnode_get_named_child_node(port->dev.fwnode, "altmodes"); > >> + if (!altmodes_node) >> + return; >> + >> + child = NULL; >> + while ((child = fwnode_get_next_child_node(altmodes_node, child))) { >> + ret = fwnode_property_read_u32(child, "svid", &svid); >> + if (ret) { >> + dev_err(&port->dev, "Error reading svid for altmode %s\n", >> + fwnode_get_name(child)); >> + continue; >> + } >> + >> + ret = fwnode_property_read_u32(child, "vdo", &vdo); >> + if (ret) { >> + dev_err(&port->dev, "Error reading vdo for altmode %s\n", >> + fwnode_get_name(child)); >> + continue; >> + } >> + >> + if (index >= n) { >> + dev_err(&port->dev, "Error not enough space for altmode %s\n", >> + fwnode_get_name(child)); >> + continue; >> + } >> + >> + desc.svid = svid; >> + desc.vdo = vdo; >> + desc.mode = index + 1; >> + alt = typec_port_register_altmode(port, &desc); >> + if (IS_ERR(alt)) { >> + dev_err(&port->dev, "Error registering altmode %s\n", >> + fwnode_get_name(child)); >> + continue; >> + } >> + >> + alt->ops = ops; >> + typec_altmode_set_drvdata(alt, drvdata); >> + altmodes[index] = alt; >> + index++; >> + } >> +} >> +EXPORT_SYMBOL_GPL(typec_port_register_altmodes_from_fwnode); >> + >> /** >> * typec_register_port - Register a USB Type-C Port >> * @parent: Parent device > > thanks, >