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-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to