diff mbox

[5/7] ARM: dts: enable regulator support for i.MX23/i.MX28

Message ID 1426984203-9133-6-git-send-email-stefan.wahren@i2se.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Stefan Wahren March 22, 2015, 12:30 a.m. UTC
This patch enables the on-chip regulator support for all i.MX23 and
i.MX28 boards.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/boot/dts/imx23.dtsi |   67 +++++++++++++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/imx28.dtsi |   67 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 132 insertions(+), 2 deletions(-)

Comments

Sascha Hauer March 23, 2015, 6:07 a.m. UTC | #1
On Sun, Mar 22, 2015 at 12:30:01AM +0000, Stefan Wahren wrote:
> This patch enables the on-chip regulator support for all i.MX23 and
> i.MX28 boards.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  arch/arm/boot/dts/imx23.dtsi |   67 +++++++++++++++++++++++++++++++++++++++++-
>  arch/arm/boot/dts/imx28.dtsi |   67 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 132 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
> index bbcfb5a..be0aee8 100644
> --- a/arch/arm/boot/dts/imx23.dtsi
> +++ b/arch/arm/boot/dts/imx23.dtsi
> @@ -404,8 +404,73 @@
>  			};
>  
>  			power@80044000 {
> +				compatible = "fsl,imx23-power";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
>  				reg = <0x80044000 0x2000>;
> -				status = "disabled";
> +				interrupts = <3>;
> +				ranges;
> +
> +				dcdc: regulator@80044010 {
> +					reg = <0x80044010 0x10>,
> +					      <0x80044090 0x10>,
> +					      <0x800440c0 0x10>;
> +					reg-names = "base-address",
> +						    "misc-address",
> +						    "status-address";
> +					compatible = "fsl,imx23-dcdc";
> +					regulator-name = "dcdc";
> +					regulator-boot-on;
> +					regulator-always-on;
> +				};

It is very unusual to describe the regulators of a device on a register
level like this and to iomemap each register individually. I think you
should drop the registers here and put this knowledge into the driver
like (nearly?) all others do.

Sascha
Stefan Wahren March 23, 2015, 5:54 p.m. UTC | #2
Hi Sascha,

> Sascha Hauer <s.hauer@pengutronix.de> hat am 23. März 2015 um 07:07
> geschrieben:
>
>
> On Sun, Mar 22, 2015 at 12:30:01AM +0000, Stefan Wahren wrote:
> > This patch enables the on-chip regulator support for all i.MX23 and
> > i.MX28 boards.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> > arch/arm/boot/dts/imx23.dtsi | 67 +++++++++++++++++++++++++++++++++++++++++-
> > arch/arm/boot/dts/imx28.dtsi | 67 +++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 132 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
> > index bbcfb5a..be0aee8 100644
> > --- a/arch/arm/boot/dts/imx23.dtsi
> > +++ b/arch/arm/boot/dts/imx23.dtsi
> > @@ -404,8 +404,73 @@
> > };
> >
> > power@80044000 {
> > + compatible = "fsl,imx23-power";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > reg = <0x80044000 0x2000>;
> > - status = "disabled";
> > + interrupts = <3>;
> > + ranges;
> > +
> > + dcdc: regulator@80044010 {
> > + reg = <0x80044010 0x10>,
> > + <0x80044090 0x10>,
> > + <0x800440c0 0x10>;
> > + reg-names = "base-address",
> > + "misc-address",
> > + "status-address";
> > + compatible = "fsl,imx23-dcdc";
> > + regulator-name = "dcdc";
> > + regulator-boot-on;
> > + regulator-always-on;
> > + };
>
> It is very unusual to describe the regulators of a device on a register
> level like this and to iomemap each register individually. I think you
> should drop the registers here and put this knowledge into the driver
> like (nearly?) all others do.

do mean dropping the base address of the regulator, too?

How would you implement it (bare register address or regmap or syscon), since
there
are overlapping register regions for different driver?

@Mark: What's your opinion?

Stefan

>
> Sascha
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown March 23, 2015, 6:37 p.m. UTC | #3
On Mon, Mar 23, 2015 at 06:54:34PM +0100, Stefan Wahren wrote:
> > Sascha Hauer <s.hauer@pengutronix.de> hat am 23. März 2015 um 07:07

> > It is very unusual to describe the regulators of a device on a register
> > level like this and to iomemap each register individually. I think you
> > should drop the registers here and put this knowledge into the driver
> > like (nearly?) all others do.

> do mean dropping the base address of the regulator, too?

> How would you implement it (bare register address or regmap or syscon), since
> there
> are overlapping register regions for different driver?

> @Mark: What's your opinion?

This might be the sort of thing normally handled by a system controller,
I have to say I had assumed this had all been discussed in the previous
iterations of this seriee.
Sascha Hauer March 24, 2015, 6:12 a.m. UTC | #4
On Mon, Mar 23, 2015 at 06:54:34PM +0100, Stefan Wahren wrote:
> Hi Sascha,
> 
> > It is very unusual to describe the regulators of a device on a register
> > level like this and to iomemap each register individually. I think you
> > should drop the registers here and put this knowledge into the driver
> > like (nearly?) all others do.
> 
> do mean dropping the base address of the regulator, too?

Yes.

Sascha
Stefan Wahren March 24, 2015, 6:45 a.m. UTC | #5
Hi Mark,

Am 23.03.2015 um 19:37 schrieb Mark Brown:
> On Mon, Mar 23, 2015 at 06:54:34PM +0100, Stefan Wahren wrote:
>>> Sascha Hauer <s.hauer@pengutronix.de> hat am 23. März 2015 um 07:07
>>> It is very unusual to describe the regulators of a device on a register
>>> level like this and to iomemap each register individually. I think you
>>> should drop the registers here and put this knowledge into the driver
>>> like (nearly?) all others do.
>> do mean dropping the base address of the regulator, too?
>> How would you implement it (bare register address or regmap or syscon), since
>> there
>> are overlapping register regions for different driver?
>> @Mark: What's your opinion?
> This might be the sort of thing normally handled by a system controller,
> I have to say I had assumed this had all been discussed in the previous
> iterations of this seriee.

sorry, but i missed this important fact.

If you speak of system controller, do you mean the usage of
drivers/mfd/syscon.c ?

Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown March 24, 2015, 5:01 p.m. UTC | #6
On Tue, Mar 24, 2015 at 07:45:39AM +0100, Stefan Wahren wrote:
> Am 23.03.2015 um 19:37 schrieb Mark Brown:

> > This might be the sort of thing normally handled by a system controller,
> > I have to say I had assumed this had all been discussed in the previous
> > iterations of this seriee.

> sorry, but i missed this important fact.

> If you speak of system controller, do you mean the usage of
> drivers/mfd/syscon.c ?

Yes.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index bbcfb5a..be0aee8 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -404,8 +404,73 @@ 
 			};
 
 			power@80044000 {
+				compatible = "fsl,imx23-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <3>;
+				ranges;
+
+				dcdc: regulator@80044010 {
+					reg = <0x80044010 0x10>,
+					      <0x80044090 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "misc-address",
+						    "status-address";
+					compatible = "fsl,imx23-dcdc";
+					regulator-name = "dcdc";
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddd: regulator@80044040 {
+					reg = <0x80044040 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx23-vddd";
+					regulator-name = "vddd";
+					regulator-min-microvolt = <1350000>;
+					regulator-max-microvolt = <1550000>;
+					vddd-supply = <&reg_vdda>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vdda: regulator@80044050 {
+					reg = <0x80044050 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx23-vdda";
+					regulator-name = "vdda";
+					regulator-min-microvolt = <1725000>;
+					regulator-max-microvolt = <1950000>;
+					vdda-supply = <&reg_vddio>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddio: regulator@80044060 {
+					reg = <0x80044060 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx23-vddio";
+					regulator-name = "vddio";
+					regulator-min-microvolt = <3000000>;
+					regulator-max-microvolt = <3575000>;
+					regulator-microvolt-offset = <80000>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
 			};
 
 			saif1: saif@80046000 {
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 02330f4..98c1be6 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -1022,8 +1022,73 @@ 
 			};
 
 			power: power@80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+				ranges;
+
+				dcdc: regulator@80044010 {
+					reg = <0x80044010 0x10>,
+					      <0x80044090 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "misc-address",
+						    "status-address";
+					compatible = "fsl,imx28-dcdc";
+					regulator-name = "dcdc";
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddd: regulator@80044040 {
+					reg = <0x80044040 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx28-vddd";
+					regulator-name = "vddd";
+					regulator-min-microvolt = <1350000>;
+					regulator-max-microvolt = <1550000>;
+					vddd-supply = <&reg_vdda>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vdda: regulator@80044050 {
+					reg = <0x80044050 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx28-vdda";
+					regulator-name = "vdda";
+					regulator-min-microvolt = <1725000>;
+					regulator-max-microvolt = <1950000>;
+					vdda-supply = <&reg_vddio>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddio: regulator@80044060 {
+					reg = <0x80044060 0x10>,
+					      <0x80044010 0x10>,
+					      <0x800440c0 0x10>;
+					reg-names = "base-address",
+						    "v5ctrl-address",
+						    "status-address";
+					compatible = "fsl,imx28-vddio";
+					regulator-name = "vddio";
+					regulator-min-microvolt = <3000000>;
+					regulator-max-microvolt = <3550000>;
+					regulator-microvolt-offset = <80000>;
+					regulator-boot-on;
+					regulator-always-on;
+				};
 			};
 
 			saif1: saif@80046000 {