Re: [PATCH 1/2] dt-bindings: reset: document Broadcom's BCM4908 USB reset binding

[Rob Herring]

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.

Rob