On Sun, Mar 10, 2019 at 10:32 PM Srinath Mannam
<srinath.man...@broadcom.com> wrote:
>
> Hi Rob,
>
> Please find my comments below,
>
> On Sat, Feb 23, 2019 at 1:05 AM Rob Herring <r...@kernel.org> wrote:
> >
> > On Fri, Feb 22, 2019 at 11:29 AM Srinath Mannam
> > <srinath.man...@broadcom.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > Thanks for the review, Please find my comments below in line.
> > >
> > > On Fri, Feb 22, 2019 at 10:50 PM Rob Herring <r...@kernel.org> wrote:
> > > >
> > > > On Wed, Feb 20, 2019 at 04:04:00PM +0530, Srinath Mannam wrote:
> > > > > Add DT binding document for Stingray USB PHY.
> > > > >
> > > > > Signed-off-by: Srinath Mannam <srinath.man...@broadcom.com>
> > > > > Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
> > > > > Reviewed-by: Scott Branden <scott.bran...@broadcom.com>
> > > > > ---
> > > > >  .../bindings/phy/brcm,stingray-usb-phy.txt         | 62 
> > > > > ++++++++++++++++++++++
> > > > >  1 file changed, 62 insertions(+)
> > > > >  create mode 100644 
> > > > > Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
> > > > >
> > > > > diff --git 
> > > > > a/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt 
> > > > > b/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
> > > > > new file mode 100644
> > > > > index 0000000..da19236
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/phy/brcm,stingray-usb-phy.txt
> > > > > @@ -0,0 +1,62 @@
> > > > > +Broadcom Stingray USB PHY
> > > > > +
> > > > > +Required properties:
> > > > > + - compatible : should be one of the listed compatibles
> > > > > +     - "brcm,sr-usb-combo-phy" is a combo PHY has one SS PHY and one 
> > > > > HS PHY.
> > > > > +     - "brcm,sr-usb-hs-phy" has a single HS PHY.
> > > > > + - reg: offset and length of the PHY blocks registers
> > > > > + - address-cells: should be 1
> > > > > + - size-cells: should be 0
> > > > > +
> > > > > +Sub-nodes:
> > > > > +  brcm,sr-usb-combo-phy have two sub-nodes for one SS PHY and one HS 
> > > > > PHY.
> > > > > +
> > > > > +Sub-nodes required properties:
> > > > > + - reg: required for brcm,sr-usb-phy model PHY.
> > > > > +     reg value 0 is HS PHY and 1 is SS PHY.
> > > > > + - phy-cells: generic PHY binding; must be 0
> > > > > +
> > > > > +Refer to phy/phy-bindings.txt for the generic PHY binding properties
> > > > > +
> > > > > +Example:
> > > > > +     usbphy0: usb-phy@0 {
> > > > > +             compatible = "brcm,sr-usb-combo-phy";
> > > > > +             reg = <0x00000000 0x100>;
> > > > > +             #address-cells = <1>;
> > > > > +             #size-cells = <0>;
> > > > > +
> > > > > +             usb0_phy0: phy@0 {
> > > > > +                     reg = <0>;
> > > > > +                     #phy-cells = <0>;
> > > > > +             };
> > > > > +
> > > > > +             usb0_phy1: phy@1 {
> > > > > +                     reg = <1>;
> > > > > +                     #phy-cells = <0>;
> > > > > +             };
> > > >
> > > > Again, you don't need child nodes here. There are not any per child
> > > > resources. Clients can refer to <&usbphy0 1> just as easily as
> > > > <&usb0_phy1>. This is why we have #phy-cells.
> > > This phy controller is combo PHY it has one Super Speed USB PHY and
> > > one High Speed USB PHY.
> > > We required to create two PHY devices inside driver to initialize and
> > > service(reset) both SS and HS PHYs separately.
> > > That is the reason we used two child nodes.
> >
> > What you do in the driver is your business. That is independent of the
> > binding. Go look at other phy drivers which use #phy-cells=1.
> > .of_xlate() function is what converts the phy cells to a struct phy.
> >
> I have followed exactly same pattern available in open source.
> ex: Documentation/devicetree/bindings/phy/brcm-sata-phy.txt
> In this also, two child nodes are used with #phy-cells 0.

You'll notice DT maintainers did not review that binding (only changes
to it). There's no shortage of DT examples of how not to do things.

Rob

Reply via email to