Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO

[Kishon Vijay Abraham I <kishon@ti.com>]

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.

Thanks
Kishon