[PATCH V2] usb: ehci-omap: Add Support DM_USB and OF_CONTROL

[Marek Vasut]

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