On 5/10/20 4:33 PM, Adam Ford wrote: [...] > @@ -177,8 +180,13 @@ int omap_ehci_hcd_stop(void) > * Based on "drivers/usb/host/ehci-omap.c" from Linux 3.1 > * See there for additional Copyrights. > */ > +#if !CONFIG_IS_ENABLED(DM_USB) || !CONFIG_IS_ENABLED(OF_CONTROL) > + > int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data *usbhs_pdata, > struct ehci_hccr **hccr, struct ehci_hcor **hcor) > +#else > +int omap_ehci_hcd_init(int index, struct omap_usbhs_board_data *usbhs_pdata) > +#endif > { > int ret; > unsigned int i, reg = 0, rev = 0; > @@ -285,10 +293,130 @@ int omap_ehci_hcd_init(int index, struct > omap_usbhs_board_data *usbhs_pdata, > for (i = 0; i < OMAP_HS_USB_PORTS; i++) > if (is_ehci_phy_mode(usbhs_pdata->port_mode[i])) > omap_ehci_soft_phy_reset(i); > - > +#if !CONFIG_IS_ENABLED(DM_USB) || !CONFIG_IS_ENABLED(OF_CONTROL) > *hccr = (struct ehci_hccr *)(OMAP_EHCI_BASE); > *hcor = (struct ehci_hcor *)(OMAP_EHCI_BASE + 0x10); > +#endif
Maybe you can have common, DM and non-DM variant of this function to avoid this extra ifdeffery ? [...] > +static int find_phy_mode(const char *phy_type) > +{ > + int i; > + > + for (i = 0; i < OMAP_HS_USB_PORTS; i++) { > + if (!strcmp(phy_type, phy_strings[i])) > + return i; > + } Isn't there already some function to find node by name ? > + return 0; > +} > + > +static const struct ehci_ops omap_ehci_ops = { > +}; > + > +static int ehci_usb_probe(struct udevice *dev) > +{ > + struct usb_platdata *plat = dev_get_platdata(dev); > + const void *fdt = gd->fdt_blob; > + struct omap_ehci *ehci; > + struct ehci_omap_priv_data *priv = dev_get_priv(dev); > + struct ehci_hccr *hccr; > + struct ehci_hcor *hcor; > + ofnode node; > + ofnode parent_node = dev->node; > + int ret = -1; > + int off, i; > + const char *mode; > + char prop[11]; > + > + priv->ehci = (struct omap_ehci *)devfdt_get_addr(dev); > + priv->portnr = dev->seq; > + priv->init_type = plat->init_type; > + > + /* Go through each port portX-mode to determing phy mode */ > + for (i = 0; i < 3; i++) { > + snprintf(prop, sizeof(prop), "port%d-mode", i + 1); > + mode = dev_read_string(dev, prop); > + if (mode) > + usbhs_bdata.port_mode[i] = find_phy_mode(mode); > + } > + > + ofnode_for_each_subnode(node, parent_node) { Should this be done in probe or in bind ? I think you might want a bind implementation which binds the controller and you probe only some of them on demand. > + /* Locate node for ti,ehci-omap */ > + off = ofnode_to_offset(node); > + ret = fdt_node_check_compatible(fdt, off, "ti,ehci-omap"); > + > + if (!ret) { > + /* If EHCI node is found, set the pointer to it */ > + ehci = (struct omap_ehci *)ofnode_get_addr(node); > + hccr = (struct ehci_hccr *)&ehci->hccapbase; > + hcor = (struct ehci_hcor *)&ehci->usbcmd; > + > + /* Do the actual EHCI initialization */ > + omap_ehci_hcd_init(0, &usbhs_bdata); > + ret = ehci_register(dev, hccr, hcor, &omap_ehci_ops, 0, > + USB_INIT_HOST); You need to check return value here (in multiple controllers case, this error will be ignored) > + } > + } > + > + return ret; > +} [...]