diff mbox

[1/3] Documentation: Update Arasan SDHC documentation for the APM X-Gene SoC SDHC DTS binding

Message ID 1400398918-1502-2-git-send-email-lho@apm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Loc Ho May 18, 2014, 7:41 a.m. UTC
This patch updates Arasan SDHC documentation for the APM X-Gene SoC SDHC controller
DTS binding.

Signed-off-by: Loc Ho <lho@apm.com>
---
 .../devicetree/bindings/mmc/arasan,sdhci.txt       |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

Comments

Arnd Bergmann May 19, 2014, 9:09 a.m. UTC | #1
On Sunday 18 May 2014 01:41:56 Loc Ho wrote:
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -8,14 +8,17 @@ Device Tree Bindings for the Arasan SDHCI Controller
>    [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>  
>  Required Properties:
> -  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a'
> +  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
> +                'xgene,arasan,sdhci-8.9a'
>    - reg: From mmc bindings: Register location and length.
> -  - clocks: From clock bindings: Handles to clock inputs.
> -  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
> 

The driver as you patch is in this version actually requires a second set
of registers. As I commented there, it's probably not the best way to do
things, but you also forgot to update the binding here.

Always make sure that the binding document actually documents what you
implement!

	Arnd
Mark Rutland May 19, 2014, 12:32 p.m. UTC | #2
On Sun, May 18, 2014 at 08:41:56AM +0100, Loc Ho wrote:
> This patch updates Arasan SDHC documentation for the APM X-Gene SoC SDHC controller
> DTS binding.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> ---
>  .../devicetree/bindings/mmc/arasan,sdhci.txt       |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 98ee2ab..8fc4cc4 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -8,14 +8,17 @@ Device Tree Bindings for the Arasan SDHCI Controller
>    [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>  
>  Required Properties:
> -  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a'
> +  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
> +                'xgene,arasan,sdhci-8.9a'

"xgene" is not a vendor, nor is it documented as such.

>    - reg: From mmc bindings: Register location and length.
> -  - clocks: From clock bindings: Handles to clock inputs.
> -  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
>    - interrupts: Interrupt specifier
>    - interrupt-parent: Phandle for the interrupt controller that services
>  		      interrupts for this device.
>  
> +Optional Properties:
> +  - clocks: From clock bindings: Handles to clock inputs.
> +  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"

Given that these used to be required, they are presumably still required
in some cases?

If we're going to alter these lines, it'd be nice to clean them up at
the same time (a list is not a tuple, clocks aren't just phandles).

Cheers,
Mark.
Loc Ho May 29, 2014, 6:54 p.m. UTC | #3
Hi Mark,

>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> index 98ee2ab..8fc4cc4 100644
>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> @@ -8,14 +8,17 @@ Device Tree Bindings for the Arasan SDHCI Controller
>>    [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>>
>>  Required Properties:
>> -  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a'
>> +  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
>> +                'xgene,arasan,sdhci-8.9a'
>
> "xgene" is not a vendor, nor is it documented as such.

I will change to "apm".

>
>>    - reg: From mmc bindings: Register location and length.
>> -  - clocks: From clock bindings: Handles to clock inputs.
>> -  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
>>    - interrupts: Interrupt specifier
>>    - interrupt-parent: Phandle for the interrupt controller that services
>>                     interrupts for this device.
>>
>> +Optional Properties:
>> +  - clocks: From clock bindings: Handles to clock inputs.
>> +  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
>
> Given that these used to be required, they are presumably still required
> in some cases?

From the driver point of view, it is optional as it no longer requires
this. But from usage point of view, if an HW requires this to work
correctly, then it musts be provided.

>
> If we're going to alter these lines, it'd be nice to clean them up at
> the same time (a list is not a tuple, clocks aren't just phandles).
>

I will update this.

-Loc
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 98ee2ab..8fc4cc4 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -8,14 +8,17 @@  Device Tree Bindings for the Arasan SDHCI Controller
   [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 
 Required Properties:
-  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a'
+  - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
+                'xgene,arasan,sdhci-8.9a'
   - reg: From mmc bindings: Register location and length.
-  - clocks: From clock bindings: Handles to clock inputs.
-  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
   - interrupts: Interrupt specifier
   - interrupt-parent: Phandle for the interrupt controller that services
 		      interrupts for this device.
 
+Optional Properties:
+  - clocks: From clock bindings: Handles to clock inputs.
+  - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
+
 Example:
 	sdhci@e0100000 {
 		compatible = "arasan,sdhci-8.9a";