On Thu, Dec 05, 2013 at 12:50:03PM +0000, Andrew Lunn wrote: > On Thu, Dec 05, 2013 at 12:22:54PM +0000, Mark Rutland wrote: > > On Thu, Dec 05, 2013 at 11:51:11AM +0000, Andrew Lunn wrote: > > > The marvell SATA driver can optionally make use of clocks specified in > > > the DT node. This has been available in the driver since Febuary 2012, > > > but the documentation is missing from the binding. Add it. > > > > > > Signed-off-by: Andrew Lunn <andrew@xxxxxxx> > > > --- > > > FYI: This will cause merge conflicts with the SATA PHY driver comming soon. > > > --- > > > Documentation/devicetree/bindings/ata/marvell.txt | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/ata/marvell.txt b/Documentation/devicetree/bindings/ata/marvell.txt > > > index b5cdd20cde9c..f6c68d157749 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: > > > +- clocks : List of pHandles to clocks > > > > s/pHandle/phandle/. Don't forget the clock-specifier too! > > > > > +- clock-names : Must be "0", "1", mapping port number to clock. > > > > This line leads to the erroneous impression that the clocks can be in > > arbitrary order (as is generally true for bindings with clock-names > > properties). The driver doesn't request clocks by name, and doesn't even > > look at clock-names. > > The driver does: > > sprintf(port_number, "%d", port); > hpriv->port_clks[port] = clk_get(&pdev->dev, port_number); > Apologies, I'd misread the code. > and: > > 153 struct clk *clk_get(struct device *dev, const char *con_id) > 154 { > 155 const char *dev_id = dev ? dev_name(dev) : NULL; > 156 struct clk *clk; > 157 > 158 if (dev) { > 159 clk = of_clk_get_by_name(dev->of_node, con_id); > 160 if (!IS_ERR(clk) && __clk_get(clk)) > 161 return clk; > 162 } > 163 > 164 return clk_get_sys(dev_id, con_id); > 165 } > > So the names are used. And since names are used, the ordering is > arbitrary. > > > I also think the names could be improved, albeit slightly ("port0" is a > > little clearer than "0"). > > I could do this, but this binding has been in use for well over a year > now. I'm just retro-actively documenting it. I can add support for > both "0" and "port0", in order to keep backwards compatibility, but it > obviously makes the driver messy. Is it worth it? No, there's no point adding more names. Apologies for the confusion. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html