[PATCH] libata, dt: Add clocks to sata_rcar bindings

[PATCH] libata, dt: Add clocks to sata_rcar bindings

[Valentine Barshak]

Now that the clocks are available in the R-Car Gen2 DT,
add clocks property description to the sata_rcar bindings.
The clocks have been tested on r8a7791 so we use that
as an example of the R-Car SATA node.

The patch is against for-next branch of the libata git repo.

Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
---
 Documentation/devicetree/bindings/ata/sata_rcar.txt | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 1e61113..6da60c0 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -7,12 +7,14 @@ Required properties:
 			  - "renesas,sata-r8a7791" for R-Car M2
 - reg			: address and length of the SATA registers;
 - interrupts		: must consist of one interrupt specifier.
+- clocks		: must contain a phandle and clock-specifier pair.
 
 Example:
 
-sata: sata@fc600000 {
-	compatible = "renesas,sata-r8a7779";
-	reg = <0xfc600000 0x2000>;
+sata0: sata@ee300000 {
+	compatible = "renesas,sata-r8a7791";
+	reg = <0 0xee300000 0 0x2000>;
 	interrupt-parent = <&gic>;
-	interrupts = <0 100 IRQ_TYPE_LEVEL_HIGH>;
+	interrupts = <0 105 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&mstp8_clks R8A7791_CLK_SATA0>;
 };
-- 
1.8.3.1

Re: [PATCH] libata, dt: Add clocks to sata_rcar bindings

[Laurent Pinchart]

Hi Valentine,

Thank you for the patch.

On Friday 17 January 2014 02:07:42 Valentine Barshak wrote:
> Now that the clocks are available in the R-Car Gen2 DT,
> add clocks property description to the sata_rcar bindings.
> The clocks have been tested on r8a7791 so we use that
> as an example of the R-Car SATA node.
> 
> The patch is against for-next branch of the libata git repo.
> 
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> ---
>  Documentation/devicetree/bindings/ata/sata_rcar.txt | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> 1e61113..6da60c0 100644
> --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> @@ -7,12 +7,14 @@ Required properties:
>  			  - "renesas,sata-r8a7791" for R-Car M2
>  - reg			: address and length of the SATA registers;
>  - interrupts		: must consist of one interrupt specifier.
> +- clocks		: must contain a phandle and clock-specifier pair.

I would say "must contain a reference to the functional clock.", as the clock 
could be referenced by a phandle only depending on the SATA IP core 
integration in the SoC.

Wouldn't it be time to have standard wordings for clocks (and interrupt) 
bindings ? 

>  Example:
> 
> -sata: sata@fc600000 {
> -	compatible = "renesas,sata-r8a7779";
> -	reg = <0xfc600000 0x2000>;
> +sata0: sata@ee300000 {
> +	compatible = "renesas,sata-r8a7791";
> +	reg = <0 0xee300000 0 0x2000>;
>  	interrupt-parent = <&gic>;
> -	interrupts = <0 100 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupts = <0 105 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&mstp8_clks R8A7791_CLK_SATA0>;
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH] libata, dt: Add clocks to sata_rcar bindings

[Mark Rutland]

On Sun, Jan 19, 2014 at 08:16:42PM +0000, Laurent Pinchart wrote:
> Hi Valentine,
> 
> Thank you for the patch.
> 
> On Friday 17 January 2014 02:07:42 Valentine Barshak wrote:
> > Now that the clocks are available in the R-Car Gen2 DT,
> > add clocks property description to the sata_rcar bindings.
> > The clocks have been tested on r8a7791 so we use that
> > as an example of the R-Car SATA node.
> > 
> > The patch is against for-next branch of the libata git repo.
> > 
> > Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> > ---
> >  Documentation/devicetree/bindings/ata/sata_rcar.txt | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> > 1e61113..6da60c0 100644
> > --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > @@ -7,12 +7,14 @@ Required properties:
> >  			  - "renesas,sata-r8a7791" for R-Car M2
> >  - reg			: address and length of the SATA registers;
> >  - interrupts		: must consist of one interrupt specifier.
> > +- clocks		: must contain a phandle and clock-specifier pair.
> 
> I would say "must contain a reference to the functional clock.", as the clock 
> could be referenced by a phandle only depending on the SATA IP core 
> integration in the SoC.

In that case the clock-specifier is simply zero cells (though admittedly
a pair including a zero-cells element is a bit odd).

The wording in the patch is consistent with the form I've been
recommending elsewhere:

- clocks: A list of phandles + clock-specifier pairs, one for each
  entry in clock-names.
- clock-names: Should contain:
  * "fclk" - the functional clock
  * "other_clk" - some other clock.

> 
> Wouldn't it be time to have standard wordings for clocks (and interrupt) 
> bindings ? 

I would certainly like to see consistent wording across bindings
(especially for interrupts given the addition of the interrupts-extended
binding).

Cheers,
Mark.

Re: [PATCH] libata, dt: Add clocks to sata_rcar bindings

[Laurent Pinchart]

Hi Mark,

On Monday 20 January 2014 09:31:19 Mark Rutland wrote:
> On Sun, Jan 19, 2014 at 08:16:42PM +0000, Laurent Pinchart wrote:
> > On Friday 17 January 2014 02:07:42 Valentine Barshak wrote:
> > > Now that the clocks are available in the R-Car Gen2 DT,
> > > add clocks property description to the sata_rcar bindings.
> > > The clocks have been tested on r8a7791 so we use that
> > > as an example of the R-Car SATA node.
> > > 
> > > The patch is against for-next branch of the libata git repo.
> > > 
> > > Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/ata/sata_rcar.txt | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > > b/Documentation/devicetree/bindings/ata/sata_rcar.txt index
> > > 1e61113..6da60c0 100644
> > > --- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > > +++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
> > > 
> > > @@ -7,12 +7,14 @@ Required properties:
> > >  			  - "renesas,sata-r8a7791" for R-Car M2
> > >  - reg			: address and length of the SATA registers;
> > >  - interrupts		: must consist of one interrupt specifier.
> > > +- clocks		: must contain a phandle and clock-specifier pair.
> > 
> > I would say "must contain a reference to the functional clock.", as the
> > clock could be referenced by a phandle only depending on the SATA IP core
> > integration in the SoC.
> 
> In that case the clock-specifier is simply zero cells (though admittedly
> a pair including a zero-cells element is a bit odd).
> 
> The wording in the patch is consistent with the form I've been
> recommending elsewhere:
> 
> - clocks: A list of phandles + clock-specifier pairs, one for each
>   entry in clock-names.
> - clock-names: Should contain:
>   * "fclk" - the functional clock
>   * "other_clk" - some other clock.
> 
> > Wouldn't it be time to have standard wordings for clocks (and interrupt)
> > bindings ?
> 
> I would certainly like to see consistent wording across bindings
> (especially for interrupts given the addition of the interrupts-extended
> binding).

What about creating a new file in Documentation/devicetree/bindings/ with 
reference wordings for common properties ?

-- 
Regards,

Laurent Pinchart