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;
> +}
[...]

Reply via email to