Re: [PATCH v6.5 20/25] usb: chipidea: add host role

[Matthieu CASTET <matthieu.castet@xxxxxxxxxx>]

>
> [three time's the charm; sorry for spam]
>
> > Note that some chipidea version need special callback for phy init
> > or errata workaround.
>
> It can be done similarly to how it's currently done in ehci-mv, I
> guess. I'm not sure which particular workaround you are referring to,
> though.
See http://marc.info/?l=linux-usb&m=133558407107168&w=2 for example.
I added Peter Chen on CC. I could provide more info on this.

tegra one also got special callback (tegra_ehci_hub_control, ...)
>
> > In the current version, this is done in platform driver (ehci-fsl.c,
> > ehci-tegra.c, ...).
>
> Are these two even chipideas? They don't look like they are from the
> register definitions.
Yes, but fsl got some extra register for special stuff (non_ehci base).
Nearly all ehci core with has_tt are chipidea.

>
> > I don't know you do you plan to manage them ?
>
> My idea is that if we need workarounds in the controller code for
> certain versions of the controller, we can do that based on the
> chipidea version that we can determine at run time and/or platform
> supplied information. This, of course, has to be part of the hdrc
> driver. If there is any platform specific phy code though, it has to
> be managed by usb_phy layer.
But AFAIK the phy setting are lost after a ehci_reset, and in the current host
code I don't see call to phy layer.


> +static int ci_ehci_setup(struct usb_hcd *hcd)
> +{
> +	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> +	int ret;
> +
> +	ret = ehci_halt(ehci);
> +	if (ret)
> +		return ret;
> +
> +	ret = ehci_init(hcd);
> +	if (ret)
> +		return ret;
> +
> +	ehci_reset(ehci);
> +
> +	return ret;
> +}
Why can't you use ehci_setup here ?

> +	ret = hw_device_reset(ci, USBMODE_CM_HOST);
Why do you need to do that ? ehci_reset will do it (with tdi_reset).


Matthieu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html