diff mbox

[2/2] ARM: dts: socfpga: Add a 3.3V fixed regulator node

Message ID 1413493413-8041-3-git-send-email-dinguyen@opensource.altera.com (mailing list archive)
State New, archived
Headers show

Commit Message

dinguyen@opensource.altera.com Oct. 16, 2014, 9:03 p.m. UTC
From: Dinh Nguyen <dinguyen@opensource.altera.com>

Without the 3.3V regulator node, the SDMMC driver will give these warnings:

dw_mmc ff704000.dwmmc0: No vmmc regulator found
dw_mmc ff704000.dwmmc0: No vqmmc regulator found

This patch adds the regulator node, and points the SD/MMC to the regulator.

Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
 arch/arm/boot/dts/socfpga_arria5.dtsi         | 11 ++++++++++-
 arch/arm/boot/dts/socfpga_arria5_socdk.dts    |  5 +++++
 arch/arm/boot/dts/socfpga_cyclone5.dtsi       |  9 +++++++++
 arch/arm/boot/dts/socfpga_cyclone5_socdk.dts  |  2 ++
 arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |  5 +++++
 5 files changed, 31 insertions(+), 1 deletion(-)

Comments

Jaehoon Chung Oct. 17, 2014, 1:43 a.m. UTC | #1
Hi, Dinh.

On 10/17/2014 06:03 AM, dinguyen@opensource.altera.com wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
> 
> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
> 
> dw_mmc ff704000.dwmmc0: No vmmc regulator found
> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
> 
> This patch adds the regulator node, and points the SD/MMC to the regulator.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
>  arch/arm/boot/dts/socfpga_arria5.dtsi         | 11 ++++++++++-
>  arch/arm/boot/dts/socfpga_arria5_socdk.dts    |  5 +++++
>  arch/arm/boot/dts/socfpga_cyclone5.dtsi       |  9 +++++++++
>  arch/arm/boot/dts/socfpga_cyclone5_socdk.dts  |  2 ++
>  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |  5 +++++
>  5 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
> index 03e8268..8093781 100644
> --- a/arch/arm/boot/dts/socfpga_arria5.dtsi
> +++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
> @@ -29,7 +29,7 @@
>  			};
>  		};
>  
> -		dwmmc0@ff704000 {
> +		mmc0: dwmmc0@ff704000 {
>  			num-slots = <1>;
>  			broken-cd;
>  			bus-width = <4>;
> @@ -41,4 +41,13 @@
>  			cpu1-start-addr = <0xffd080c4>;
>  		};
>  	};
> +
> +	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "HPS 3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;

Always-on is right?

> +	};
>  };
> diff --git a/arch/arm/boot/dts/socfpga_arria5_socdk.dts b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
> index 27d551c..b7e4023 100644
> --- a/arch/arm/boot/dts/socfpga_arria5_socdk.dts
> +++ b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
> @@ -68,6 +68,11 @@
>  	};
>  };
>  
> +&mmc0 {
> +	vmmc-supply = <&regulator_3_3v_hps>;
> +	vqmmc-supply = <&regulator_3_3v_hps>;
> +};
> +
>  &usb1 {
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dtsi b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
> index 28c05e7..743bc3b 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5.dtsi
> +++ b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
> @@ -48,4 +48,13 @@
>  			cpu1-start-addr = <0xffd080c4>;
>  		};
>  	};
> +
> +	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "HPS 3.3V";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-boot-on;
> +		regulator-always-on;
> +	};
>  };
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> index 739c3b7..e1f56ba 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
> @@ -70,6 +70,8 @@
>  
>  &mmc0 {
>  	cd = <&gpio1 18 0>;
> +	vmmc-supply = <&regulator_3_3v_hps>;
> +	vqmmc-supply = <&regulator_3_3v_hps>;

Is vmmc and vqmmc  used the common supply?

Best Regards,
Jaehoon Chung

>  };
>  
>  &usb1 {
> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
> index d26f155..ea0c454 100644
> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
> @@ -53,6 +53,11 @@
>  	rxc-skew-ps = <2000>;
>  };
>  
> +&mmc0 {
> +	vmmc-supply = <&regulator_3_3v_hps>;
> +	vqmmc-supply = <&regulator_3_3v_hps>;
> +};
> +
>  &usb1 {
>  	status = "okay";
>  };
>
dinguyen@opensource.altera.com Oct. 17, 2014, 1:37 p.m. UTC | #2
On 10/16/14, 8:43 PM, Jaehoon Chung wrote:
> Hi, Dinh.
> 
> On 10/17/2014 06:03 AM, dinguyen@opensource.altera.com wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>>
>> dw_mmc ff704000.dwmmc0: No vmmc regulator found
>> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>>
>> This patch adds the regulator node, and points the SD/MMC to the regulator.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
>> ---
>>  arch/arm/boot/dts/socfpga_arria5.dtsi         | 11 ++++++++++-
>>  arch/arm/boot/dts/socfpga_arria5_socdk.dts    |  5 +++++
>>  arch/arm/boot/dts/socfpga_cyclone5.dtsi       |  9 +++++++++
>>  arch/arm/boot/dts/socfpga_cyclone5_socdk.dts  |  2 ++
>>  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |  5 +++++
>>  5 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
>> index 03e8268..8093781 100644
>> --- a/arch/arm/boot/dts/socfpga_arria5.dtsi
>> +++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
>> @@ -29,7 +29,7 @@
>>  			};
>>  		};
>>  
>> -		dwmmc0@ff704000 {
>> +		mmc0: dwmmc0@ff704000 {
>>  			num-slots = <1>;
>>  			broken-cd;
>>  			bus-width = <4>;
>> @@ -41,4 +41,13 @@
>>  			cpu1-start-addr = <0xffd080c4>;
>>  		};
>>  	};
>> +
>> +	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "HPS 3.3V";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
> 
> Always-on is right?

