Hi Rob,
Thank you, I will send next patch set with the changes as you suggested.
Regards,
Srinath.
On Tue, Mar 12, 2019 at 3:00 AM Rob Herring <r...@kernel.org> wrote:
>
> 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
