Re: [PATCH 1/2] dt-bindings: usb: snps,dwc3: Allow power-domains property

[Felipe Balbi <balbi@kernel.org>]

Hi,

Rob Herring <robh@kernel.org> writes:
>> >> > >> >  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 3 +++
>> >> > >> >  1 file changed, 3 insertions(+)
>> >> > >> >
>> >> > >> > diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> > >> > index 6d78048c4613..bcefd1c2410a 100644
>> >> > >> > --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> > >> > +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> >> > >> > @@ -91,6 +91,9 @@ properties:
>> >> > >> >          - usb2-phy
>> >> > >> >          - usb3-phy
>> >> > >> >
>> >> > >> > +  power-domains:
>> >> > >> > +    maxItems: 1
>> >> > >>
>> >> > >> AFAICT this can be incorrect. Also, you could have Cc the dwc3
>> >> > >> maintainer to get comments.
>> >>
>> >> Felipe is correct. We have 2 power-domains: Core domain and PMU.
>> >
>> > Power management unit? Performance management unit?
>> >
>> > That doesn't change that the rk3399 is 1 and we're stuck with it. So I
>> > can say 1 or 2 domains, or we add the 2nd domain when someone needs
>> > it.
>>
>> Isn't the snps,dwc3.yaml document supposed to document dwc3's view of
>> the world? In that case, dwc3 expects 2 power domains. It just so
>> happens that in rk3399 they are fed from the same power supply, but
>> dwc3' still thinks there are two of them. No?
>
> Yes. That is how bindings *should* be. However, RK3399 defined one PD
> long ago and it's an ABI. So we are stuck with it. Everyone else put

Are you confusing things, perhaps? DWC3, the block Synopsys licenses,
has, as Thinh confirmed, 2 internal power domains. How OEMs (TI, Intel,
Rockchip, Allwinner, etc) decide to integrate the IP into their systems
is something different. That is part of the (so-called)
wrapper. Different integrators will wrap Synopsys IP however they see
fit, as long as they can provide a suitable translation layer between
Synopsys own view of the world (its own interconnect implementation, of
which there are 3 to choose from, IIRC) and the rest of the SoC.

Perhaps what RK3399 did was provide a single power domain at the wrapper
level that feeds both of DWC3's own power domains, but DWC3 itself still
has 2 power domains, that's not something rockchip can change without
risking the loss of support from Synopsys, as it would not be Synopsys
IP anymore.

> power-domains in the parent because obviously the DWC3 has 0
> power-domains.

How did you come to this conclusion?

>> It's a similar situation when you have multiple clock domains with the
>> same parent clock.
>
> Yes, that's a common problem in clock bindings too. Not really
> anything we can do about that other than require a detailed reference
> manual with every binding and someone (me) reviewing the manual
> against the binding. Neither of those are going to happen. Even on Arm
> Primecell blocks which clearly (and publicly) document the clocks,
> we've gotten these wrong (or .dts authors just didn't follow the
> binding).

Heh

-- 
balbi