Hi, On Monday 24 February 2014 04:35 PM, Roger Quadros wrote: > On 02/24/2014 11:51 AM, Kishon Vijay Abraham I wrote: >> Hi Roger, >> >> On Friday 21 February 2014 05:59 PM, Roger Quadros wrote: >>> On 02/21/2014 02:25 PM, Kishon Vijay Abraham I wrote: >>>> Hi Roger, >>>> >>>> On Wednesday 19 February 2014 06:07 PM, Roger Quadros wrote: >>>>> Hi, >>>>> >>>>> On 02/12/2014 11:46 AM, Kishon Vijay Abraham I wrote: >>>>>> On Wednesday 29 January 2014 08:17 PM, Heikki Krogerus wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: >>>>>>>> On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: >>>>>>>>> On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: >>>>>>>>> For the controller drivers the PHYs are just a resource like any >>>>>>>>> other. The controller drivers can't have any responsibility of >>>>>>>>> them. They should not care if PHY drivers are available for them or >>>>>>>>> not, or even if the PHY framework is available or not. >>>>>>>> >>>>>>>> huh? If memory isn't available you don't continue probing, right ? If >>>>>>>> your IORESOURCE_MEM is missing, you also don't continue probing, if >>>>>>>> your >>>>>>>> IRQ line is missing, you bail too. Those are also nothing but resources >>>>>>>> to the driver, what you're asking here is to treat PHY as a _different_ >>>>>>>> resource; which might be fine, but we need to make sure we don't >>>>>>>> continue probing when a PHY is missing in a platform that certainly >>>>>>>> needs a PHY. >>>>>>> >>>>>>> Yes, true. In my head I was comparing the PHY only to resources like >>>>>>> gpios, clocks, dma channels, etc. that are often optional to the >>>>>>> drivers. >>>>>>> >>>>>>>>>>>> But I really want to see the argument against using no-op. As far >>>>>>>>>>>> as I >>>>>>>>>>>> could see, everybody needs a PHY driver one way or another, some >>>>>>>>>>>> platforms just haven't sent any PHY driver upstream and have their >>>>>>>>>>>> own >>>>>>>>>>>> hacked up solution to avoid using the PHY layer. >>>>>>>>>>> >>>>>>>>>>> Not true in our case. Platforms using Intel's SoCs and chip sets may >>>>>>>>>>> or may not have controllable USB PHY. Quite often they don't. The >>>>>>>>>>> Baytrails have usually ULPI PHY for USB2, but that does not mean >>>>>>>>>>> they >>>>>>>>>>> provide any vendor specific functions or any need for a driver in >>>>>>>>>>> any >>>>>>>>>>> case. >>>>>>>>>> >>>>>>>>>> that's different from what I heard. >>>>>>>>> >>>>>>>>> I don't know where you got that impression, but it's not true. The >>>>>>>>> Baytrail SoCs for example don't have internal USB PHYs, which means >>>>>>>>> the manufacturers using it can select what they want. So we have >>>>>>>>> boards where PHY driver(s) is needed and boards where it isn't. >>>>>>>> >>>>>>>> alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? >>>>>>>> You have an external PIPE3 interface ? That's quite an achievement, >>>>>>>> kudos to your HW designers. Getting timing closure on PIPE3 is a >>>>>>>> difficult task. >>>>>>> >>>>>>> No, only the USB2 PHY is external. I'm giving you wrong information, >>>>>>> I'm sorry about that. Need to concentrate on what I'm writing. >>>>>>> >>>>>>> <snip> >>>>>>> >>>>>>>>> This is really good to get. We have some projects where we are dealing >>>>>>>>> with more embedded environments, like IVI, where the kernel should be >>>>>>>>> stripped of everything useless. Since the PHYs are autonomous, we >>>>>>>>> should be able to disable the PHY libraries/frameworks. >>>>>>>> >>>>>>>> hmmm, in that case it's a lot easier to treat. We can use >>>>>>>> ERR_PTR(-ENXIO) as an indication that the framework is disabled, or >>>>>>>> something like that. >>>>>>>> >>>>>>>> The difficult is really reliably supporting e.g. OMAP5 (which won't >>>>>>>> work >>>>>>>> without a PHY) and your BayTrail with autonomous PHYs. What can we use >>>>>>>> as an indication ? >>>>>>> >>>>>>> OMAP has it's own glue driver, so shouldn't it depend on the PHY >>>>>>> layer? >>>>>> >>>>>> right, but the PHY is connected to the dwc3 core and not to the glue. >>>>>>> >>>>>>>> I mean, I need to know that a particular platform depends on a PHY >>>>>>>> driver before I decide to return -EPROBE_DEFER or just assume the PHY >>>>>>>> isn't needed ;-) >>>>>>> >>>>>>> I don't think dwc3 (core) should care about that. The PHY layer needs >>>>>>> to tell us that. If the PHY driver that the platform depends is not >>>>>>> available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up >>>>>>> returning -EPROBE_DEFER. >>>>>> >>>>>> I don't think the PHY layer can 'reliably' tell if PHY driver is >>>>>> available or >>>>>> not. Consider when the phy_provider_register fails, there is no way to >>>>>> know if >>>>>> PHY driver is available or not. There are a few cases where PHY layer >>>>>> returns >>>>>> -EPROBE_DEFER but none of them can tell for sure that PHY driver is >>>>>> either >>>>>> available and failed or not available at all. It would be best for us to >>>>>> leave >>>>>> that to the platforms if we want to be sure if the platform needs a PHY >>>>>> or not. >>>>>> >>>>> >>>>> Just to summarize this thread on what we need >>>> >>>> Thanks for summarizing. >>>>> >>>>> 1) dwc3 core shouldn't worry about platform specific stuff i.e. PHY >>>>> needed or not. >>>>> It should be as generic as possible. >>>>> >>>>> 2) dwc3 core should continue probe even if PHY layer is not enabled, as >>>>> not all platforms need it. >>>>> >>>>> 3) dwc3 core should continue probe if PHY device is not available. >>>>> (-ENODEV?) >>>>> >>>>> 4) dwc3 core should error out on any error condition if PHY device is >>>>> available and caused some error, >>>>> e.g. init error. >>>>> >>>>> 5) dwc3 core should return EPROBE_DEFER if PHY device is available but >>>>> device driver is not yet loaded. >>>>> >>>>> 6) platform glue should do the necessary sanity checks for availability >>>>> of all resources like PHY device, PHY layer, etc, before populating the >>>>> dwc3 device. e.g. in OMAP5 case we could check if both usb2 and usb3 PHY >>>>> nodes are available in the DT and PHY layer is enabled, from dwc3-omap.c? >>>>> In J6 case we could check that at least usb2 phy node is there for the >>>>> High-Speed only controller, and so on. >>>> >>>> The PHY is connected to the dwc3 core. So I'm not sure if we should be >>>> doing >>>> checks for PHY in the glue layer. >>> >>> Sorry, I didn't get you. My reasoning was that since OMAP platform has this >>> strict requirement of requiring >>> explicit PHY control in order to work, we must do the sanity checks in OMAP >>> specific code and not in the dwc3 core code. It has nothing to do with how >>> hardware is laid out. >> >> What kind of sanity check do you think can be done in OMAP code? We don't use >> any of the PHY API's in glue code. If we add the same PHY APIs in glue code >> it >> will be duplication of the same code without much value besides breaking the >> design guideline of the software to be modelled similar to hardware. >> > > I wasn't saying about using PHY APIs in glue code, but just doing the basic > sanity checks like > presence of PHY layer, the required USB PHY DT nodes and the required drivers > for the platform.
hmm.. instead of doing sanity checks we can just select them in Kconfig like what I suggested before no? We could check if there is a phandle on the child dwc3 node though. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html