On 14/10/15 20:20, Stephen Warren wrote: > (Dropping a lot of CCs since it looks like this needs some more work) > > On 10/06/2015 01:01 PM, Stephen Warren wrote: >> From: Stephen Warren <swar...@nvidia.com> >> >> Add device-tree binding documentation for the XUSB (xHCI) controller >> present on Tegra124 and later SoCs. >> >> Signed-off-by: Andrew Bresticker <abres...@chromium.org> >> [swarren, combined separate MFD, mailbox, XHCI bindings into one node] >> Signed-off-by: Stephen Warren <swar...@nvidia.com> > >> diff --git >> a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt >> b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt > >> +Optional properties: >> +-------------------- >> + - phys: Must contain an entry for each entry in phy-names. >> + See ../phy/phy-bindings.txt for details. >> + - phy-names: Should include an entry for each PHY used by the >> controller. >> + Names must be of the form "<type>-<number>" where <type> is one of >> "utmi", >> + "hsic", or "usb3" and <number> is a 0-based index. On Tegra124, >> there may >> + be up to 3 UTMI, 2 HSIC, and 2 USB3 PHYs. > > Now that I've looked at this binding in more detail, I think this > phy-related text is wrong. > > First off, I don't think the phys are optional at all. Whenever a phy is > used, we must include it in the phys/phy-names list. I guess what we > need is a more precise definition of which entries are required, and > then to make these properties mandatory. > > I think the naming of the phys above may be wrong too. The text above > sounds like it's using the names of the actual phys, rather than of the > ports that are using them. Since there's a big mux between the ports and > the phys, I suspect it would make more sense to identify the ports in > phy-names, and have the values in phys reflect the configuration of the > mux. That way no matter how the mux is configured, the names used in > phy-names always map 1:1 to the XUSB controller ports. Does that make > sense? For some of the ports, this makes no difference since all the mux > does is allow XUSB vs. some other module access to the PHY. But I think > for at least the USB3 ports, each port could be mux'd to various > different PHYs/lanes, so this distinction does have an effect.
Yes but please note that the documentation is a bit misleading on the number of actual options you have. For example, usb3-port0 is only available on pcie lane 0. I have just posted a patch to correct this [0]. > Question: Should the XUSB controller driver determine which of its ports > to enable/use based solely on which phy-names are present, or should we > have some explicit property to define which ports are in use? Downstream > does have a property for this purpose. Given that the number of port-to-phy mappings are finite and there are not too many, could be good to define them all and have the board dts enabled the ones it is using. Right now it is not very clear exactly what ports are available on which lanes. > Question: Each physical USB connector contains signals from both a USB2 > port and a USB3 port. The HW can be configured to associate different > USB2 and USB3 controllers with each-other for this purpose. The > configuration for this happens via a register in the XUSB padctl module > in register SS_PORT_MAP. However, the XUSB padctl binding doesn't allow > specification of that mapping yet, and the driver doesn't program that > register. I'm not sure if we should have a similar mapping property in > the XUSB controller's DT node, a custom API between the two drivers to > retrieve that information, or perhaps the XUSB controller driver doesn't > actually care, since HW implements all required interactions. Does > anyone know? Downstream also has a property that describes this mapping. My understanding is the the XUSB controller has 3 usb2 ports and 2 usb3 ports. The SS_PORT_MAP allows you to map any of the 3 usb2 ports from the XUSB module to any of the 2 usb3 ports. Yes this mapping is not supported currently by the padctl binding but this was in Andrew's series for XHCI [1] and the patch you are probably interested in is here [2]. Jon > (Although note that the downstream properties for those two purposes, or > at least the code that handles them, seems like a mess to me so far). > > Either way, I think we need to update the XUSB padctl binding to contain > this information before we consider the XUSB controller binding to be > fully fleshed out, so we can make sure the two bindings interact well. > >> +Example: > >> + phys = <&padctl TEGRA_XUSB_PADCTL_UTMI_P1>, /* mini-PCIe USB */ >> + <&padctl TEGRA_XUSB_PADCTL_UTMI_P2>, /* USB A */ >> + <&padctl TEGRA_XUSB_PADCTL_USB3_P0>; /* USB A */ >> + phy-names = "utmi-1", "utmi-2", "usb3-0"; > > We're also missing an update to the XUSB padctl binding to define all > those TEGRA_XUSB_PADCTL_UTMI_P* values. I think we need to complete that > in order to make sure that both bindings are fully thought out. > > Thierry, I found your WIP patches perhaps related to this at: >> https://github.com/thierryreding/linux/commits/staging/xhci > > However, those top two patches there don't contain any DT binding or > .dts file updates so it's difficult to tell where the changes are going. > Do you have binding/.dts patches elsewhere to show the intended result? > > I notice that series may also have support for configuring the per-board > tuning parameters too. That'd be useful to get in place too. Do you have > a source for the actual tuning values for any of the T210 boards that > mainline U-Boot now supports? [0] http://marc.info/?l=linux-tegra&m=144498745102948&w=2 [1] https://github.com/abrestic/linux/tree/tegra-xhci-v8-test [2] https://github.com/abrestic/linux/commit/82281999409b6c5a6a51df449a1c45204a249df8 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
