On Thu, Dec 05, 2013 at 11:33:46AM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 04 December 2013 10:46 PM, Andrew Lunn wrote:
> > Describe the binding for the Marvell MVEBU SATA phy. This driver
> > can be used at least with Kirkwood, Dove and maybe others.
> > Additionally, update the SATA binding with the properties to link
> > to the phy nodes.
> > 
> > Signed-off-by: Andrew Lunn <[email protected]>
> > ---
> >  Documentation/devicetree/bindings/ata/marvell.txt  |  6 ++++++
> >  .../devicetree/bindings/phy/phy-mvebu-sata.txt     | 22 
> > ++++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/marvell.txt 
> > b/Documentation/devicetree/bindings/ata/marvell.txt
> > index b5cdd20cde9c..e072fa105b49 100644
> > --- a/Documentation/devicetree/bindings/ata/marvell.txt
> > +++ b/Documentation/devicetree/bindings/ata/marvell.txt
> > @@ -6,11 +6,17 @@ Required Properties:
> >  - interrupts    : Interrupt controller is using
> >  - nr-ports      : Number of SATA ports in use.
> >  
> > +Optional Properties:
> > +- phys             : List of phandles to sata phys
> > +- phy-names        : Should be "0", "1", etc, one number per phandle
> 
> over aligned..

Could you explain please? Here is what it looks like without the patch
formatting, which could be messing up the display of tabs, due to the
+ at the beginning:

Required Properties:
- compatibility : "marvell,orion-sata"
- reg           : Address range of controller
- interrupts    : Interrupt controller is using
- nr-ports      : Number of SATA ports in use.

Optional Properties:
- phys          : List of phandles to sata phys
- phy-names     : Should be "0", "1", etc, one number per phandle



> > +
> >  Example:
> >  
> >     sata@80000 {
> >             compatible = "marvell,orion-sata";
> >             reg = <0x80000 0x5000>;
> >             interrupts = <21>;
> > +           phys = <&sata_phy0>, <&sata_phy1>;
> > +           phy-names = "0", "1";
> 
> more descriptive phy-names? sata-phy0?
> >             nr-ports = <2>;

I could do, but i was following how the clocks work. Unfortunately,
the binding documentation is out of date and does not contain
clocks. A real example is:

                sata@80000 {
                        compatible = "marvell,orion-sata";
                        reg = <0x80000 0x5000>;
                        interrupts = <21>;
                        clocks = <&gate_clk 14>, <&gate_clk 15>;
                        clock-names = "0", "1";
                        phys = <&sata_phy0>, <&sata_phy1>;
                        phy-names = "0", "1";
                        status = "disabled";
                };

So clocks and the phy are described nearly identically. I can however
handle phys differently if you wish.

I will also submit a separate patch to the binding documentation to
add the clocks.


> >     }
> > diff --git a/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt 
> > b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> > new file mode 100644
> > index 000000000000..1cf9cef50b4b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-mvebu-sata.txt
> 
> Just name this mvebu-phy.txt so that we can add bindings of other mvebu PHYs
> here when it's added.

O.K. I also have a pcie phy driver, but it does not work
yet. devm_phy_get() is too restrictive for a complex device like a
PCIe controller.

> > +           sata-phy@1 {
> 
> The value after '@' must match the first address specified in the reg property
> of the node according to the ePAPR spec.

O.K, will fix.

> > +                   compatible = "marvell,mvebu-sata-phy";
> > +                   reg = <0x84000 0x0334>;
> > +                   clocks = <&gate_clk 15>;
> > +                   clock-names = "sata";
> > +                   #phy-cells = <1>;
> 
> Is it on purpose that your are having phy-cells value to 1?

Yes. Each instance only controls one phy.

     Thanks
        Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to