RE: [v2,3/3] dt-bindings: ata: ahci_tegra: Add tegra210 AHCI

[Preetham Chandru <pchandru-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>]

>-----Original Message-----
>From: Jonathan Hunter
>Sent: Monday, November 28, 2016 7:22 PM
>To: Mikko Perttunen; Preetham Chandru; tj@kernel.org;
>swarren@wwwdotorg.org; thierry.reding@gmail.com;
>preetham260@gmail.com
>Cc: Laxman Dewangan; linux-ide@vger.kernel.org; Venu Byravarasu; Pavan
>Kunapuli; linux-tegra@vger.kernel.org
>Subject: Re: [v2,3/3] dt-bindings: ata: ahci_tegra: Add tegra210 AHCI
>
>
>On 28/11/16 13:05, Mikko Perttunen wrote:
>> On 24.11.2016 09:43, PREETHAM RAMACHANDRA wrote:
>>> From: Preetham Chandru R <pchandru@nvidia.com>
>
>I did not receive the original (please CC linux-tegra as well if you did not
>originally), but there should be some description here.
>
>>> Signed-off-by: Preetham Chandru R <pchandru@nvidia.com>
>>> ---
>>>  .../bindings/ata/nvidia,tegra124-ahci.txt          | 48
>>> ++++++++++++++++------
>>>  1 file changed, 36 insertions(+), 12 deletions(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> b/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> index 66c83c3..446214f 100644
>>> --- a/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> +++ b/Documentation/devicetree/bindings/ata/nvidia,tegra124-ahci.txt
>>> @@ -1,9 +1,9 @@
>>> -Tegra124 SoC SATA AHCI controller
>>> +Tegra SoC SATA AHCI controller
>>>
>>>  Required properties :
>>> -- compatible : For Tegra124, must contain "nvidia,tegra124-ahci".
>>> Otherwise,
>>> -  must contain '"nvidia,<chip>-ahci", "nvidia,tegra124-ahci"', where
>>> <chip>
>>> -  is tegra132.
>>> +- compatible : Must be one of:
>>> +  - Tegra124 : "nvidia,tegra124-ahci"
>>> +  - Tegra210 : "nvidia,tegra210-ahci"
>>>  - reg : Should contain 2 entries:
>>>    - AHCI register set (SATA BAR5)
>>>    - SATA register set
>>
>> I think you added a new set in the first patch, AUX; you should add it
>> here as well.
>>
>>> @@ -13,8 +13,6 @@ Required properties :
>>>  - clock-names : Must include the following entries:
>>>    - sata
>>>    - sata-oob
>>> -  - cml1
>>> -  - pll_e
>>>  - resets : Must contain an entry for each entry in reset-names.
>>>    See ../reset/reset.txt for details.
>>>  - reset-names : Must include the following entries:
>>> @@ -24,9 +22,35 @@ Required properties :
>>>  - phys : Must contain an entry for each entry in phy-names.
>>>    See ../phy/phy-bindings.txt for details.
>>>  - phy-names : Must include the following entries:
>>> -  - sata-phy : XUSB PADCTL SATA PHY
>>> -- hvdd-supply : Defines the SATA HVDD regulator
>>> -- vddio-supply : Defines the SATA VDDIO regulator
>>> -- avdd-supply : Defines the SATA AVDD regulator
>>> -- target-5v-supply : Defines the SATA 5V power regulator
>>> -- target-12v-supply : Defines the SATA 12V power regulator
>>> +  - For T124:
>>> +    - sata-phy : XUSB PADCTL SATA PHY
>>> +  - For T210:
>>> +    - sata-0
>>> +- For T124:
>>> +  - hvdd-supply : Defines the SATA HVDD regulator
>>> +  - vddio-supply : Defines the SATA VDDIO regulator
>>> +  - avdd-supply : Defines the SATA AVDD regulator
>>> +  - target-5v-supply : Defines the SATA 5V power regulator
>>> +- For T210:
>>> +  - l0-hvddio-sata-supply : Defines the SATA HVDDIO regulator
>>> +  - l0-dvddio-sata-supply : Defines the SATA DVDDIO regulator
>>> +  - hvdd-pex-pll-e-supply : Defines the PEX PLL_E regulator
>>> +  - dvdd-sata-pll-supply  : Defines the SATA PLL regulator
>>> +  - hvdd-sata-supply      : Defines the SATA HVDD regulator
>>> +- nvidia,disable-features : Must include the following entries:
>>> +  - devslp
>>> +  - dipm
>
>My understanding is that the AHCI controller requires the SATA powergate
>to be enabled. Now we have support for powergates via the genpd
>framework we should add the 'power-domains' property for this device.
>
>Do you know if there is any sequencing requirement with regard to powering
>on the above rails and the powergate? If not we should check as we should
>ensure that we have the proper sequencing.
>

All the regulators mentioned under T210 section are related to phy and they need to be enabled first.  
I will add the power-domain support for AHCI in a different patch. 

>Cheers
>Jon
>
>--
>nvpublic