RE: [PATCH v2 06/11] dt-bindings: usb: generic-ohci: Document RZ/G2L SoC bindings

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH v2 06/11] dt-bindings: usb: generic-ohci: Document
> RZ/G2L SoC bindings
> 
> On Mon, Jun 21, 2021 at 10:39:38AM +0100, Biju Das wrote:
> > Renesas RZ/G2L SoC has USBPHY Control and USB2.0 PHY module. We need
> > to turn on both these phy modules before accessing host registers.
> >
> > Apart from this, document the optional property dr_mode present on
> > both
> > RZ/G2 and R-Car Gen3 SoCs.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/usb/generic-ohci.yaml | 32
> > +++++++++++++++++--
> >  1 file changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > index 0f5f6ea702d0..c0644fae5db9 100644
> > --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > @@ -8,6 +8,26 @@ title: USB OHCI Controller Device Tree Bindings
> >
> >  allOf:
> >    - $ref: "usb-hcd.yaml"
> > +  - if:
> > +      properties:
> > +        compatible:
> > +            contains:
> > +              const: renesas,r9a07g044-ohci
> > +    then:
> > +      properties:
> > +        phys:
> > +          maxItems: 2
> > +        phy-names:
> > +          items:
> > +            - const: usbphyctrl
> > +            - const: usb
> 
> Why can't your extra thing be last? Then you only need to set
> minItems/maxItems in the if/then schema.

OK. will move this to the last and will take out phy-names.

> 
> Though this seems like an abuse of the phy binding. There's not 2 phys,
> right? Just some extra registers related to the phy? Can't it be hidden in
> your phy driver?

Usbphyctrl is separate IP block which mainly controls reset and power down of the actual USB/PHY.
This block has separate registers. So modelled as separate driver and device handles port reset
Based on port index.

Will remove phy-names "usbphyctrl" and will just use "usb" like example here[1]. Is it ok?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/usb/usb-hcd.yaml?h=v5.13-rc7

Regards,
Biju


> 
> > +    else:
> > +      properties:
> > +        phys:
> > +          maxItems: 1
> > +        phy-names:
> > +          items:
> > +            - const: usb
> >
> >  maintainers:
> >    - Greg Kroah-Hartman <gregkh@linuxfoundation.org> @@ -43,6 +63,7 @@
> > properties:
> >                - brcm,bcm7435-ohci
> >                - ibm,476gtr-ohci
> >                - ingenic,jz4740-ohci
> > +              - renesas,r9a07g044-ohci
> >                - snps,hsdk-v1.0-ohci
> >            - const: generic-ohci
> >        - const: generic-ohci
> > @@ -101,14 +122,21 @@ properties:
> >        Overrides the detected port count
> >
> >    phys:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> >
> >    phy-names:
> > -    const: usb
> > +    minItems: 1
> > +    maxItems: 2
> >
> >    iommus:
> >      maxItems: 1
> >
> > +  dr_mode:
> > +    enum:
> > +      - host
> > +      - otg
> > +
> >  required:
> >    - compatible
> >    - reg
> > --
> > 2.17.1
> >
> >