Re: [PATCH] usb: phy: generic: request regulator optionally

[Stefan Agner]

On 2016-09-07 11:53, Mark Brown wrote:
> On Tue, Sep 06, 2016 at 11:01:15AM -0700, Stefan Agner wrote:
>> On 2016-09-06 01:22, Mark Brown wrote:
> 
>> > This is nonsense unless the device can work without this supply.  Given
>> > that the supply is called VCC that doesn't seem entirely likely.
> 
>> Afaik it is kind of a generic device tree binding, I guess the physical
>> device can have various appearances and properties...
> 
> Is it really realistic that a meaningful proportion of them will work
> without power?
> 

No IP in a SoC runs without power, but still we don't model the supply
to every IP....

I would have guessed that there are SoCs with an internal USB PHY which
are powered implicitly by the SoC, and/or it is unclear how they exactly
get powered... If we make the supply mandatory, we can only guess how it
is wired up and probably end up to assign some "global" SoC supply
regulator.

>> A quick survey showed several device trees which do not specify
>> vcc-supply...
> 
> The regulator framework will attempt to be forgiving in what it accepts,
> the absence of a mandatory supply is sadly not a good indication that
> the supply does not physically exist...
> 
>> That said, I checked the device at hand, and it actually has a USB PHY
>> power supply inputs, but the device tree does not model them.
> 
> ...like here.
> 
>> > That's how to use _get_optional() but it's really unusual that you
>> > should be using _get_optional().
> 
>> Despite the above findings, I still think it is the right thing to do as
>> long as we specify vcc-supply to be optional.
> 
> I disagree, and bear in mind that it is more complex all round to handle
> optional supples - the reason they exist is that on devices where
> supplies may be omitted you usually have to do some kind of special case
> handling (like enabling internal regulators or something).  If you don't
> have any such special case handling but instead simply omit enables and
> disables then that's a fairly clear abuse of the API.

Note that in this case the code already supports an optional supply e.g.
when it has not specified via platform data. The code does not do any
special in that case, it is assumed to be implicitly powered.

I am not really a USB PHY experts and don't have a good overview of what
is out there, I just did a short survey across the device trees we have,
and found some which were lacking the vcc-supply. I guess Felipe has a
better overview and just should make a call on that matter.

In case we decide to make vcc-supply mandatory for the device tree case,
I will send a new patch altering the bindings documentation accordingly
and get rid of the needs_vcc = of_property_read_bool(node,
"vcc-supply");. This should not break existing device trees since
devm_regulator_get returns the dummy regulators.

--
Stefan