Re: [RFC PATCH v1 2/6] dt-bindings: phy: rockchip: add rk3328 usb3 phy

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/01/2025 14:32, Peter Geis wrote:
>>
>>
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        enum:
>>> +          - rockchip,rk3328-usb3phy-utmi
>>> +
>>> +      reg:
>>> +        maxItems: 1
>>> +
>>> +      "#phy-cells":
>>> +        const: 0
>>
>> Does not look correct. Your parent device is the phy, not child. Why do
>> you create children per each type of phy?
> 
> Because that's how it's done elsewhere in Rockchip's phys [1]. How
> should it be done?


The phys have separate supplies and IO addresses? Then it is reasonable
to keep them separate and as children. But then more questions appear:
why resets - which are also per utmi or port - are in top-level node?
This should be represented in coherent way: either you define the
properties/nodes per PHY or just everything in one/entire PHY
controller. Not mixed.

Same concerns about clocks in top-level.

It also might be that everything is a bit mixed, so you have entire phy
controller handling common resources and still separate phy for USB2 and
USB3 as children, but that should be conscious choice coming from actual
hardware. You have entire "description:" in binding to explain the
hardware and any questions I asked now.


Best regards,
Krzysztof