Hi Paul, > > > phy_type > > > phy_utmi_width > > > phy_ulpi_ddr > > > phy_ulpi_ext_vbus > > > i2c_enable > > > ulpi_fs_ls > > > host_support_fs_ls_low_power > > > host_ls_low_power_phy_clk > > > > > > The last 8 are related to the Phy. I wonder if they should be in a > > > separate Phy dt file? > > > > Looking at the dwc3 driver, it can have a "usb-phy" phandle property in > > its dt node that points to a different phy node in the dt, possibly with > > a different driver. However, all of the glue drivers default the phy to > > a nop_usb_xceiv phy driver, which is basically a noop driver AFAICS. > > > > I'm not sure if the dwc2 hardware could ever be connected to an external > > PHY that actually needs a driver? Or does the dwc2 core do all the > > talking to the PHY and are these parameters just to tell the core how to > > talk to the PHY? > > The Phy implementation is up to the system designer. But yes, the > parameters we are talking about are to tell the core how to talk to > the Phy. So maybe they do belong in the core's dt file. (I don't know > anything about how the dt stuff is supposed to work.) I'm also working on common sense wrt dt here, but I'll give this a go. I guess some of them are (also) a description of what the phy looks like (which the dwc2 driver can then of course use to know how to talk to it) and would be the same for a particular phy even if you connect it to another controller. These parameters could make sense in a separate node, I'd say. But some parameters might not actually describe the phy, but only provide a parameter to dwc2 related to talking to the phy. These parameters would not actually make sense when this phy was connected to another usb controller, so putting them in a separate phy node doesn't make sense to me. Not sure if any of the above parameters fall into this category... > > As you might gather from the above, I'm not really in the loop about how > > the hardware works here, so I don't think I'm qualified to decide on the > > best approach here... Perhaps I should adapt my patch to just the > > non-phy related parameters and we can do the phy-stuff later when it is > > clear how they should work? > > That won't work. The Phy parameters have to be set according to > the type of Phy that is connected, and some of them cannot be > autodetected. So they have to be defined somewhere. Actually, for my particular hardware this would be fine, since the defaults work (but the defaults / autodetected values for the non-phy params are also fine, so I don't really need any of this dt patch, I just coded it up for completeness). Regarding this "cannot be autodetected" stuff: I was wondering about the phy_utmi_width parameter (which has a big "NOT DETECTABLE" comment in the code). I noticed there is value in HWCFG4 that specifies wether the hardware can do 8, 16, or both 8 and 16 bits. In the case of 8 and 16, this means the value for this parameter can be autodetected (rather, the hardware simply ignores whatever you write, I think). IIRC my hardware had 8/16 set in the HWCFG4 register, but both 8 and 16 bits work. My suspicion is that the particular PHY used in my platform simply supports both 8 and 16 bits, but that might not always be the case (even though the dwc controller _does_ support both values). Does this sound correct to you? Gr. Matthijs -- 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
