Re: [PATCH 1/2] dt-bindings: reset: document Broadcom's BCM4908 USB reset binding
From: Florian Fainelli
Date: Wed Dec 09 2020 - 19:28:30 EST
On 12/9/20 3:35 PM, Rob Herring wrote:
> On Fri, Dec 04, 2020 at 05:39:14PM +0100, Rafał Miłecki wrote:
>> On 04.12.2020 17:32, Florian Fainelli wrote:
>>> On 12/4/2020 1:37 AM, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@xxxxxxxxxx>
>>>>
>>>> Document binding of block responsible for initializing USB controllers
>>>> (OHCI, EHCI, XHCI).
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
>>>> ---
>>>> .../reset/brcm,bcm4908-usb-reset.yaml | 60 +++++++++++++++++++
>>>> 1 file changed, 60 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>> new file mode 100644
>>>> index 000000000000..31beb1c8f3cd
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/reset/brcm,bcm4908-usb-reset.yaml
>>>> @@ -0,0 +1,60 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/reset/brcm,bcm4908-usb-reset.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Broadcom BCM4908 USB host controller reset
>>>> +
>>>> +description: >
>>>> + BCM4908 has a separated block controlling all USB controllers. It handles the
>>>> + whole setup process and takes care of initializing PHYs at the right time
>>>> + (state).
>>>> +
>>>> +maintainers:
>>>> + - Rafał Miłecki <rafal@xxxxxxxxxx>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + enum:
>>>> + - brcm,bcm4908-usb-reset
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + resets:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> +
>>>> + phys:
>>>> + minItems: 2
>>>> + maxItems: 2
>>>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +
>>>> + phy-names:
>>>> + items:
>>>> + - const: usb2
>>>> + - const: usb3
>>>> +
>>>> + "#reset-cells":
>>>> + const: 0
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - phys
>>>> + - phy-names
>>>> + - "#reset-cells"
>>>> +
>>>> +additionalProperties: true
>>>> +
>>>> +examples:
>>>> + - |
>>>> + reset-controller@8000c200 {
>>>> + compatible = "brcm,bcm4908-usb-reset";
>>>> + reg = <0x8000c200 0x100>;
>>>> +
>>>> + phys = <&usb2_phy>, <&usb3_phy>;
>>>> + phy-names = "usb2", "usb3";
>>>
>>> This looks quite unusual, usually the *HCI controllers would be
>>> consumers of the PHY and the PHY may be a consumer of the reset controller.
>>>
>>> (still going through my emails have not fully read your separate email
>>> on the topic, so pardon me if this is being discussed twice).
>>
>> I agree, it's the the best solution I found for this specific design.
>>
>> This specific hw block perform various operations before, in the middle and
>> after PHY initialization. That made me make reset controlller initialize PHYs.
>>
>> I'm happy to implement a more proper design if someone can just suggest how.
>> I don't have any better idea :(
>
> So the reset controller block has more than just resets? I'd hide all
> this in the phy driver rather than hide the phy in the reset driver. So
> for DT provide the phy a phandle to the reset block to poke the
> registers directly.
Rafal was pointed to drivers/phy/broadcom/phy-brcm-usb-init.c which
should be a good starting point for driving the 4908 USB PHY. From there
the USB PHY may optionally be a consumer of a reset controller if it
needs to, not clear if that is necessary.
--
Florian