yes, it's always on.

> 
>> +	};
>>  };
>> diff --git a/arch/arm/boot/dts/socfpga_arria5_socdk.dts b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
>> index 27d551c..b7e4023 100644
>> --- a/arch/arm/boot/dts/socfpga_arria5_socdk.dts
>> +++ b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
>> @@ -68,6 +68,11 @@
>>  	};
>>  };
>>  
>> +&mmc0 {
>> +	vmmc-supply = <&regulator_3_3v_hps>;
>> +	vqmmc-supply = <&regulator_3_3v_hps>;
>> +};
>> +
>>  &usb1 {
>>  	status = "okay";
>>  };
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dtsi b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
>> index 28c05e7..743bc3b 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5.dtsi
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
>> @@ -48,4 +48,13 @@
>>  			cpu1-start-addr = <0xffd080c4>;
>>  		};
>>  	};
>> +
>> +	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "HPS 3.3V";
>> +		regulator-min-microvolt = <3300000>;
>> +		regulator-max-microvolt = <3300000>;
>> +		regulator-boot-on;
>> +		regulator-always-on;
>> +	};
>>  };
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>> index 739c3b7..e1f56ba 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
>> @@ -70,6 +70,8 @@
>>  
>>  &mmc0 {
>>  	cd = <&gpio1 18 0>;
>> +	vmmc-supply = <&regulator_3_3v_hps>;
>> +	vqmmc-supply = <&regulator_3_3v_hps>;
> 
> Is vmmc and vqmmc  used the common supply?
> 

Yes..

Dinh
Doug Anderson Oct. 17, 2014, 4:57 p.m. UTC | #3
Dinh,

On Thu, Oct 16, 2014 at 2:03 PM,  <dinguyen@opensource.altera.com> wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>
> dw_mmc ff704000.dwmmc0: No vmmc regulator found
> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>
> This patch adds the regulator node, and points the SD/MMC to the regulator.
>
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
>  arch/arm/boot/dts/socfpga_arria5.dtsi         | 11 ++++++++++-
>  arch/arm/boot/dts/socfpga_arria5_socdk.dts    |  5 +++++
>  arch/arm/boot/dts/socfpga_cyclone5.dtsi       |  9 +++++++++
>  arch/arm/boot/dts/socfpga_cyclone5_socdk.dts  |  2 ++
>  arch/arm/boot/dts/socfpga_cyclone5_sockit.dts |  5 +++++
>  5 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
> index 03e8268..8093781 100644
> --- a/arch/arm/boot/dts/socfpga_arria5.dtsi
> +++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
> @@ -29,7 +29,7 @@
>                         };
>                 };
>
> -               dwmmc0@ff704000 {
> +               mmc0: dwmmc0@ff704000 {
>                         num-slots = <1>;
>                         broken-cd;
>                         bus-width = <4>;
> @@ -41,4 +41,13 @@
>                         cpu1-start-addr = <0xffd080c4>;
>                 };
>         };
> +
> +       regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {

nit: no @0 since there is no "reg" (register) under this node.

nit: usually people don't like "_" in node names.  ...I would probably
do this but I'm not an expert:
  regulator_3_3v_hps: hps-regulator {

This regulator also looks pretty bogus to me.  Is this really a
regulator that software has no control over?  It means you can't fully
reset a card but I guess that's OK.

I'd also expect this regulator to be defined in the same dts / dtsi
file that it's used in.  Your current patch says "there's a generic
3.3V regulator on all boards of the socfpga_arria5 class even if they
don't use MMC, but the "socfpga_arria5_socdk" uses it for MMC.  Is
that really true?
dinguyen@opensource.altera.com Oct. 17, 2014, 7:19 p.m. UTC | #4
On 10/17/2014 11:57 AM, Doug Anderson wrote:
> Dinh,
> 
> On Thu, Oct 16, 2014 at 2:03 PM,  <dinguyen@opensource.altera.com> wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>>
>> dw_mmc ff704000.dwmmc0: No vmmc regulator found
>> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>>
>> This patch adds the regulator node, and points the SD/MMC to the regulator.
>>
[...]
>>
>> -               dwmmc0@ff704000 {
>> +               mmc0: dwmmc0@ff704000 {
>>                         num-slots = <1>;
>>                         broken-cd;
>>                         bus-width = <4>;
>> @@ -41,4 +41,13 @@
>>                         cpu1-start-addr = <0xffd080c4>;
>>                 };
>>         };
>> +
>> +       regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
> 
> nit: no @0 since there is no "reg" (register) under this node.
> 
> nit: usually people don't like "_" in node names.  ...I would probably
> do this but I'm not an expert:
>   regulator_3_3v_hps: hps-regulator {

Ok..will fix up.

> 
> This regulator also looks pretty bogus to me.  Is this really a
> regulator that software has no control over?  It means you can't fully
> reset a card but I guess that's OK.

Yes, this is a generic 3.3V regulator that is used for the 3.3V power
rail for the SOC, and any IP the needs 3.3V.

Schematics are here:
http://www.rocketboards.org/pub/Documentation/AlteraSoCDevelopmentBoard/c5_soc_devkit_c.pdf?t=1365712679

http://www.rocketboards.org/pub/Documentation/ArrowSoCKitEvaluationBoard/SoCKit_RevC.pdf

And right, I don't see anyway to be able to reset the card.

> 
> I'd also expect this regulator to be defined in the same dts / dtsi
> file that it's used in.  Your current patch says "there's a generic
> 3.3V regulator on all boards of the socfpga_arria5 class even if they
> don't use MMC, but the "socfpga_arria5_socdk" uses it for MMC.  Is
> that really true?
> 

yeah...My original patch had the regulator placed in it's appropriate
dts board file, but I saw that as a bit of duplication. I figured that
most people are just copying Altera's devkit schematic, that if a new
board file shows up that has a different controllable regulator, then
the board file can have the new regulator node. Meanwhile, this patch
saves adding an additional regulator node for the existing board files.

The 3.3V is not just used by MMC, but the for all 3.3V peripherals.

Thanks for feedback.

Dinh
Doug Anderson Oct. 17, 2014, 7:43 p.m. UTC | #5
Hi,

On Fri, Oct 17, 2014 at 12:19 PM, Dinh Nguyen
<dinguyen@opensource.altera.com> wrote:
> On 10/17/2014 11:57 AM, Doug Anderson wrote:
>> Dinh,
>>
>> On Thu, Oct 16, 2014 at 2:03 PM,  <dinguyen@opensource.altera.com> wrote:
>>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>
>>> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>>>
>>> dw_mmc ff704000.dwmmc0: No vmmc regulator found
>>> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>>>
>>> This patch adds the regulator node, and points the SD/MMC to the regulator.
>>>
> [...]
>>>
>>> -               dwmmc0@ff704000 {
>>> +               mmc0: dwmmc0@ff704000 {
>>>                         num-slots = <1>;
>>>                         broken-cd;
>>>                         bus-width = <4>;
>>> @@ -41,4 +41,13 @@
>>>                         cpu1-start-addr = <0xffd080c4>;
>>>                 };
>>>         };
>>> +
>>> +       regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
>>
>> nit: no @0 since there is no "reg" (register) under this node.
>>
>> nit: usually people don't like "_" in node names.  ...I would probably
>> do this but I'm not an expert:
>>   regulator_3_3v_hps: hps-regulator {
>
> Ok..will fix up.
>
>>
>> This regulator also looks pretty bogus to me.  Is this really a
>> regulator that software has no control over?  It means you can't fully
>> reset a card but I guess that's OK.
>
> Yes, this is a generic 3.3V regulator that is used for the 3.3V power
> rail for the SOC, and any IP the needs 3.3V.
>
> Schematics are here:
> http://www.rocketboards.org/pub/Documentation/AlteraSoCDevelopmentBoard/c5_soc_devkit_c.pdf?t=1365712679
>
> http://www.rocketboards.org/pub/Documentation/ArrowSoCKitEvaluationBoard/SoCKit_RevC.pdf
>
> And right, I don't see anyway to be able to reset the card.
>
>>
>> I'd also expect this regulator to be defined in the same dts / dtsi
>> file that it's used in.  Your current patch says "there's a generic
>> 3.3V regulator on all boards of the socfpga_arria5 class even if they
>> don't use MMC, but the "socfpga_arria5_socdk" uses it for MMC.  Is
>> that really true?
>>
>
> yeah...My original patch had the regulator placed in it's appropriate
> dts board file, but I saw that as a bit of duplication. I figured that
> most people are just copying Altera's devkit schematic, that if a new
> board file shows up that has a different controllable regulator, then
> the board file can have the new regulator node. Meanwhile, this patch
> saves adding an additional regulator node for the existing board files.
>
> The 3.3V is not just used by MMC, but the for all 3.3V peripherals.

The advantage of putting it in each board file is you could name it
based on the schematics.  I see SoCKit_RevC it is named "VCC3P3"

It also allows you to specify the 3.3 supply as the output of a real
regulator driver.  In s5_soc_devkit it looks like this comes from
LTC2978.  That's got i2c connections.  What happens when you add a
driver for that?
dinguyen@opensource.altera.com Oct. 17, 2014, 8:04 p.m. UTC | #6
On 10/17/2014 02:43 PM, Doug Anderson wrote:
> Hi,
> 
> On Fri, Oct 17, 2014 at 12:19 PM, Dinh Nguyen
> <dinguyen@opensource.altera.com> wrote:
>> On 10/17/2014 11:57 AM, Doug Anderson wrote:
>>> Dinh,
>>>
>>> On Thu, Oct 16, 2014 at 2:03 PM,  <dinguyen@opensource.altera.com> wrote:
>>>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>>
>>>> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>>>>
>>>> dw_mmc ff704000.dwmmc0: No vmmc regulator found
>>>> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>>>>
>>>> This patch adds the regulator node, and points the SD/MMC to the regulator.
>>>>
>> [...]
>>>>
>>>> -               dwmmc0@ff704000 {
>>>> +               mmc0: dwmmc0@ff704000 {
>>>>                         num-slots = <1>;
>>>>                         broken-cd;
>>>>                         bus-width = <4>;
>>>> @@ -41,4 +41,13 @@
>>>>                         cpu1-start-addr = <0xffd080c4>;
>>>>                 };
>>>>         };
>>>> +
>>>> +       regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
>>>
>>> nit: no @0 since there is no "reg" (register) under this node.
>>>
>>> nit: usually people don't like "_" in node names.  ...I would probably
>>> do this but I'm not an expert:
>>>   regulator_3_3v_hps: hps-regulator {
>>
>> Ok..will fix up.
>>
>>>
>>> This regulator also looks pretty bogus to me.  Is this really a
>>> regulator that software has no control over?  It means you can't fully
>>> reset a card but I guess that's OK.
>>
>> Yes, this is a generic 3.3V regulator that is used for the 3.3V power
>> rail for the SOC, and any IP the needs 3.3V.
>>
>> Schematics are here:
>> http://www.rocketboards.org/pub/Documentation/AlteraSoCDevelopmentBoard/c5_soc_devkit_c.pdf?t=1365712679
>>
>> http://www.rocketboards.org/pub/Documentation/ArrowSoCKitEvaluationBoard/SoCKit_RevC.pdf
>>
>> And right, I don't see anyway to be able to reset the card.
>>
>>>
>>> I'd also expect this regulator to be defined in the same dts / dtsi
>>> file that it's used in.  Your current patch says "there's a generic
>>> 3.3V regulator on all boards of the socfpga_arria5 class even if they
>>> don't use MMC, but the "socfpga_arria5_socdk" uses it for MMC.  Is
>>> that really true?
>>>
>>
>> yeah...My original patch had the regulator placed in it's appropriate
>> dts board file, but I saw that as a bit of duplication. I figured that
>> most people are just copying Altera's devkit schematic, that if a new
>> board file shows up that has a different controllable regulator, then
>> the board file can have the new regulator node. Meanwhile, this patch
>> saves adding an additional regulator node for the existing board files.
>>
>> The 3.3V is not just used by MMC, but the for all 3.3V peripherals.
> 
> The advantage of putting it in each board file is you could name it
> based on the schematics.  I see SoCKit_RevC it is named "VCC3P3"
> 

That's fine. I can edit the patch so that each regulator node goes into
it's respective board dts file.

> It also allows you to specify the 3.3 supply as the output of a real
> regulator driver.  In s5_soc_devkit it looks like this comes from
> LTC2978.  That's got i2c connections.  What happens when you add a
> driver for that?
> 

Do you mean the c5_soc_devkit? If so, then the 3.3V is coming from the
LTC3885 regulator. The LTC2978 is a regulator for the the 3.3V_HPS or
3.3V_REG_HPS which is used by others.

So I should change the regulator name from 3.3V_HPS to just 3.3V for the
c5_soc_devkit.

Dinh
Doug Anderson Oct. 17, 2014, 10:36 p.m. UTC | #7
Dinh,

On Fri, Oct 17, 2014 at 1:04 PM, Dinh Nguyen
<dinguyen@opensource.altera.com> wrote:
> On 10/17/2014 02:43 PM, Doug Anderson wrote:
>> Hi,
>>
>> On Fri, Oct 17, 2014 at 12:19 PM, Dinh Nguyen
>> <dinguyen@opensource.altera.com> wrote:
>>> On 10/17/2014 11:57 AM, Doug Anderson wrote:
>>>> Dinh,
>>>>
>>>> On Thu, Oct 16, 2014 at 2:03 PM,  <dinguyen@opensource.altera.com> wrote:
>>>>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>>>
>>>>> Without the 3.3V regulator node, the SDMMC driver will give these warnings:
>>>>>
>>>>> dw_mmc ff704000.dwmmc0: No vmmc regulator found
>>>>> dw_mmc ff704000.dwmmc0: No vqmmc regulator found
>>>>>
>>>>> This patch adds the regulator node, and points the SD/MMC to the regulator.
>>>>>
>>> [...]
>>>>>
>>>>> -               dwmmc0@ff704000 {
>>>>> +               mmc0: dwmmc0@ff704000 {
>>>>>                         num-slots = <1>;
>>>>>                         broken-cd;
>>>>>                         bus-width = <4>;
>>>>> @@ -41,4 +41,13 @@
>>>>>                         cpu1-start-addr = <0xffd080c4>;
>>>>>                 };
>>>>>         };
>>>>> +
>>>>> +       regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
>>>>
>>>> nit: no @0 since there is no "reg" (register) under this node.
>>>>
>>>> nit: usually people don't like "_" in node names.  ...I would probably
>>>> do this but I'm not an expert:
>>>>   regulator_3_3v_hps: hps-regulator {
>>>
>>> Ok..will fix up.
>>>
>>>>
>>>> This regulator also looks pretty bogus to me.  Is this really a
>>>> regulator that software has no control over?  It means you can't fully
>>>> reset a card but I guess that's OK.
>>>
>>> Yes, this is a generic 3.3V regulator that is used for the 3.3V power
>>> rail for the SOC, and any IP the needs 3.3V.
>>>
>>> Schematics are here:
>>> http://www.rocketboards.org/pub/Documentation/AlteraSoCDevelopmentBoard/c5_soc_devkit_c.pdf?t=1365712679
>>>
>>> http://www.rocketboards.org/pub/Documentation/ArrowSoCKitEvaluationBoard/SoCKit_RevC.pdf
>>>
>>> And right, I don't see anyway to be able to reset the card.
>>>
>>>>
>>>> I'd also expect this regulator to be defined in the same dts / dtsi
>>>> file that it's used in.  Your current patch says "there's a generic
>>>> 3.3V regulator on all boards of the socfpga_arria5 class even if they
>>>> don't use MMC, but the "socfpga_arria5_socdk" uses it for MMC.  Is
>>>> that really true?
>>>>
>>>
>>> yeah...My original patch had the regulator placed in it's appropriate
>>> dts board file, but I saw that as a bit of duplication. I figured that
>>> most people are just copying Altera's devkit schematic, that if a new
>>> board file shows up that has a different controllable regulator, then
>>> the board file can have the new regulator node. Meanwhile, this patch
>>> saves adding an additional regulator node for the existing board files.
>>>
>>> The 3.3V is not just used by MMC, but the for all 3.3V peripherals.
>>
>> The advantage of putting it in each board file is you could name it
>> based on the schematics.  I see SoCKit_RevC it is named "VCC3P3"
>>
>
> That's fine. I can edit the patch so that each regulator node goes into
> it's respective board dts file.
>
>> It also allows you to specify the 3.3 supply as the output of a real
>> regulator driver.  In s5_soc_devkit it looks like this comes from
>> LTC2978.  That's got i2c connections.  What happens when you add a
>> driver for that?
>>
>
> Do you mean the c5_soc_devkit? If so, then the 3.3V is coming from the
> LTC3885 regulator. The LTC2978 is a regulator for the the 3.3V_HPS or
> 3.3V_REG_HPS which is used by others.

Oops, right.  c5_soc_devkit_c.

I guess I got confused since your regulator name in the dts was "HPS
3.3V".  When I saw "3.3V_PM_HPS" I assumed it was the same.

On <http://www.rocketboards.org/pub/Documentation/AlteraSoCDevelopmentBoard/c5_soc_devkit_c.pdf?t=1365712679>
I don't see a 3885.  Oh, 3855.  I see.


> So I should change the regulator name from 3.3V_HPS to just 3.3V for the
> c5_soc_devkit.

Yes, that's my opinion.  Matching the schematics can be a huge benefit
and can avoid confusion!  :)

-Doug
diff mbox

Patch

diff --git a/arch/arm/boot/dts/socfpga_arria5.dtsi b/arch/arm/boot/dts/socfpga_arria5.dtsi
index 03e8268..8093781 100644
--- a/arch/arm/boot/dts/socfpga_arria5.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria5.dtsi
@@ -29,7 +29,7 @@ 
 			};
 		};
 
-		dwmmc0@ff704000 {
+		mmc0: dwmmc0@ff704000 {
 			num-slots = <1>;
 			broken-cd;
 			bus-width = <4>;
@@ -41,4 +41,13 @@ 
 			cpu1-start-addr = <0xffd080c4>;
 		};
 	};
+
+	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "HPS 3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
diff --git a/arch/arm/boot/dts/socfpga_arria5_socdk.dts b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
index 27d551c..b7e4023 100644
--- a/arch/arm/boot/dts/socfpga_arria5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
@@ -68,6 +68,11 @@ 
 	};
 };
 
+&mmc0 {
+	vmmc-supply = <&regulator_3_3v_hps>;
+	vqmmc-supply = <&regulator_3_3v_hps>;
+};
+
 &usb1 {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/socfpga_cyclone5.dtsi b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
index 28c05e7..743bc3b 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5.dtsi
+++ b/arch/arm/boot/dts/socfpga_cyclone5.dtsi
@@ -48,4 +48,13 @@ 
 			cpu1-start-addr = <0xffd080c4>;
 		};
 	};
+
+	regulator_3_3v_hps: fixed_3_3v_hps_regulator@0 {
+		compatible = "regulator-fixed";
+		regulator-name = "HPS 3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
index 739c3b7..e1f56ba 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
@@ -70,6 +70,8 @@ 
 
 &mmc0 {
 	cd = <&gpio1 18 0>;
+	vmmc-supply = <&regulator_3_3v_hps>;
+	vqmmc-supply = <&regulator_3_3v_hps>;
 };
 
 &usb1 {
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
index d26f155..ea0c454 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
@@ -53,6 +53,11 @@ 
 	rxc-skew-ps = <2000>;
 };
 
+&mmc0 {
+	vmmc-supply = <&regulator_3_3v_hps>;
+	vqmmc-supply = <&regulator_3_3v_hps>;
+};
+
 &usb1 {
 	status = "okay";
 };