| Message ID | 1426984203-9133-6-git-send-email-stefan.wahren@i2se.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 |
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.
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
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
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 --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 = <®_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 = <®_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 = <®_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 = <®_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 {
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(-)